[PATCH] fix for bug4240

Greg Ward greg at gerg.ca
Thu Jun 26 07:57:11 CDT 2014


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 at 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
-- 
Greg Ward                            http://www.gerg.ca
<greg at gerg.ca>                       @gergdotca


More information about the Mercurial-devel mailing list