[PATCH v2] fancyopts: making config defaults actually override defaults

[Yuya Nishihara]

On Fri, 24 Mar 2017 00:32:00 -0700, Rodrigo Damazio Bovendorp via Mercurial-devel wrote:
> # HG changeset patch
> # User Rodrigo Damazio <rdamazio at 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.