Re: [PATCH 3/3] diff: use skip_to_opt_val()

[Jacob Keller <jacob.keller@gmail.com>]

On Sun, Dec 3, 2017 at 9:04 AM, Christian Couder
<christian.couder@gmail.com> wrote:
> From: Christian Couder <christian.couder@gmail.com>
>
> Let's simplify diff option parsing using skip_to_opt_val().
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  diff.c | 22 ++++++++--------------
>  1 file changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index 2ebe2227b4..067b498187 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -4508,17 +4508,11 @@ int diff_opt_parse(struct diff_options *options,
>                 options->output_format |= DIFF_FORMAT_NUMSTAT;
>         else if (!strcmp(arg, "--shortstat"))
>                 options->output_format |= DIFF_FORMAT_SHORTSTAT;
> -       else if (!strcmp(arg, "-X") || !strcmp(arg, "--dirstat"))
> -               return parse_dirstat_opt(options, "");
> -       else if (skip_prefix(arg, "-X", &arg))
> -               return parse_dirstat_opt(options, arg);
> -       else if (skip_prefix(arg, "--dirstat=", &arg))
> +       else if (skip_prefix(arg, "-X", &arg) || skip_to_opt_val(arg, "--dirstat", &arg))
>                 return parse_dirstat_opt(options, arg);
>         else if (!strcmp(arg, "--cumulative"))
>                 return parse_dirstat_opt(options, "cumulative");
> -       else if (!strcmp(arg, "--dirstat-by-file"))
> -               return parse_dirstat_opt(options, "files");
> -       else if (skip_prefix(arg, "--dirstat-by-file=", &arg)) {
> +       else if (skip_to_opt_val(arg, "--dirstat-by-file", &arg)) {
>                 parse_dirstat_opt(options, "files");
>                 return parse_dirstat_opt(options, arg);
>         }
> @@ -4540,13 +4534,13 @@ int diff_opt_parse(struct diff_options *options,
>                 return stat_opt(options, av);
>
>         /* renames options */
> -       else if (starts_with(arg, "-B") || starts_with(arg, "--break-rewrites=") ||
> -                !strcmp(arg, "--break-rewrites")) {
> +       else if (starts_with(arg, "-B") ||
> +                skip_to_opt_val(arg, "--break-rewrites", &optarg)) {
>                 if ((options->break_opt = diff_scoreopt_parse(arg)) == -1)
>                         return error("invalid argument to -B: %s", arg+2);
>         }
> -       else if (starts_with(arg, "-M") || starts_with(arg, "--find-renames=") ||
> -                !strcmp(arg, "--find-renames")) {
> +       else if (starts_with(arg, "-M") ||
> +                skip_to_opt_val(arg, "--find-renames", &optarg)) {
>                 if ((options->rename_score = diff_scoreopt_parse(arg)) == -1)
>                         return error("invalid argument to -M: %s", arg+2);
>                 options->detect_rename = DIFF_DETECT_RENAME;
> @@ -4554,8 +4548,8 @@ int diff_opt_parse(struct diff_options *options,
>         else if (!strcmp(arg, "-D") || !strcmp(arg, "--irreversible-delete")) {
>                 options->irreversible_delete = 1;
>         }
> -       else if (starts_with(arg, "-C") || starts_with(arg, "--find-copies=") ||
> -                !strcmp(arg, "--find-copies")) {
> +       else if (starts_with(arg, "-C") ||
> +                skip_to_opt_val(arg, "--find-copies", &optarg)) {
>                 if (options->detect_rename == DIFF_DETECT_COPY)
>                         options->flags.find_copies_harder = 1;
>                 if ((options->rename_score = diff_scoreopt_parse(arg)) == -1)
> --
> 2.15.1.271.g1a4e40aa5d.dirty
>

This causes a regression in the --relative option which prevents it
from working properly.

If I have a repository with a modified file in a subdirectory, such as:

a/file

then git diff-index --relative --name-only HEAD from within "a" will
return "a/file" instead of "file"

This breaks git completion, (among other things).

Thanks,
Jake