Re: Bug or unexpected behaviour in git show <rev>:a\b

From: Jeff King <peff@peff.net>
Date: Jan 25, 2020, 00:05 UTC
Message-ID: <20200125000542.GA566779@coredump.intra.peff.net>
In-Reply-To: <20200124190119.GB1672342@coredump.intra.peff.net>

On Fri, Jan 24, 2020 at 02:01:19PM -0500, Jeff King wrote:

Show 6 quoted lines

> Also, unrelated to your problem, but I find it interesting in the output
> above that "git show HEAD:foo --" produces a less useful error message
> than "git show HEAD:foo" without the separator, even though the user has
> given us even more information about their intent. I think the DWIM
> verify_filename() has grown a lot more diagnosis code over the years
> that could also be used in the other code path.

I took a look at this, but it gets ugly pretty quickly, so I gave up. The problem is that all of the good errors are produced by get_oid() with GET_OID_ONLY_TO_DIE, but the moment where we decide to produce the bad error is sometimes removed from the get_oid() call. E.g., in something like "a..b", we'd want to report on whichever of "a" or "b" caused a problem, but at the point of reporting the error, we only have the unparsed "a..b".

Certainly this can be overcome, but it seemed like a rabbit-hole that I didn't feel like going down today. Anybody is welcome to give it a shot. I suspect the whole thing would be simpler if get_oid() could optionally return an error string to the caller, instead of forcing us to call it a second time with the "only to die" flag.

Speaking of which, I did go part-way down a different rabbit-hole in preparation, and ended up with these patches. So it seemed worth posting them.

  [1/3]: t1400: avoid "test" string comparisons
  [2/3]: t1506: drop space after redirection operator
  [3/3]: sha1-name: mark get_oid() error messages for translation

 sha1-name.c                    | 30 +++++++++++------------
 t/t1400-update-ref.sh          | 36 ++++++++++++++++++----------
 t/t1506-rev-parse-diagnosis.sh | 44 +++++++++++++++++-----------------
 3 files changed, 61 insertions(+), 49 deletions(-)

-Peff