Issue2054

Title alias: overriding existing command issues
Priority bug Status resolved
Superseder Nosy List ThomasAH, brodie, cdadhemar, cmisare, djc, hgbot, mg, mpm, tonfa
Assigned To mpm Topics

Created on 2010-02-19.18:26:28 by cdadhemar, last changed 2010-10-09.15:36:15 by mpm.

Files
File name Uploaded Type Edit Remove
aliases-test.sh cmisare, 2010-03-06.10:13:37 application/x-shellscript
Messages
msg13493 (view) Author: brodie Date: 2010-08-26.15:40:20
Documenting the behavior and adding a test for it would be a good idea. I honestly 
hadn't thought of that case.

It'd also be nice to have dispatch tell you what your alias expands to if it had 
invalid arguments:

  hg outgoing: invalid arguments
  alias "outgoing" expands to "outgoing foo foo"

mpm suggested on IRC that we could also print the expansion with --debug.
msg13492 (view) Author: hgbot Date: 2010-08-26.11:00:09
Fixed by http://hg.intevation.org/mercurial/crew/rev/18e1e7520b67
Brodie Rao <brodie@bitheap.org>
alias: make shadowing behavior more consistent (issue2054)
msg13491 (view) Author: ThomasAH Date: 2010-08-26.10:30:41
Pushed to stable by mpm as 18e1e7520b67

I noted a small change with e.g.:

[alias]
sum = summary --debug
summary = summary --remote


Now "hg sum" and "hg summary" both execute "hg summary --debug --remote".

One could say this is an improvement as setting a default option
automatically works for all existing aliases, but this might cause duplicate
options if someone already added more aliases because of this, e.g.:

out = outgoing foo
outgoing = outgoing foo

Now "hg outgoing" will produce:
hg outgoing: invalid arguments

I really like the new behaviour, so can very well live with this change in
the stable branch, I just wanted to document it and have it confirmed by mpm
or anyone from crew.
msg11942 (view) Author: cmisare Date: 2010-03-06.10:13:37
Not sure how relevant this is, but I figure more information doesn't hurt.

Running the attached script using both Mercurial 1.4.2 (installed as the
Fedora 12 package, mercurial-1.4.2-1.fc12.x86_64) and Mercurial 1.4.3
(downloaded from the site) on Python 2.6.2 form Fedora 12 x86_64, I can see
that a defined alias trying to override a standard command is not being
respected.
msg11857 (view) Author: brodie Date: 2010-02-23.19:38:29
One potential patch posted to mercurial-devel: 
http://selenic.com/pipermail/mercurial-devel/2010-February/019139.html
msg11831 (view) Author: cdadhemar Date: 2010-02-21.14:12:46
Try with the following set of extensions and alias enabled in your .hgrc :

[extensions]
purge =
record =
color =
rebase =
graphlog =
mq = 

[alias]
summary = summary --remote

This make the alias not working with mercurial 1.4.2 with python 2.6.2 on
Fedora 12 x86_64. Comment out the mq extension and it will work ! I have the
same issue with mercurial 1.4.3 at work (not sure about OS and Python
versions at work).

I hope it helped !
Cheers,
CH
msg11823 (view) Author: brodie Date: 2010-02-19.22:50:31
Can you make a simple test case that demonstrates this inconsistent behavior?

I'm able to trigger a similar issue, but it involves commands that are wrapped 
by extensions:

# mq extension enabled (adds flag to summary), alias doesn't apply
$ HGRCPATH= hg --config extensions.mq= --config alias.summary='summary --
remote' summary   
parent: 10512:261cc6b0f15c tip
 Merge with stable
branch: default
commit: (clean)
update: (current)
# no extensions enabled, alias applies
$ HGRCPATH= hg --config alias.summary='summary --remote' summary
parent: 10512:261cc6b0f15c tip
 Merge with stable
branch: default
commit: (clean)
update: (current)
remote: 1 or more incoming

I'm unable to reproduce the issue just by changing the number of commands in 
the cmdtable.
msg11822 (view) Author: cdadhemar Date: 2010-02-19.18:26:28
Hello !

I am using mercurial 1.4.3.

Reading .hgrc documentation I found that using [defaults] is deprecated in
favor of using [alias]. So I started to use [alias] and I found the
following issue :

When overriding an existing command to add a default behavior (summary ->
summary --remote), we have no warranty that the alias will be taken into
account instead of the original command. 

At http://selenic.com/repo/hg/file/261cc6b0f15c/mercurial/cmdutil.py#l27 :

We loop on the commands.table.keys() so we have no warranty on the ordering
here (python dict() does not provide such warranty). In the case of an alias
which overrides an existing command we keep in the 'choice' map the latest
key that matches. If we are lucky we have the alias we defined, if not we
have the original command ...

The lucky factor depends on how many extensions have been enabled in .hgrc,
and thus how big is the commands.table before adding the aliases
(http://selenic.com/repo/hg/file/261cc6b0f15c/mercurial/dispatch.py#l391) :
I guess that in some cases the code :

commands.table.update(cmdtable)  

at http://selenic.com/repo/hg/file/261cc6b0f15c/mercurial/dispatch.py#l386
change the ordering of the keys in the commands.table dict ...


Thus the only safe option to override a command defaults options is then to
use the [defaults] section in hgrc. Why the usage of the [defaults] section
is deprecated in the documentation ? Does this bug make [defaults] no more
deprecated ?

This bug is of course not critical at all (even .hgrc documentation warn me
about overriding a command using alias ...) and difficult to reproduce. But
still this is a bug...

Thank you very much for your help !
Cheers,
Charles-Henri
History
Date User Action Args
2010-10-09 15:36:15mpmsetstatus: testing -> resolved
nosy: mpm, ThomasAH, tonfa, mg, djc, brodie, cdadhemar, cmisare, hgbot
2010-08-26 15:40:20brodiesetnosy: mpm, ThomasAH, tonfa, mg, djc, brodie, cdadhemar, cmisare, hgbot
messages: + msg13493
2010-08-26 11:00:09hgbotsetnosy: + hgbot
messages: + msg13492
2010-08-26 10:30:41ThomasAHsetstatus: chatting -> testing
nosy: + mpm, ThomasAH
messages: + msg13491
assignedto: brodie -> mpm
2010-03-06 10:13:37cmisaresetfiles: + aliases-test.sh
nosy: + cmisare
messages: + msg11942
2010-02-23 19:38:29brodiesetassignedto: brodie
messages: + msg11857
nosy: tonfa, mg, djc, brodie, cdadhemar
2010-02-21 14:12:46cdadhemarsetnosy: tonfa, mg, djc, brodie, cdadhemar
messages: + msg11831
2010-02-20 13:06:15mgsetnosy: + mg
2010-02-19 22:50:31brodiesetstatus: unread -> chatting
nosy: + brodie
messages: + msg11823
2010-02-19 22:17:19tonfasetnosy: + tonfa
2010-02-19 21:28:17djcsetnosy: + djc
2010-02-19 18:26:28cdadhemarcreate