not a very informative error message

[Yuya Nishihara]

On Sun, 09 Aug 2015 22:45:44 -0400, Matt Harbison wrote:
> On Sun, 09 Aug 2015 13:16:12 -0400, Gregory Szorc  
> <gregory.szorc at gmail.com> wrote:
> 
> > On Sun, Aug 9, 2015 at 1:25 AM, Yuya Nishihara <yuya at tcha.org> wrote:
> >
> >> On Sat, 08 Aug 2015 22:58:23 -0400, Matt Harbison wrote:
> >> > 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
> >>
> >> > 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.
> >>
> >> Maybe a ParseError could have more verbose output:
> >>
> >>   % hg log -r 1f8f96084e75+
> >>   hg: parse error at 13: not a prefix: end
> >>   1f8f96084e75+
> >>               ~^
> >
> > +1. I tried to implement this a while back, thinking it would be an easy
> > feature and gave up because it required some invasive changes for the
> > raised exceptions to capture context necessary for reporting. Maybe I
> > wasn't approaching the problem correctly. I'm sure Yuya would know better
> > than me.

No, I have no idea to implement it without touching ParseError. And we'll
have to refactor ParseError because it is created with different types of
arguments:

  (message)
  (message, "filename:lineno")
  (context, "filename:lineno")
  (message, pos)

> It seems like at least some paths can give reasonable feedback:
> 
> $ ../hg log -r '1234#'
> hg: parse error at 4: syntax error in revset '1234#'
> 
> (Though this is caught in revset.tokenize, prior to being run through
> parser.  Parse and tokenize are both are called on the same line, so care
> needs to be taken if the solution is to catch ParseError outside of the
> parser to add on the string being parsed.)

I think the easiest (but ugly) hack is to reraise ParseError with the context
attached.

diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -45,6 +45,10 @@ def _formatparse(write, inst):
                          (inst.args[1], inst.args[0]))
         if (inst.args[0][0] == ' '):
             write(_("unexpected leading whitespace\n"))
+        context = getattr(inst, 'context')
+        if context:
+            write(context + '\n')
+            write(' ' * inst.args[1] + '\033[1;32m^\033[m\n')  # XXX want ui.label()
     else:
         write(_("hg: parse error: %s\n") % inst.args[0])
         if similar:
diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -2651,9 +2651,13 @@ def foldconcat(tree):
 
 def parse(spec, lookup=None):
     p = parser.parser(elements)
-    tree, pos = p.parse(tokenize(spec, lookup=lookup))
-    if pos != len(spec):
-        raise error.ParseError(_("invalid token"), pos)
+    try:
+        tree, pos = p.parse(tokenize(spec, lookup=lookup))
+        if pos != len(spec):
+            raise error.ParseError(_("invalid token"), pos)
+    except error.ParseError as inst:
+        inst.context = spec  # XXX ugly
+        raise
     return parser.simplifyinfixops(tree, ('or',))
 
 def posttreebuilthook(tree, repo):