Re: [PATCH] diff: handle --no-abbrev outside of repository
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 28, 2016, 23:03 UTC
- Message-ID
- <xmqqbmwzi4wa.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20161128182508.10570-1-jack@nottheoilrig.com>
Jack Bates <bk874k@nottheoilrig.com> writes:
Show 5 quoted lines
> 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.
Show 26 quoted lines
> 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)) {