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

[Jeff King <peff@peff.net>]

On Mon, Dec 05, 2016 at 01:15:00AM -0500, Jeff King wrote:

> On Mon, Dec 05, 2016 at 01:01:16AM -0500, Jeff King wrote:
> 
> >   Note that setting abbrev to "0" outside of a repository was broken
> >   recently by 4f03666ac (diff: handle sha1 abbreviations outside of
> >   repository, 2016-10-20). It adds a special out-of-repo code path for
> >   handling abbreviations which behaves differently than find_unique_abbrev()
> >   by truly giving a zero-length sha1, rather than taking "0" to mean "do
> >   not abbreviate".
> > 
> >   That bug was not triggerable until now, because there was no way to
> >   set the value to zero (using --abbrev=0 silently bumps it to the
> >   MINIMUM_ABBREV).
> 
> Actually, I take this last paragraph back. You _can_ trigger the bug
> with just:
> 
>   echo one >foo
>   echo two >bar
>   git diff --no-index --raw foo bar
> 
> which prints only "..." for each entry.
> 
> I didn't notice it before because without "--raw", we show the patch
> format. That uses the --full-index option, and does not respect --abbrev
> at all (which seems kind of bizarre, but has been that way forever).
> 
> So I think there _is_ a regression in v2.11, and the second half of your
> change fixes it.

Sorry for the sequence of emails, but as usual with "diff --no-index",
the deeper I dig the more confusion I find. :)

After digging into your related thread in:

  http://public-inbox.org/git/20161205065523.yspqt34p3dp5g5fk@sigill.intra.peff.net/

I'm not convinced that "--no-index --raw" output isn't generally
nonsensical in the first place. So yes, there's a regression there (and
it's not just "oops, we didn't abbreviate correctly", but rather that
the output format is broken). But I'm not sure it's something people are
using. So it should be fixed on the 'maint' track, but I don't think
it's incredibly urgent.

-Peff