Patchwork fix for bug4240

login
register
mail settings
Submitter Prabhu GS
Date June 24, 2014, 9:39 a.m.
Message ID <CAJJU0hPEZkHXPv=3eMLX67mkMMCA0FHwjiMR1a=1StBbY5dmbA@mail.gmail.com>
Download mbox | patch
Permalink /patch/5051/
State Changes Requested
Headers show

Comments

Prabhu GS - June 24, 2014, 9:39 a.m.
Hi all,

After hg 3.0, "hg showconfig --help" was showing the configuration man page
instead
of the expected help page. I have fixed this regression by the below patch.
Please let me know your views.



# HG changeset patch
# User Prabhu Gnana Sundar <pprabhugs@gmail.com>
# Date 1403602215 -19800
#      Tue Jun 24 15:00:15 2014 +0530
# Node ID 128bef6940f0eeb567b9b006178d2ecdce785904
# Parent  99db956b88ab699644c99095fecadbc4c83adbfc
fix: hg showconfig --help should show command help (bug4240)

This patch fixes the 'showconfig --help' regression issue.
This patch removes an unwanted assignement of aliases[0] to cmd and
shows the correct command in the first line of help description.




This is my first patch to hg. I am also attaching the patch, just in case
needed.

Thanks and regards
Prabhu
# HG changeset patch
# User Prabhu Gnana Sundar <pprabhugs@gmail.com>
# Date 1403602215 -19800
#      Tue Jun 24 15:00:15 2014 +0530
# Node ID 128bef6940f0eeb567b9b006178d2ecdce785904
# Parent  99db956b88ab699644c99095fecadbc4c83adbfc
fix: hg showconfig --help should show command help (bug4240)

This patch fixes the 'showconfig --help' regression issue.
This patch removes an unwanted assignement of aliases[0] to cmd and
shows the correct command in the first line of help description.

diff -r 99db956b88ab -r 128bef6940f0 mercurial/cmdutil.py
--- a/mercurial/cmdutil.py	Sun Jun 01 16:01:01 2014 -0700
+++ b/mercurial/cmdutil.py	Tue Jun 24 15:00:15 2014 +0530
@@ -44,6 +44,8 @@
                     found = a
                     break
         if found is not None:
+            aliases = [x for x in aliases if x != cmd]
+            aliases.insert(0, cmd)
             if aliases[0].startswith("debug") or found.startswith("debug"):
                 debugchoice[found] = (aliases, table[e])
             else:
diff -r 99db956b88ab -r 128bef6940f0 mercurial/dispatch.py
--- a/mercurial/dispatch.py	Sun Jun 01 16:01:01 2014 -0700
+++ b/mercurial/dispatch.py	Tue Jun 24 15:00:15 2014 +0530
@@ -494,7 +494,6 @@
         cmd, args = args[0], args[1:]
         aliases, entry = cmdutil.findcmd(cmd, commands.table,
                                          ui.configbool("ui", "strict"))
-        cmd = aliases[0]
         args = aliasargs(entry[0], args)
         defaults = ui.config("defaults", cmd)
         if defaults:
Augie Fackler - June 24, 2014, 6:51 p.m.
On Tue, Jun 24, 2014 at 03:09:12PM +0530, Prabhu GS wrote:
> Hi all,
>
> After hg 3.0, "hg showconfig --help" was showing the configuration man page
> instead
> of the expected help page. I have fixed this regression by the below patch.
> Please let me know your views.

Please take a look at the checklist here
http://mercurial.selenic.com/wiki/ContributingChanges, especially
points 1, 2, 5 (the above text should be in the commit message) and
maybe 6.


>
>
>
> # HG changeset patch
> # User Prabhu Gnana Sundar <pprabhugs@gmail.com>
> # Date 1403602215 -19800
> #      Tue Jun 24 15:00:15 2014 +0530
> # Node ID 128bef6940f0eeb567b9b006178d2ecdce785904
> # Parent  99db956b88ab699644c99095fecadbc4c83adbfc
> fix: hg showconfig --help should show command help (bug4240)

better topic might be "dispatch: fix so that --help more reliably shows command help (issue4240)"

