Re: [PATCH] diff: handle --no-abbrev outside of repository

[Junio C Hamano <gitster@pobox.com>]

Jack Bates <bk874k@nottheoilrig.com> writes:

> The "git diff --no-index" codepath doesn't handle the --no-abbrev
> option.
>
> Signed-off-by: Jack Bates <jack@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)) {