On Fri, Jan 24, 2020 at 02:01:19PM -0500, Jeff King wrote: > 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
