not a very informative error message

[Matt Harbison]

On Sat, 08 Aug 2015 22:13:04 -0400, Cameron Simpson <cs at zip.com.au> wrote:

> On 08Aug2015 21:11, Matt Harbison <mharbison72 at gmail.com> wrote:
>> On Fri, 07 Aug 2015 07:19:38 -0400, Neal Becker <ndbecker2 at gmail.com>  
>> wrote:
>>
>>> $ hg id
>>> 1f8f96084e75+ (collision) tip
>>>
>>> $ hg cat -r 1f8f96084e75+ test_awgn.py > /dev/null
>>              01234567890123
>>
>>> hg: parse error at 13: not a prefix: end
>>
>> Since this isn't a valid revision or revset, all it can tell you is  
>> that the problem occurred at the 13th character.
>>
>> I'm not sure what you are trying to do here.  The '+' on the end of the  
>> revision simply means that the working directory is dirty- it isn't  
>> part of the revision value.
>
> Well, as an outsider, I read Neal's email to indicate that here is a  
> case where hg's error message in unhelpful to the user.
>
> Specificly, I'd read it to indicate that:
>
> (a) the error message, in this context, sucks. "position 13" in what?  
> "not a prefix"? Oh, not a prefix of a revision hash. "end"? Huh?
>
> (b) it is not an unnatural mistake to grab the whole "1f8f96084e75+"  
> word from the output of "hg id". It might be good to recognise this  
> explicitly in hg's revcode parser and suggest to the user that that is  
> what they did.
>
> Regarding (a), when I write error messages and when I read them, this:
>
>   not a prefix: end
>
> normally suggests that the literal text "end" is either the whole value  
> being rejected, or the tail of the rejected value (i.e. the parse failed  
> when it reached the string "end" in some larger string).
>
> One, IMO genuine, problem with the error is that it does not even make  
> it clear what hg is unhappy about. The "-r" option? The filename? The  
> spelling of "cat"?  Some context is very needed in this message.
>
> Cheers,
> Cameron Simpson <cs at zip.com.au>

I pretty much agree that it isn't helpful, but it looks like the same  
parser code is used for revsets, filesets and templates.  Therefore it  
really can't know about common mistakes specific to one of these in order  
to make suggestions.  It's just parsing based on language tokens it has  
been told about for which ever module is using it, and bails when it gets  
confused.  I'm not sure if this part can be made better.

That said, it would be nice if it would say that the revision arg is bad.   
Debugging a command that used a revision, files and a template would be a  
headache.  I wonder if we can catch the parse error and prefix the message  
with 'revision:' or whatever.  Bad commands are caught and offer a  
suggestion if you misspell cat, for example.  I'm not sure what would  
happen if multiple --rev args are given, but we would be no worse off I  
guess.