Re: [PATCH 10/10] get_short_sha1: list ambiguous objects on error

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 29, 2016, 17:19 UTC
Message-ID: <xmqqbmz6hbdk.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160929130322.562ng4t2ktk6qzok@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 21 quoted lines

>> $ git rev-parse --disambiguate-list=b2e1
>> b2e1196 tag v2.8.0-rc1
>> b2e11d1 tree
>> b2e1632 commit 2007-11-14 - Merge branch 'bs/maint-commit-options'
>> b2e1759 blob
>> b2e18954 blob
>> b2e1895c blob
>
> I think the "right" way to do this is pipe the list of sha1s into
> another git commit which can format them however you want.
> Unfortunately, there isn't a single command that does a great job:
>
>   - "cat-file --batch-check" can show you the sha1 and type, but it
>     won't abbreviate sha1s, and it won't show you commit/tag information
>
>   - "log --stdin --no-walk" will format the commit however you like, but
>     skips the trees and blobs entirely, and the tag can only be seen via
>     "%d"
>
>   - "for-each-ref" has flexible formatting, too, but wants to format
>     refs, not objects (and doesn't read from stdin).

    - "name-rev" is used to give "describe --contains", and can read
      from its standard input, but has no format customization.
      Another downside of it is that it only wants to see
      committishes.

Show 18 quoted lines

> IMHO that is a sign that our formatting tools aren't as good as they
> could be (I think the right tool is cat-file, but it should be able to
> do all of the formatting that the other commands can do).
>
> Of course if you really just want human-readable output, then:
>
>   $ git cat-file -e b2e1
>   error: short SHA1 b2e1 is ambiguous
>   hint: The candidates are:
>   hint:   b2e1196 tag v2.8.0-rc1
>   hint:   b2e11d1 tree
>   hint:   b2e1632 commit 2007-11-14 - Merge branch 'bs/maint-commit-options'
>   hint:   b2e1759 blob
>   hint:   b2e18954 blob
>   hint:   b2e1895c blob
>   fatal: Not a valid object name b2e1
>
> is pretty easy.

Yes. I think adding this to rev-parse that is meant for machines is probably a mistake, as this "hint" machinery's output will become even more human friendly over time as we gain experience.

 - If the hypothetical "--disambiguate-list" option wants to produce
   machine parseable output for scripts, it would mean its output
   (and whatgver the reading script can do based on its output for
   humans) will become less useful for humans over time.

 - If the hypothetical "--disambiguate-list" option only wants to
   replicate the human readable output that is designed to be
   improved over time and expects its output _not_ to be interpreted
   by scripts but merely be relayed, then why aren't these scripts
   just invoking the commands that already gives the "hint:" output
   and showing that directly to humans in the first place?

> That being said, I don't mind if somebody wanted to do a rev-parse
> option on top of my series. The formatting code is already split into
> its own function.

So let's not go there.