And then explain the failure mode here (that is, you see 'hg help
config' output and not 'hg help -c showconfig' output).

>
> This patch fixes the 'showconfig --help' regression issue.
> This patch removes an unwanted assignement of aliases[0] to cmd and
> shows the correct command in the first line of help description.
>
> diff -r 99db956b88ab -r 128bef6940f0 mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py      Sun Jun 01 16:01:01 2014 -0700
> +++ b/mercurial/cmdutil.py      Tue Jun 24 15:00:15 2014 +0530
> @@ -44,6 +44,8 @@
>                      found = a
>                      break
>          if found is not None:
> +            aliases = [x for x in aliases if x != cmd]
> +            aliases.insert(0, cmd)
>              if aliases[0].startswith("debug") or found.startswith("debug"):
>                  debugchoice[found] = (aliases, table[e])
>              else:
> diff -r 99db956b88ab -r 128bef6940f0 mercurial/dispatch.py
> --- a/mercurial/dispatch.py     Sun Jun 01 16:01:01 2014 -0700
> +++ b/mercurial/dispatch.py     Tue Jun 24 15:00:15 2014 +0530
> @@ -494,7 +494,6 @@
>          cmd, args = args[0], args[1:]
>          aliases, entry = cmdutil.findcmd(cmd, commands.table,
>                                           ui.configbool("ui", "strict"))
> -        cmd = aliases[0]
>          args = aliasargs(entry[0], args)
>          defaults = ui.config("defaults", cmd)
>          if defaults:
>
>
>
> This is my first patch to hg. I am also attaching the patch, just in case
> needed.

Shouldn't be needed, especially if you use the patchbomb extension as
suggested by the checklist.

>
> Thanks and regards
> Prabhu

> # HG changeset patch
> # User Prabhu Gnana Sundar <pprabhugs@gmail.com>
> # Date 1403602215 -19800
> #      Tue Jun 24 15:00:15 2014 +0530
> # Node ID 128bef6940f0eeb567b9b006178d2ecdce785904
> # Parent  99db956b88ab699644c99095fecadbc4c83adbfc
> fix: hg showconfig --help should show command help (bug4240)
>
> This patch fixes the 'showconfig --help' regression issue.
> This patch removes an unwanted assignement of aliases[0] to cmd and
> shows the correct command in the first line of help description.
>
> diff -r 99db956b88ab -r 128bef6940f0 mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py	Sun Jun 01 16:01:01 2014 -0700
> +++ b/mercurial/cmdutil.py	Tue Jun 24 15:00:15 2014 +0530
> @@ -44,6 +44,8 @@
>                      found = a
>                      break
>          if found is not None:
> +            aliases = [x for x in aliases if x != cmd]
> +            aliases.insert(0, cmd)
>              if aliases[0].startswith("debug") or found.startswith("debug"):
>                  debugchoice[found] = (aliases, table[e])
>              else:
> diff -r 99db956b88ab -r 128bef6940f0 mercurial/dispatch.py
> --- a/mercurial/dispatch.py	Sun Jun 01 16:01:01 2014 -0700
> +++ b/mercurial/dispatch.py	Tue Jun 24 15:00:15 2014 +0530
> @@ -494,7 +494,6 @@
>          cmd, args = args[0], args[1:]
>          aliases, entry = cmdutil.findcmd(cmd, commands.table,
>                                           ui.configbool("ui", "strict"))
> -        cmd = aliases[0]
>          args = aliasargs(entry[0], args)
>          defaults = ui.config("defaults", cmd)
>          if defaults:

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Greg Ward - June 26, 2014, 12:57 p.m.
On 24 June 2014, Prabhu GS said:
> Hi all,
> 
> After hg 3.0, "hg showconfig --help" was showing the configuration man page
> instead
> of the expected help page. I have fixed this regression by the below patch.
> Please let me know your views.
> 
> 
> 
> # HG changeset patch
> # User Prabhu Gnana Sundar <pprabhugs@gmail.com>
> # Date 1403602215 -19800
> #      Tue Jun 24 15:00:15 2014 +0530
> # Node ID 128bef6940f0eeb567b9b006178d2ecdce785904
> # Parent  99db956b88ab699644c99095fecadbc4c83adbfc
> fix: hg showconfig --help should show command help (bug4240)
> 
> This patch fixes the 'showconfig --help' regression issue.
> This patch removes an unwanted assignement of aliases[0] to cmd and
> shows the correct command in the first line of help description.

Great, thanks for your contribution! Augie mentioned most of the
things you missed, but you also need to update the test suite. Any
time we have a bug but the tests still pass, that means the test suite
is missing something. In other words: if you don't have a failing
test, you don't have a bug. The goal is not merely to fix the bug *for
today*, but to ensure that it stays fixed *forever*. The way to do
that is by writing a test.

I think the test to modify is probably tests/test-config.t. See

  http://mercurial.selenic.com/wiki/WritingTests

for more info.

> diff -r 99db956b88ab -r 128bef6940f0 mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py      Sun Jun 01 16:01:01 2014 -0700
> +++ b/mercurial/cmdutil.py      Tue Jun 24 15:00:15 2014 +0530
> @@ -44,6 +44,8 @@
>                      found = a
>                      break
>          if found is not None:
> +            aliases = [x for x in aliases if x != cmd]
> +            aliases.insert(0, cmd)

There's more than one way to do this. Also, I don't like one-letter
variables names (although there's nothing against them in Mercurial's
style guide). Maybe:

    aliases = [cmd] + [alias for alias in aliases if alias != cmd]

or maybe:

    aliases = [cmd]
    aliases += [alias for alias in aliases if alias != cmd]

Your way is perfectly correct, but inserting at the head of a list has
to move the existing list elements, so it's O(N) in runtime. Not
important in this case, but it's good to keep in mind when you are
working in performance-critical code.

       Greg

Patch

diff -r 99db956b88ab -r 128bef6940f0 mercurial/cmdutil.py
--- a/mercurial/cmdutil.py      Sun Jun 01 16:01:01 2014 -0700
+++ b/mercurial/cmdutil.py      Tue Jun 24 15:00:15 2014 +0530
@@ -44,6 +44,8 @@ 
                     found = a
                     break
         if found is not None:
+            aliases = [x for x in aliases if x != cmd]
+            aliases.insert(0, cmd)
             if aliases[0].startswith("debug") or found.startswith("debug"):
                 debugchoice[found] = (aliases, table[e])
             else:
diff -r 99db956b88ab -r 128bef6940f0 mercurial/dispatch.py
--- a/mercurial/dispatch.py     Sun Jun 01 16:01:01 2014 -0700
+++ b/mercurial/dispatch.py     Tue Jun 24 15:00:15 2014 +0530
@@ -494,7 +494,6 @@ 
         cmd, args = args[0], args[1:]
         aliases, entry = cmdutil.findcmd(cmd, commands.table,
                                          ui.configbool("ui", "strict"))
-        cmd = aliases[0]
         args = aliasargs(entry[0], args)
         defaults = ui.config("defaults", cmd)
         if defaults: