Re: [BUG] redundant error message

From: Jeff King <peff@peff.net>
Date: Dec 5, 2013, 21:00 UTC
Message-ID: <20131205210000.GA19617@sigill.intra.peff.net>
In-Reply-To: <xmqqfvq73vhw.fsf@gitster.dls.corp.google.com>

On Thu, Dec 05, 2013 at 12:15:07PM -0800, Junio C Hamano wrote:

Show 23 quoted lines

> >> Hmph, it looks like it is following the usual "zero-or-more dashed
> >> options, zero-or-more revs and then zero-or-one double-dash and then
> >> zero-or-more paths" rule to parse the thing.  "foobar" is a file and
> >> not a rev, so "--" should not be there, no?
> >> 
> >> Confused why you think it is not right...
> >
> > Because once you say "--", then all ambiguity goes away, no?
> 
> But it is tricky (not from implementation but from semantics point
> of view) to make rev-parse follow that "-- separates revs and paths"
> rule literally.  The primary use of rev-parse is to convert revs in
> extended SHA-1 expressions into concrete object names, so that
> scripts do not have to worry about having to deal with object names
> in a format that is not 40-hexdecimal.  "git rev-parse foobar --"
> that gives
> 
> 	foobar
>         --
> 
> without any error, because 'foobar' cannot be made into an object
> name, would be behaving in a way unexpected by the calling script,
> no?

Yes, I do expect an error. But it should not be "-- after filename". It should be "foobar is not a revision".

Thinking on it more, though, the problem is purely limited to wrong error messages. If "foobar" exists as a rev, we do parse it correctly. If it does not, we are in the wrong code path, but it _must_ be an error at that point (either because foobar does not exist as a file, or it does and has "--" after it).

It would be nice to get the error messages right, though. I do not see any reason why it could not follow the same steps as "git log", converting revisions (or throwing an error as appropriate) on the left side of the "--", and passing through the right side untouched.

-Peff