Jack Bates <bk8...@nottheoilrig.com> writes:

> The "git diff --no-index" codepath doesn't handle the --no-abbrev
> option.
>
> Signed-off-by: Jack Bates <j...@nottheoilrig.com>
> ---

This patch also needs a new test to protect the fix from future
breakages.

It is unfortunate that parsing of these options that are done in
diff_opt_parse() are not used by most of the codepaths; they instead
rely on revision.c parser to parse them into revs->abbrev and then
copied to revs->diffopt.abbrev in setup_revisions().  We would want
to rethink the structure of the code around this, and possibly move
towards using setup_revisions() more when appropriate and removing
diff_opt_parse() or something like that; the three-way fallback
codepath in builtin/am.c is the only other caller of this function
and it uses it to parse a fixed "--diff-filter=AM" option into
rev_info.diffopt and manually sets up rev_info as if revision parser
was given "diff --cached HEAD", which we should be able to replace
with a call to setup_revisions() of "--diff-filter=AM --cached HEAD",
I would suspect.  But that is a much larger change.

In any case, for now, the fix in this patch is a single best step
that moves us forward.  

Thanks.

>  diff.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/diff.c b/diff.c
> index ec87283..0447eff 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -3106,7 +3106,8 @@ static const char *diff_abbrev_oid(const struct 
> object_id *oid, int abbrev)
>                       abbrev = FALLBACK_DEFAULT_ABBREV;
>               if (abbrev > GIT_SHA1_HEXSZ)
>                       die("BUG: oid abbreviation out of range: %d", abbrev);
> -             hex[abbrev] = '\0';
> +             if (abbrev)
> +                     hex[abbrev] = '\0';
>               return hex;
>       }
>  }
> @@ -4024,6 +4025,8 @@ int diff_opt_parse(struct diff_options *options,
>                           offending, optarg);
>               return argcount;
>       }
> +     else if (!strcmp(arg, "--no-abbrev"))
> +             options->abbrev = 0;
>       else if (!strcmp(arg, "--abbrev"))
>               options->abbrev = DEFAULT_ABBREV;
>       else if (skip_prefix(arg, "--abbrev=", &arg)) {

Reply via email to