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

login
register
mail settings
Submitter Ankur Ankan
Date April 19, 2013, 5:59 a.m.
Message ID <CANnmyOv9NFm21G7ogoT=W-GqxSWjK3NA_0vJNvtY31YJZHTSUw@mail.gmail.com>
Download mbox | patch
Permalink /patch/1451/
State Changes Requested
Headers show

Comments

Ankur Ankan - April 19, 2013, 5:59 a.m.
# HG changeset patch
# User Ankur Ankan <ankurankan@gmail.com>
# Date 1366350797 -19800
#      Fri Apr 19 11:23:17 2013 +0530
# Node ID c383a984c6047a259a95152d1c7b26eaeb0b2c9a
# Parent  a59e575c6ff87b517a8bb167c509a93003bfef53
help: show topic help with --help (issue3749)

The possible commands are hg --help topicname / hg topicname --help.
If more options are specified in the command line(eg hg -v --help topic
--debug)
it would show help for the topic.
Normally a command after parsing is checked whether it is in the command
list
or not, if not an UnknownCommand exception is raised, and since topic is not
a command it would raise an UnknownCommand Exception. The most obvious
solution
was to add topics to command list but it might break other things
(eg hg revset wouldn't raise UnknownCommand error). And also a topic
name should not be in command list. Other option was to edit
findcmd or _parse but it would result in messy code with too many
try-except.
So the best option is to check for --help in the command line if parse
raises
an UnknownCommand error and call commands.help_ and if there's no --help in
the command line the normal exception would be raised.

+   annotate      show changeset information by line for each file
+   clone         make a copy of an existing repository
+   commit        commit the specified files or all outstanding changes
+   diff          diff repository (or selected files)
+   export        dump the header and diffs for one or more changesets
+   forget        forget the specified files on the next commit
+   init          create a new repository in the given directory
+   log           show revision history of entire repository or files
+   merge         merge working directory with another revision
+   pull          pull changes from the specified source
+   push          push changes to the specified destination
+   remove        remove the specified files on the next commit
+   serve         start stand-alone webserver
+   status        show changed files in the working directory
+   summary       summarize working directory state
+   update        update working directory (or switch revisions)
+
+  use "hg help" for the full list of commands or "hg -v" for details
+  [255]
+
 Test templating help

   $ hg help templating | egrep '(desc|diffstat|firstline|nonempty)  '



On Wed, Apr 17, 2013 at 7:13 PM, Mads Kiilerich <mads@kiilerich.com> wrote:

>   [Resending with proper formatting ... mehope]
>
> Hi
>
> Nice patch.
>
> Anyway, here are some comments that perhaps can make it even better.
>
> On 04/17/2013 11:47 AM, Ankur Ankan wrote:
>
>  # HG changeset patch
> # User Ankur Ankan <ankurankan@gmail.com>
> # Date 1366191853 -19800
> #      Wed Apr 17 15:14:13 2013 +0530
> # Node ID 3c08507d8eff9c67d376e7283e096e64e794197b
> # Parent  a59e575c6ff87b517a8bb167c509a93003bfef53
> help: show topic help with --help (issue3749)
>
>
> I would like to see a bit more description here. (
> http://mercurial.selenic.com/wiki/ContributingChanges#Patch_descriptions )
>
> First of all I would like to see an example of what command line it is
> that will be possible. It is not obvious from the summary line.
>
> I assume that one thing you learned while creating this patch is that it
> is a bit tricky because help topics not are commands and options like
> --help can't take optional parameters. A reviewer might be wondering why
> this can't be implemented the same way as ordinary --help is. Briefly
> explaining the tricky part makes it easier to review ... and to understand
> the change later on.
>
> Have you considered the alternative approach of adding help topics to the
> command list? It with perhaps be interesting with a brief note telling why
> you decided not do that.
>
>  diff -r a59e575c6ff8 -r 3c08507d8eff mercurial/dispatch.py
> --- a/mercurial/dispatch.py Tue Apr 16 15:33:18 2013 +0200
> +++ b/mercurial/dispatch.py Wed Apr 17 15:14:13 2013 +0530
> @@ -635,7 +635,18 @@
>          encoding.fallbackencoding = fallback
>
>      fullargs = args
> -    cmd, func, args, options, cmdoptions = _parse(lui, args)
> +    try:
> +        cmd, func, args, options, cmdoptions = _parse(lui, args)
> +    except error.UnknownCommand:
> +        if '--help' in fullargs[:-1]:
> +            commands.help_(ui, fullargs[-1])
>
>
> These two lines seems to assume that the options are given exactly as
> '--help topicname'.
>
> Shouldn't 'hg topicname --help' be supported too? The tests indicate that
> it intentionally shouldn't. I would expect a brief explanation of that in
> the description.
>
> And how about 'hg -v --help urls --debug' ?
>
>  +            return
> +        else:
>
>
> else is not necessary after an unconditional return. It could be argued
> that being explicit is good ... but usually we like briefness and no
> redundancy even more.
>
>  +            for args in fullargs:
> +                if args[0] != '-':
> +                    cmd = args
> +                    break
> +            raise error.UnknownCommand(cmd)
>
>
> This is all just to raise the exception again if there was no --help? Then
> just use 'raise'.
>
>
>      if options["config"]:
>          raise util.Abort(_("option --config may not be abbreviated!"))
> diff -r a59e575c6ff8 -r 3c08507d8eff tests/test-help.t
> --- a/tests/test-help.t Tue Apr 16 15:33:18 2013 +0200
> +++ b/tests/test-help.t Wed Apr 17 15:14:13 2013 +0530
> @@ -741,6 +741,419 @@
>        The reserved name "." indicates the working directory parent. If no
>        working directory is checked out, it is equivalent to null. If an
>        uncommitted merge is in progress, "." is the revision of the first
> parent.
> +Test for topic help with --help
>
>
> I suggest adding an empty line before the description of the new test -
> that makes the test file more readable.
>
>  +
> +  $ hg --help revset
>
>
> (revset is one of the bigger help topics. Using a short one like
> 'multirevs' makes the test less verbose and more readable.)
>
>  +Test for Unknown Command when hg revset --help is used
> +
> +  $ hg revset --help
> +  hg: unknown command 'revset'
> +  Mercurial Distributed SCM
>
>
> It might also be an idea to test some other 'evil' command lines such as
> the example I gave above - no matter if you intentionally do support them
> or not.
>
> 'hg --help foo' might also be interesting to test ... unless we already
> have test coverage for that.
>
> /Mads
>

Patch

diff -r a59e575c6ff8 -r c383a984c604 mercurial/dispatch.py
--- a/mercurial/dispatch.py Tue Apr 16 15:33:18 2013 +0200
+++ b/mercurial/dispatch.py Fri Apr 19 11:23:17 2013 +0530
@@ -635,7 +635,17 @@ 
         encoding.fallbackencoding = fallback

     fullargs = args
-    cmd, func, args, options, cmdoptions = _parse(lui, args)
+    try:
+        cmd, func, args, options, cmdoptions = _parse(lui, args)
+    except error.UnknownCommand:
+        if '--help' in fullargs:
+            for args in fullargs:
+                if args[0] != '-':
+                    cmd = args
+                    break
+            commands.help_(ui, cmd)
+            return
+        raise

     if options["config"]:
         raise util.Abort(_("option --config may not be abbreviated!"))
diff -r a59e575c6ff8 -r c383a984c604 tests/test-help.t
--- a/tests/test-help.t Tue Apr 16 15:33:18 2013 +0200
+++ b/tests/test-help.t Fri Apr 19 11:23:17 2013 +0530
@@ -742,6 +742,147 @@ 
       working directory is checked out, it is equivalent to null. If an
       uncommitted merge is in progress, "." is the revision of the first
parent.

+Test for topic help with --help (hg --help topicname)
+
+  $ hg --help multirevs
+  Specifying Multiple Revisions
+  """""""""""""""""""""""""""""
+
+      When Mercurial accepts more than one revision, they may be specified
+      individually, or provided as a topologically continuous range,
separated
+      by the ":" character.
+
+      The syntax of range notation is [BEGIN]:[END], where BEGIN and END
are
+      revision identifiers. Both BEGIN and END are optional. If BEGIN is
not
+      specified, it defaults to revision number 0. If END is not
specified, it
+      defaults to the tip. The range ":" thus means "all revisions".
+
+      If BEGIN is greater than END, revisions are treated in reverse order.
+
+      A range acts as a closed interval. This means that a range of 3:5
gives 3,
+      4 and 5. Similarly, a range of 9:6 gives 9, 8, 7, and 6.
+
+Test for topic help with --help (hg topicname --help)
+
+  $ hg multirevs --help
+  Specifying Multiple Revisions
+  """""""""""""""""""""""""""""
+
+      When Mercurial accepts more than one revision, they may be specified
+      individually, or provided as a topologically continuous range,
separated
+      by the ":" character.
+
+      The syntax of range notation is [BEGIN]:[END], where BEGIN and END
are
+      revision identifiers. Both BEGIN and END are optional. If BEGIN is
not
+      specified, it defaults to revision number 0. If END is not
specified, it
+      defaults to the tip. The range ":" thus means "all revisions".
+
+      If BEGIN is greater than END, revisions are treated in reverse order.
+
+      A range acts as a closed interval. This means that a range of 3:5
gives 3,
+      4 and 5. Similarly, a range of 9:6 gives 9, 8, 7, and 6.
+
+Test topic help with --help with additional options
+
+  $ hg -v --help urls --debug
+  URL Paths
+  """""""""
+
+      Valid URLs are of the form:
+
+        local/filesystem/path[#revision]
+        file://local/filesystem/path[#revision]
+        http://[user[:pass]@]host[:port]/[path][#revision]
+        https://[user[:pass]@]host[:port]/[path][#revision]
+        ssh://[user@]host[:port]/[path][#revision]
+
+      Paths in the local filesystem can either point to Mercurial
repositories
+      or to bundle files (as created by "hg bundle" or "hg incoming
--bundle").
+      See also "hg help paths".
+
+      An optional identifier after # indicates a particular branch, tag, or
+      changeset to use from the remote repository. See also "hg help
revisions".
+
+      Some features, such as pushing to http:// and https:// URLs are only
+      possible if the feature is explicitly enabled on the remote Mercurial
+      server.
+
+      Note that the security of HTTPS URLs depends on proper configuration
of
+      web.cacerts.
+
+      Some notes about using SSH with Mercurial:
+
+      - SSH requires an accessible shell account on the destination
machine and
+        a copy of hg in the remote path or specified with as remotecmd.
+      - path is relative to the remote user's home directory by default.
Use an
+        extra slash at the start of a path to specify an absolute path:
+
+          ssh://example.com//tmp/repository
+
+      - Mercurial doesn't use its own compression via SSH; the right thing
to do
+        is to configure it in your ~/.ssh/config, e.g.:
+
+          Host *.mylocalnetwork.example.com
+            Compression no
+          Host *
+            Compression yes
+
+        Alternatively specify "ssh -C" as your ssh command in your
configuration
+        file or with the --ssh command line option.
+
+      These URLs can all be stored in your configuration file with path
aliases
+      under the [paths] section like so:
+
+        [paths]
+        alias1 = URL1
+        alias2 = URL2
+        ...
+
+      You can then use the alias for any command that uses a URL (for
example
+      "hg pull alias1" will be treated as "hg pull URL1").
+
+      Two path aliases are special because they are used as defaults when
you do
+      not provide the URL to a command:
+
+      default:
+        When you create a repository with hg clone, the clone command
saves the
+        location of the source repository as the new repository's 'default'
+        path. This is then used when you omit path from push- and pull-like
+        commands (including incoming and outgoing).
+
+      default-push:
+        The push command will look for a path named 'default-push', and
prefer
+        it over 'default' if both are defined.
+
+Test --help with unknown command/topicname
+
+  $ hg --help foo
+  hg: unknown command 'foo'
+  Mercurial Distributed SCM
+
+  basic commands:
+
+   add           add the specified files on the next commit