| Submitter | via Mercurial-devel |
|---|---|
| Date | March 24, 2017, 7:32 a.m. |
| Message ID | <60b3222e01f96f91ece7.1490340720@rdamazio.mtv.corp.google.com> |
| Download | mbox | patch |
| Permalink | /patch/19625/ |
| State | Changes Requested |
| Headers | show |
Comments
On Fri, 24 Mar 2017 00:32:00 -0700, Rodrigo Damazio Bovendorp via Mercurial-devel wrote: > # HG changeset patch > # User Rodrigo Damazio <rdamazio@google.com> > # Date 1490340211 25200 > # Fri Mar 24 00:23:31 2017 -0700 > # Node ID 60b3222e01f96f91ece7eda9681a89bf3bb930a6 > # Parent df82f375fa005b9c71b463182e6b54aa47fa5999 > fancyopts: making config defaults actually override defaults Sorry for late review. This looks generally good to me. I have a few comments inline. > c = list(entry[1]) > + > + # override new-style defaults from config file by actually changing the > + # option defaults. > + for idx, opt in enumerate(c): > + optname = opt[1] > + shortname = opt[0] > + defaulttype = type(opt[2]) > + rawdefault = ( > + ui.config("commands", "%s.default.%s" % (cmd, optname)) or > + ui.config("commands", "%s.default.%s" % (cmd, shortname))) I prefer not supporting "default.<shortname>" because shortname doesn't look like a config name. And if we do support it, an empty longname value should supersede a shortname: ui.config(<longname>, default=ui.config(<shortname>)) > + if rawdefault: > + # parse the new default using the type of the original default. > + default = fancyopts.parsevalue(optname, rawdefault, defaulttype, > + util.parsebool(rawdefault)) Can we use ui.configbool/list/int() instead of parsevalue() ? That will provide a slightly better error message.
FYI I'm getting back to this old patch, but I'll send it through phabricator this time. On Sat, Apr 1, 2017 at 11:07 AM, Yuya Nishihara <yuya@tcha.org> wrote: > On Fri, 24 Mar 2017 00:32:00 -0700, Rodrigo Damazio Bovendorp via > Mercurial-devel wrote: > > # HG changeset patch > > # User Rodrigo Damazio <rdamazio@google.com> > > # Date 1490340211 25200 > > # Fri Mar 24 00:23:31 2017 -0700 > > # Node ID 60b3222e01f96f91ece7eda9681a89bf3bb930a6 > > # Parent df82f375fa005b9c71b463182e6b54aa47fa5999 > > fancyopts: making config defaults actually override defaults > > Sorry for late review. > > This looks generally good to me. I have a few comments inline. > > > c = list(entry[1]) > > + > > + # override new-style defaults from config file by actually > changing the > > + # option defaults. > > + for idx, opt in enumerate(c): > > + optname = opt[1] > > + shortname = opt[0] > > + defaulttype = type(opt[2]) > > + rawdefault = ( > > + ui.config("commands", "%s.default.%s" % (cmd, optname)) > or > > + ui.config("commands", "%s.default.%s" % (cmd, > shortname))) > > I prefer not supporting "default.<shortname>" because shortname doesn't > look > like a config name. And if we do support it, an empty longname value should > supersede a shortname: > > ui.config(<longname>, default=ui.config(<shortname>)) > > > + if rawdefault: > > + # parse the new default using the type of the original > default. > > + default = fancyopts.parsevalue(optname, rawdefault, > defaulttype, > > + > util.parsebool(rawdefault)) > > Can we use ui.configbool/list/int() instead of parsevalue() ? That will > provide a slightly better error message. >
Patch
diff -r df82f375fa00 -r 60b3222e01f9 mercurial/dispatch.py --- a/mercurial/dispatch.py Tue Mar 21 23:30:13 2017 +0100 +++ b/mercurial/dispatch.py Fri Mar 24 00:23:31 2017 -0700 @@ -473,11 +473,30 @@ ui.configbool("ui", "strict")) cmd = aliases[0] args = aliasargs(entry[0], args) + + # override old-style defaults from config file by prepending + # command-line flags defaults = ui.config("defaults", cmd) if defaults: args = pycompat.maplist( util.expandpath, pycompat.shlexsplit(defaults)) + args c = list(entry[1]) + + # override new-style defaults from config file by actually changing the + # option defaults. + for idx, opt in enumerate(c): + optname = opt[1] + shortname = opt[0] + defaulttype = type(opt[2]) + rawdefault = ( + ui.config("commands", "%s.default.%s" % (cmd, optname)) or + ui.config("commands", "%s.default.%s" % (cmd, shortname))) + if rawdefault: + # parse the new default using the type of the original default. + default = fancyopts.parsevalue(optname, rawdefault, defaulttype, + util.parsebool(rawdefault)) + c[idx] = (opt[0], opt[1], default, opt[3]) + else: cmd = None c = [] diff -r df82f375fa00 -r 60b3222e01f9 mercurial/fancyopts.py --- a/mercurial/fancyopts.py Tue Mar 21 23:30:13 2017 +0100 +++ b/mercurial/fancyopts.py Fri Mar 24 00:23:31 2017 -0700 @@ -142,18 +142,27 @@ t = type(obj) if callable(obj): state[name] = defmap[name](val) - elif t is type(1): - try: - state[name] = int(val) - except ValueError: - raise error.Abort(_('invalid value %r for option %s, ' - 'expected int') % (val, opt)) - elif t is type(''): - state[name] = val elif t is type([]): state[name].append(val) - elif t is type(None) or t is type(False): - state[name] = boolval + else: + # non-callable single value. + state[name] = parsevalue(name, val, t, boolval) # return unparsed args return args + +def parsevalue(name, val, typ, boolval=True): + if typ is type(1): + try: + return int(val) + except ValueError: + raise error.Abort(_('invalid value %r for option %s, ' + 'expected int') % (val, name)) + elif typ is type(''): + return val + elif (typ is type(None) or typ is type(False)) and ( + type(boolval) is type(False)): + return boolval + else: + raise error.Abort(_('invalid value %r for option %s, ' + 'unknown type') % (val, name)) diff -r df82f375fa00 -r 60b3222e01f9 tests/test-dispatch.t --- a/tests/test-dispatch.t Tue Mar 21 23:30:13 2017 +0100 +++ b/tests/test-dispatch.t Fri Mar 24 00:23:31 2017 -0700 @@ -8,8 +8,10 @@ $ hg -v log -v x $ echo a > a + $ echo b > b $ hg ci -Ama adding a + adding b Missing arg: @@ -34,6 +36,7 @@ $ hg cat a a + $ cp $HGRCPATH hgrc.bak $ cat >> $HGRCPATH <<EOF > [defaults] > cat = -r null @@ -41,6 +44,54 @@ $ hg cat a a: no such file in rev 000000000000 [1] + $ cp -f hgrc.bak $HGRCPATH + +new-style [commands] defaults and overrides + + $ hg cat a + a + $ cat >> $HGRCPATH <<EOF + > [commands] + > cat.default.r = null + > EOF + $ hg cat a + a: no such file in rev 000000000000 + [1] + + $ cp -f hgrc.bak $HGRCPATH + $ cat >> $HGRCPATH <<EOF + > [commands] + > cat.default.rev = null + > EOF + $ hg cat a + a: no such file in rev 000000000000 + [1] + + $ mv -f hgrc.bak $HGRCPATH + $ echo foo >> a + $ hg rm b + $ echo bar > c + $ hg add c + $ hg status + M a + A c + R b + $ cat >> $HGRCPATH <<EOF + > [commands] + > status.default.removed = 1 + > EOF + $ hg status + R b + $ hg status --modified + M a + R b + $ hg status --modified --no-removed + M a + $ hg status --no-removed + M a + A c + R b + $ hg revert a b c $ cd "$TESTTMP"