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

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:

quoted

  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