On Wed, Dec 6, 2017 at 4:16 PM, Jacob Keller <jacob.keller@xxxxxxxxx> wrote:
> On Sun, Dec 3, 2017 at 9:04 AM, Christian Couder
> <christian.couder@xxxxxxxxx> wrote:
>> From: Christian Couder <christian.couder@xxxxxxxxx>
>>
>> Let's simplify diff option parsing using skip_to_opt_val().
>>
>> Signed-off-by: Christian Couder <chriscool@xxxxxxxxxxxxx>
>> ---
>> 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
I believe this occurs because skip_to_optional_val overwrites the arg
even if it's not there, and the --relative argument expects prefix to
remain unchanged if the optional value is not provided.
Thanks,
Jake
