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 <y...@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 <rdama...@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. >
smime.p7s
Description: S/MIME Cryptographic Signature
_______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel