| Submitter | Christophe de Vienne |
|---|---|
| Date | Sept. 1, 2017, 7:44 a.m. |
| Message ID | <06bc5adebc48cfb657cd.1504251864@needle> |
| Download | mbox | patch |
| Permalink | /patch/23588/ |
| State | Changes Requested |
| Headers | show |
Comments
On Fri, 01 Sep 2017 09:44:24 +0200, Christophe de Vienne wrote: > # HG changeset patch > # User Christophe de Vienne <christophe@cdevienne.info> > # Date 1504023891 -7200 > # Tue Aug 29 18:24:51 2017 +0200 > # Node ID 06bc5adebc48cfb657cdefda41704a96f686fae7 > # Parent b2eb0aa445cbe7cadc8b7691c6266908adfc5057 > command options: abort on unicode defaults > > If the default value of an option is a unicode string (something > than happen easily when using a 'from __future__ import unicode_literals'), > any value passed on the command line will be ignored because the fancyopts > module only checks for byte strings and not unicode strings. > > Changing fancyopts behavior is easy but would make assumptions on how > the python3 port should be done, which is outside the scope of this patch. > > The chosen approach is to abort when a unicode default value is detected, > with a hint for the developer. > > diff -r b2eb0aa445cb -r 06bc5adebc48 mercurial/dispatch.py > --- a/mercurial/dispatch.py Tue Aug 22 21:21:43 2017 -0400 > +++ b/mercurial/dispatch.py Tue Aug 29 18:24:51 2017 +0200 > @@ -558,6 +558,14 @@ > args = pycompat.maplist( > util.expandpath, pycompat.shlexsplit(defaults)) + args > c = list(entry[1]) > + for option in entry[1]: > + default = option[2] Maybe this can be checked at fancyopts where the options table is processed. Alternatively, it could be added to extensions._validatecmdtable(). > + if not pycompat.ispy3 and isinstance(default, type(u'')): This can be enabled on Python 3. unicode strings should be banned. > + raise error.Abort( error.ProgrammingError? > + "A unicode default value (%s) was passed to the " > + "%s.%s option" % (repr(default), cmd, option[1]), > + "You should change the %s.%s default value to a " > + "non-unicode string" % (cmd, option[1]))
Patch
diff -r b2eb0aa445cb -r 06bc5adebc48 mercurial/dispatch.py --- a/mercurial/dispatch.py Tue Aug 22 21:21:43 2017 -0400 +++ b/mercurial/dispatch.py Tue Aug 29 18:24:51 2017 +0200 @@ -558,6 +558,14 @@ args = pycompat.maplist( util.expandpath, pycompat.shlexsplit(defaults)) + args c = list(entry[1]) + for option in entry[1]: + default = option[2] + if not pycompat.ispy3 and isinstance(default, type(u'')): + raise error.Abort( + "A unicode default value (%s) was passed to the " + "%s.%s option" % (repr(default), cmd, option[1]), + "You should change the %s.%s default value to a " + "non-unicode string" % (cmd, option[1])) else: cmd = None c = [] diff -r b2eb0aa445cb -r 06bc5adebc48 tests/test-dispatch.t --- a/tests/test-dispatch.t Tue Aug 22 21:21:43 2017 -0400 +++ b/tests/test-dispatch.t Tue Aug 29 18:24:51 2017 +0200 @@ -60,3 +60,23 @@ [255] #endif + +Test abort on command options with unicode default value + + $ hg init $TESTTMP/opt-unicode-default + + $ cat > $TESTTMP/test_unicode_default_value.py << EOF + > from mercurial import registrar + > cmdtable = {} + > command = registrar.command(cmdtable) + > @command('ext', [('', 'opt', u'value', u'help')], 'ext [OPTIONS]') + > def ext(*args, **opts): + > print(opts['opt']) + > EOF + $ cat > $TESTTMP/opt-unicode-default/.hg/hgrc << EOF + > [extensions] + > test_unicode_default_value = $TESTTMP/test_unicode_default_value.py + > EOF + $ hg -R $TESTTMP/opt-unicode-default ext + abort: ("A unicode default value (u'value') was passed to the ext.opt option", 'You should change the ext.opt default value to a non-unicode string') + [255]