Patchwork Bug 3749 --help does not show non-command help topics

login
register
mail settings
Submitter Ankur Ankan
Date April 16, 2013, 9:53 a.m.
Message ID <CANnmyOtiUGA1gUkkwvwU9PsuvC0qT_EGUudkcCY+c1Y8sjwVjw@mail.gmail.com>
Download mbox | patch
Permalink /patch/1343/
State Superseded
Headers show

Comments

Ankur Ankan - April 16, 2013, 9:53 a.m.
# HG changeset patch
# User Ankur Ankan <ankurankan@gmail.com>
# Date 1366096607 -19800
#      Tue Apr 16 12:46:47 2013 +0530
# Node ID 9014d2cc9eee50f8b2369c142ba9041a86b29720
# Parent  257afe5489d4a6ebca6a9e13bba1b69a9289b89d
help: --help showing non-command help topics




I know this patch doesn't look good. A better solution that I thought was
to pass an optional parameter (say ishelp) to findcmd which would be True
if --help flag is set. And if ishelp is True findcmd would return the cmd
in aliases without checking if it is in the table. The problem here is what
should be the value of "entry"?

Thanks
Ankur


On Mon, Apr 15, 2013 at 10:24 AM, Matt Mackall <mpm@selenic.com> wrote:

> Not bad for a first attempt, but needs some work.
>
> On Mon, 2013-04-15 at 09:58 +0530, Ankur Ankan wrote:
> > # HG changeset patch
> > # User Ankur Ankan <ankurankan@gmail.com>
> > # Date 1365998534 -19800
> > #      Mon Apr 15 09:32:14 2013 +0530
> > # Node ID 12eb5092b99745e6117e38489c145b7b0cb80576
> > # Parent  4e1ae55e63ef13bbee13256f236d93efe817be69
> > bug 3749
>
> This has the least guideline-compliant patch description I've seen in a
> while. From http://mercurial.selenic.com/wiki/ContributingChanges:
>
> * first line of commit message is of the form "topic: uncapitalized, no
> trailing period"
> * bugs that are resolved are mentioned in summary in the form
> "(issueNNNN)" (no space)
> * all relevant info is in the commit message for posterity (not a "0 of N"
> message)
>
> Look at this for thousands of examples:
>
> $ hg log -M --template '{desc|firstline}\n' | less
>
> > diff -r 4e1ae55e63ef -r 12eb5092b997 mercurial/dispatch.py
> > --- a/mercurial/dispatch.py Fri Apr 12 17:00:42 2013 -0400
> > +++ b/mercurial/dispatch.py Mon Apr 15 09:32:14 2013 +0530
> > @@ -144,13 +144,16 @@
> >      except error.SignalInterrupt:
> >          ui.warn(_("killed!\n"))
> >      except error.UnknownCommand, inst:
> > -        ui.warn(_("hg: unknown command '%s'\n") % inst.args[0])
> >          try:
> > -            # check if the command is in a disabled extension
> > -            # (but don't check for extensions themselves)
> > -            commands.help_(ui, inst.args[0], unknowncmd=True)
> > -        except error.UnknownCommand:
> > -            commands.help_(ui, 'shortlist')
> > +            commands.help_(ui,inst.args[0])
>
> check-code sends its regards:
>
> $ python contrib/check-code.py mercurial/dispatch.py
> mercurial/dispatch.py:151:
>  >             commands.help_(ui,inst.args[0])
>  missing whitespace after ,
> mercurial/dispatch.py:152:
>  >         except:
>  naked except clause
>
> > +        except:
> > +            ui.warn(_("hg: unknown command '%s'\n") % inst.args[0])
>
> Ok, this now works:
>
>  hg --help revsets
>
> ..but so does this, which probably shouldn't:
>
>  hg revsets --help
>
> And so does this, which almost certainly shouldn't:
>
>  hg revsets
>
> --
> Mathematics is the supreme nostalgia of our time.
>
>
>
Matt Mackall - April 16, 2013, 1:18 p.m.
On Tue, 2013-04-16 at 15:23 +0530, Ankur Ankan wrote:
> # HG changeset patch
> # User Ankur Ankan <ankurankan@gmail.com>
> # Date 1366096607 -19800
> #      Tue Apr 16 12:46:47 2013 +0530
> # Node ID 9014d2cc9eee50f8b2369c142ba9041a86b29720
> # Parent  257afe5489d4a6ebca6a9e13bba1b69a9289b89d
> help: --help showing non-command help topics

Getting closer.

The commit message should probably be something like:

  help: show topic help with --help (issue3749)

  This should make 'hg --help revsets' work, with enabling 'hg revsets
  --help'.

Also: this probably needs two short tests added to tests/test-help.t to
match the above.

> I know this patch doesn't look good. A better solution that I thought
> was to pass an optional parameter (say ishelp) to findcmd which would
> be True if --help flag is set. And if ishelp is True findcmd would
> return the cmd in aliases without checking if it is in the table. The
> problem here is what should be the value of "entry"?

I think your first patch was closer?

Patch

diff -r 257afe5489d4 -r 9014d2cc9eee mercurial/dispatch.py
--- a/mercurial/dispatch.py Wed Apr 10 02:27:35 2013 +0900
+++ b/mercurial/dispatch.py Tue Apr 16 12:46:47 2013 +0530
@@ -425,6 +425,7 @@ 
 def _parse(ui, args):
     options = {}
     cmdoptions = {}
+    fullargs = args

     try:
         args = fancyopts.fancyopts(args, commands.globalopts, options)
@@ -433,14 +434,26 @@ 

     if args:
         cmd, args = args[0], args[1:]
-        aliases, entry = cmdutil.findcmd(cmd, commands.table,
+        if fullargs[0] == '--help':
+            aliases = [fullargs[-1]]
+        else:
+            aliases, entry = cmdutil.findcmd(cmd, commands.table,
                                          ui.configbool("ui", "strict"))
         cmd = aliases[0]
-        args = aliasargs(entry[0], args)
+
+        try:
+            args = aliasargs(entry[0], args)
+        except NameError:
+            args = []
+
         defaults = ui.config("defaults", cmd)
         if defaults:
             args = map(util.expandpath, shlex.split(defaults)) + args
 -        c = list(entry[1])
+
+        try:
+            c = list(entry[1])
+        except NameError:
+            c = []
     else:
         cmd = None
         c = []
@@ -460,7 +473,10 @@ 
         options[n] = cmdoptions[n]
         del cmdoptions[n]

-    return (cmd, cmd and entry[0] or None, args, options, cmdoptions)
+    try:
+        return (cmd, cmd and entry[0] or None, args, options, cmdoptions)
+    except NameError:
+        return (cmd, None, args, options, cmdoptions)

 def _parseconfig(ui, config):
     """parse the --config options from the command line"""