Issue2017

Title Better way to call commands from commands.py with correct default (optional arguments)
Priority wish Status resolved
Superseder Nosy List kiilerix, tonfa, vincenthillenbrink
Assigned To Topics

Created on 2010-02-03.09:50:30 by vincenthillenbrink, last changed 2010-02-23.08:17:13 by vincenthillenbrink.

Files
File name Uploaded Type Edit Remove
bug.py vincenthillenbrink, 2010-02-03.09:50:30 application/octet-stream
Messages
msg11847 (view) Author: vincenthillenbrink Date: 2010-02-23.08:17:13
Closed. Thanks to kiilerix and tonfa for the quick responses and apologies
for not closing this sooner, but I ran into some other issues that I haven't
had time to work on since...
msg11742 (view) Author: tonfa Date: 2010-02-15.21:21:54
Lets close this. The easy way is to call commands via dispatch.run() this 
will correctly handle default arguments.

Long term we can think about doing proper typing of arguement.
msg11578 (view) Author: tonfa Date: 2010-02-03.11:05:55
If you use prefix = '', it will work.

I guess the long term fix is to link the defaults used in the command table 
and the defaults given by opts.get().

I.e. if the default for prefix is '', opts.get('prefix') should return ''.
msg11577 (view) Author: kiilerix Date: 2010-02-03.11:05:07
Mercurial uses a kind of duck typing for these 'commands' functions. Some
opts values are mandatory. There is no explicit documentation of what is
mandatory, but if it is mandatory then it is mandatory.

The archive command gives 'prefix' a default value of "". You should do the
same.
msg11575 (view) Author: vincenthillenbrink Date: 2010-02-03.09:50:30
I installed 1.4.2 on Mac OS via macports.

However, I cannot seem to make commands.archive() work for 'files' mode,
whereas the command 'hg archive' and archival.archive() both work fine.

It's just that these lines of code from commands.py (lines 171-179) seem
very puzzling:

    prefix = opts.get('prefix') # This could very well be None
    if dest == '-': # Not relevant for 'files' mode
        # ...deleted...
    # prefix can still be None (moreover, it *should* be none for 'files')
    # Now, make_filename() will do a len(None) in a moment
    prefix = cmdutil.make_filename(repo, prefix, node)
    # So we don't get here
    archival.archive(repo, dest, node, kind, not opts.get('no_decode'),
                     matchfn, prefix)

Apologies if this is my mistake.
History
Date User Action Args
2010-02-23 08:17:13vincenthillenbrinksetstatus: deferred -> resolved
messages: + msg11847
2010-02-15 21:21:54tonfasetpriority: bug -> wish
status: chatting -> deferred
messages: + msg11742
title: Unneccessary call to cmdutil.make_filename() crashes commands.archive() in 'files' mode -> Better way to call commands from commands.py with correct default (optional arguments)
2010-02-03 11:05:55tonfasetnosy: + tonfa
messages: + msg11578
2010-02-03 11:05:07kiilerixsetstatus: unread -> chatting
nosy: + kiilerix
messages: + msg11577
2010-02-03 09:50:30vincenthillenbrinkcreate