Patchwork fancyopts: making config defaults actually override defaults

login
register
mail settings
Submitter via Mercurial-devel
Date March 12, 2017, 2:03 a.m.
Message ID <8c833b81a994e2d3304c.1489284210@rdamazio.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/19133/
State Changes Requested
Headers show

Comments

via Mercurial-devel - March 12, 2017, 2:03 a.m.
# HG changeset patch
# User Rodrigo Damazio <rdamazio@google.com>
# Date 1489274373 28800
#      Sat Mar 11 15:19:33 2017 -0800
# Node ID 8c833b81a994e2d3304c3b06793f536355528aab
# Parent  62939e0148f170b67ca8c7374f36c413b67fd387
fancyopts: making config defaults actually override defaults

This introduces the new defaults format "command.option" which directly
overrides the default of the option, instead of prepending the command-line
option.
David Soria Parra - March 14, 2017, 7:50 p.m.
On Sat, Mar 11, 2017 at 06:03:30PM -0800, Rodrigo Damazio Bovendorp via Mercurial-devel wrote:
> # HG changeset patch
> # User Rodrigo Damazio <rdamazio@google.com>
> # Date 1489274373 28800
> #      Sat Mar 11 15:19:33 2017 -0800
> # Node ID 8c833b81a994e2d3304c3b06793f536355528aab
> # Parent  62939e0148f170b67ca8c7374f36c413b67fd387
> fancyopts: making config defaults actually override defaults
> 

Overall this LGTM, and clearly makes defaults much better :).  My concern
is that we are encouraging the use of defaults again, while they are
deprecated. Defaults have inherent problems that they overwrite arguments
which might be mutable exclusive with others (e.g. --graph in incoming
and outgoing), or lead to undesired behavior if it's set by an admin. an exmaple
is if you would specifiy defaults.update.check=True, the user will not find an
--no-check option in the help message or anywhere else. This is not a problem if
we assume defaults are alway set by the user and he knows about them.

> diff -r 62939e0148f1 -r 8c833b81a994 mercurial/fancyopts.py
> --- a/mercurial/fancyopts.py	Wed Mar 08 18:11:41 2017 -0500
> +++ b/mercurial/fancyopts.py	Sat Mar 11 15:19:33 2017 -0800
> @@ -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):
we have parsebool in util, maybe it's a good idea to put a more generic version
there?
via Mercurial-devel - March 14, 2017, 10:16 p.m.
On Tue, Mar 14, 2017 at 12:50 PM, David Soria Parra <
dsp@experimentalworks.net> wrote:

> On Sat, Mar 11, 2017 at 06:03:30PM -0800, Rodrigo Damazio Bovendorp via
> Mercurial-devel wrote:
> > # HG changeset patch
> > # User Rodrigo Damazio <rdamazio@google.com>
> > # Date 1489274373 28800
> > #      Sat Mar 11 15:19:33 2017 -0800
> > # Node ID 8c833b81a994e2d3304c3b06793f536355528aab
> > # Parent  62939e0148f170b67ca8c7374f36c413b67fd387
> > fancyopts: making config defaults actually override defaults
> >
>
> Overall this LGTM, and clearly makes defaults much better :).  My concern
> is that we are encouraging the use of defaults again, while they are
> deprecated. Defaults have inherent problems that they overwrite arguments
> which might be mutable exclusive with others (e.g. --graph in incoming
> and outgoing), or lead to undesired behavior if it's set by an admin. an
> exmaple
> is if you would specifiy defaults.update.check=True, the user will not
> find an
> --no-check option in the help message or anywhere else. This is not a
> problem if
> we assume defaults are alway set by the user and he knows about them.
>

Thanks for the review.

Yes, we discussed the update --check case specifically during Sprint:
https://public.etherpad-mozilla.org/p/sprint-hg4.2
(search for "Flags and defaults breakout")

The conclusion was that this gains us the ability to do proper single-flag
overrides, which is good, but doesn't solve all the issues. There are other
changes we also want to make flags and defaults useful again:
- make the passed-in flag values not be simple primitive types, but rather
enhance them with addition information about where the value is coming
from, so commands like update can decide that an explicit --clean overrides
a system default of --check, and should only fail if both come from the
same level
- we want to add a --no- counterflag for every flag, not just booleans, as
a way to unset it (useful for revision-specifying flags for instance)
- we want to add environment variables to the stack of overrides along with
the different levels of config files and command-line arguments
- we want to try making all positional arguments map to flags (e.g. "hg
update 123" would be equivalent to "hg update -r 123" by making 123 be
passed to the command as the -r flag, also allowing config overrides for
those)
- we want to investigate why we support callables in flag defaults, and
remove support if that's not useful to anyone
Ryan McElroy - March 22, 2017, 10:50 a.m.
Rodrigo: for some reason, patchwork thinks you are Martin. Any idea why? 
https://patchwork.mercurial-scm.org/patch/19133/


On 3/14/17 10:16 PM, Rodrigo Damazio via Mercurial-devel wrote:
> On Tue, Mar 14, 2017 at 12:50 PM, David Soria Parra 
> <dsp@experimentalworks.net <mailto:dsp@experimentalworks.net>> wrote:
>
>     On Sat, Mar 11, 2017 at 06:03:30PM -0800, Rodrigo Damazio
>     Bovendorp via Mercurial-devel wrote:
>     > # HG changeset patch
>     > # User Rodrigo Damazio <rdamazio@google.com
>     <mailto:rdamazio@google.com>>
>     > # Date 1489274373 28800
>     > #      Sat Mar 11 15:19:33 2017 -0800 <tel:33%202017%20-0800>
>     > # Node ID 8c833b81a994e2d3304c3b06793f536355528aab
>     > # Parent  62939e0148f170b67ca8c7374f36c413b67fd387
>     > fancyopts: making config defaults actually override defaults
>     >
>
>     Overall this LGTM, and clearly makes defaults much better :).  My
>     concern
>     is that we are encouraging the use of defaults again, while they are
>     deprecated. Defaults have inherent problems that they overwrite
>     arguments
>     which might be mutable exclusive with others (e.g. --graph in incoming
>     and outgoing), or lead to undesired behavior if it's set by an
>     admin. an exmaple
>     is if you would specifiy defaults.update.check=True, the user will
>     not find an
>     --no-check option in the help message or anywhere else. This is
>     not a problem if
>     we assume defaults are alway set by the user and he knows about them.
>
>
> Thanks for the review.
>
> Yes, we discussed the update --check case specifically during Sprint:
> https://public.etherpad-mozilla.org/p/sprint-hg4.2
> (search for "Flags and defaults breakout")

Note that I copied the notes over the the wiki for posterity: 
https://www.mercurial-scm.org/wiki/4.2sprint/Notes

If people do some cleanup passes and categorization, that would be 
useful. I may contribute here at some point as well.

>
> The conclusion was that this gains us the ability to do proper 
> single-flag overrides, which is good, but doesn't solve all the 
> issues. There are other changes we also want to make flags and 
> defaults useful again:
> - make the passed-in flag values not be simple primitive types, but 
> rather enhance them with addition information about where the value is 
> coming from, so commands like update can decide that an explicit 
> --clean overrides a system default of --check, and should only fail if 
> both come from the same level
> - we want to add a --no- counterflag for every flag, not just 
> booleans, as a way to unset it (useful for revision-specifying flags 
> for instance)
> - we want to add environment variables to the stack of overrides along 
> with the different levels of config files and command-line arguments
> - we want to try making all positional arguments map to flags (e.g. 
> "hg update 123" would be equivalent to "hg update -r 123" by making 
> 123 be passed to the command as the -r flag, also allowing config 
> overrides for those)
> - we want to investigate why we support callables in flag defaults, 
> and remove support if that's not useful to anyone
>
>

While I like this initial direction, I think this needs to be turned 
into a series of smaller steps, and we need more discussion around 
whether we will revive the defaults section or keep it deprecated. I'll 
drop this from patchwork for now while we discuss.
via Mercurial-devel - March 22, 2017, 5:14 p.m.
On Wed, Mar 22, 2017 at 3:50 AM, Ryan McElroy <rm@fb.com> wrote:

> Rodrigo: for some reason, patchwork thinks you are Martin. Any idea why?
> https://patchwork.mercurial-scm.org/patch/19133/
>
No idea, but I'm using a script he created for mailing patches directly to
gmail's backend servers (to avoid reformatting) - Martin, any chance you
hardcoded your own address there? :)

On 3/14/17 10:16 PM, Rodrigo Damazio via Mercurial-devel wrote:
>
> On Tue, Mar 14, 2017 at 12:50 PM, David Soria Parra <
> dsp@experimentalworks.net> wrote:
>
>> On Sat, Mar 11, 2017 at 06:03:30PM -0800, Rodrigo Damazio Bovendorp via
>> Mercurial-devel wrote:
>> > # HG changeset patch
>> > # User Rodrigo Damazio <rdamazio@google.com>
>> > # Date 1489274373 28800
>> > #      Sat Mar 11 15:19:33 2017 -0800
>> > # Node ID 8c833b81a994e2d3304c3b06793f536355528aab
>> > # Parent  62939e0148f170b67ca8c7374f36c413b67fd387
>> > fancyopts: making config defaults actually override defaults
>> >
>>
>> Overall this LGTM, and clearly makes defaults much better :).  My concern
>> is that we are encouraging the use of defaults again, while they are
>> deprecated. Defaults have inherent problems that they overwrite arguments
>> which might be mutable exclusive with others (e.g. --graph in incoming
>> and outgoing), or lead to undesired behavior if it's set by an admin. an
>> exmaple
>> is if you would specifiy defaults.update.check=True, the user will not
>> find an
>> --no-check option in the help message or anywhere else. This is not a
>> problem if
>> we assume defaults are alway set by the user and he knows about them.
>>
>
> Thanks for the review.
>
> Yes, we discussed the update --check case specifically during Sprint:
> https://public.etherpad-mozilla.org/p/sprint-hg4.2
> (search for "Flags and defaults breakout")
>
>
> Note that I copied the notes over the the wiki for posterity:
> https://www.mercurial-scm.org/wiki/4.2sprint/Notes
>
> If people do some cleanup passes and categorization, that would be useful.
> I may contribute here at some point as well.
>
>
> The conclusion was that this gains us the ability to do proper single-flag
> overrides, which is good, but doesn't solve all the issues. There are other
> changes we also want to make flags and defaults useful again:
> - make the passed-in flag values not be simple primitive types, but rather
> enhance them with addition information about where the value is coming
> from, so commands like update can decide that an explicit --clean overrides
> a system default of --check, and should only fail if both come from the
> same level
> - we want to add a --no- counterflag for every flag, not just booleans, as
> a way to unset it (useful for revision-specifying flags for instance)
> - we want to add environment variables to the stack of overrides along
> with the different levels of config files and command-line arguments
> - we want to try making all positional arguments map to flags (e.g. "hg
> update 123" would be equivalent to "hg update -r 123" by making 123 be
> passed to the command as the -r flag, also allowing config overrides for
> those)
> - we want to investigate why we support callables in flag defaults, and
> remove support if that's not useful to anyone
>
>
>
> While I like this initial direction, I think this needs to be turned into
> a series of smaller steps, and we need more discussion around whether we
> will revive the defaults section or keep it deprecated. I'll drop this from
> patchwork for now while we discuss.
>

No problem. How/where/when/what would you like to discuss?
via Mercurial-devel - March 22, 2017, 6:51 p.m.
On Wed, Mar 22, 2017 at 10:14 AM, Rodrigo Damazio <rdamazio@google.com> wrote:
> On Wed, Mar 22, 2017 at 3:50 AM, Ryan McElroy <rm@fb.com> wrote:
>>
>> Rodrigo: for some reason, patchwork thinks you are Martin. Any idea why?
>> https://patchwork.mercurial-scm.org/patch/19133/
>
> No idea, but I'm using a script he created for mailing patches directly to
> gmail's backend servers (to avoid reformatting) - Martin, any chance you
> hardcoded your own address there? :)

Nope. IIUC, patchwork has decided to associate any email from
mercurial-devel@mercurial-scm.org with me, because it probably first
received one from there from me. And the reason it is from
mercurial-devel@mercurial-scm.org in the first place is because we
(Google) set the DMARC/SPF/whatever headers to prevent spoofing of
@google.com addresses, so the mailing list has no choice but to
rewrite it as coming from the mailing list itself. I may very well
have misunderstood how that works, but hopefully you get the idea
anyway.

>
>> On 3/14/17 10:16 PM, Rodrigo Damazio via Mercurial-devel wrote:
>>
>> On Tue, Mar 14, 2017 at 12:50 PM, David Soria Parra
>> <dsp@experimentalworks.net> wrote:
>>>
>>> On Sat, Mar 11, 2017 at 06:03:30PM -0800, Rodrigo Damazio Bovendorp via
>>> Mercurial-devel wrote:
>>> > # HG changeset patch
>>> > # User Rodrigo Damazio <rdamazio@google.com>
>>> > # Date 1489274373 28800
>>> > #      Sat Mar 11 15:19:33 2017 -0800
>>> > # Node ID 8c833b81a994e2d3304c3b06793f536355528aab
>>> > # Parent  62939e0148f170b67ca8c7374f36c413b67fd387
>>> > fancyopts: making config defaults actually override defaults
>>> >
>>>
>>> Overall this LGTM, and clearly makes defaults much better :).  My concern
>>> is that we are encouraging the use of defaults again, while they are
>>> deprecated. Defaults have inherent problems that they overwrite arguments
>>> which might be mutable exclusive with others (e.g. --graph in incoming
>>> and outgoing), or lead to undesired behavior if it's set by an admin. an
>>> exmaple
>>> is if you would specifiy defaults.update.check=True, the user will not
>>> find an
>>> --no-check option in the help message or anywhere else. This is not a
>>> problem if
>>> we assume defaults are alway set by the user and he knows about them.
>>
>>
>> Thanks for the review.
>>
>> Yes, we discussed the update --check case specifically during Sprint:
>> https://public.etherpad-mozilla.org/p/sprint-hg4.2
>> (search for "Flags and defaults breakout")
>>
>>
>> Note that I copied the notes over the the wiki for posterity:
>> https://www.mercurial-scm.org/wiki/4.2sprint/Notes
>>
>> If people do some cleanup passes and categorization, that would be useful.
>> I may contribute here at some point as well.
>>
>>
>> The conclusion was that this gains us the ability to do proper single-flag
>> overrides, which is good, but doesn't solve all the issues. There are other
>> changes we also want to make flags and defaults useful again:
>> - make the passed-in flag values not be simple primitive types, but rather
>> enhance them with addition information about where the value is coming from,
>> so commands like update can decide that an explicit --clean overrides a
>> system default of --check, and should only fail if both come from the same
>> level
>> - we want to add a --no- counterflag for every flag, not just booleans, as
>> a way to unset it (useful for revision-specifying flags for instance)
>> - we want to add environment variables to the stack of overrides along
>> with the different levels of config files and command-line arguments
>> - we want to try making all positional arguments map to flags (e.g. "hg
>> update 123" would be equivalent to "hg update -r 123" by making 123 be
>> passed to the command as the -r flag, also allowing config overrides for
>> those)
>> - we want to investigate why we support callables in flag defaults, and
>> remove support if that's not useful to anyone
>>
>>
>>
>> While I like this initial direction, I think this needs to be turned into
>> a series of smaller steps, and we need more discussion around whether we
>> will revive the defaults section or keep it deprecated. I'll drop this from
>> patchwork for now while we discuss.
>
>
> No problem. How/where/when/what would you like to discuss?
>
Ryan McElroy - March 23, 2017, 10:07 a.m.
On 3/22/17 6:51 PM, Martin von Zweigbergk wrote:
> On Wed, Mar 22, 2017 at 10:14 AM, Rodrigo Damazio <rdamazio@google.com> wrote:
>> On Wed, Mar 22, 2017 at 3:50 AM, Ryan McElroy <rm@fb.com> wrote:
>>> Rodrigo: for some reason, patchwork thinks you are Martin. Any idea why?
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.mercurial-2Dscm.org_patch_19133_&d=DwIBaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=Jw8rundaE7TbmqBYd1txIQ&m=qgti-7e8saOlUI0k7ljLZ6HAh4SsKVlgoONeD60WZRA&s=pKbjIO05blt_27t3IcMhFfzDFctfA-bMMaXrmGmiXJM&e=
>> No idea, but I'm using a script he created for mailing patches directly to
>> gmail's backend servers (to avoid reformatting) - Martin, any chance you
>> hardcoded your own address there? :)
> Nope. IIUC, patchwork has decided to associate any email from
> mercurial-devel@mercurial-scm.org with me, because it probably first
> received one from there from me. And the reason it is from
> mercurial-devel@mercurial-scm.org in the first place is because we
> (Google) set the DMARC/SPF/whatever headers to prevent spoofing of
> @google.com addresses, so the mailing list has no choice but to
> rewrite it as coming from the mailing list itself. I may very well
> have misunderstood how that works, but hopefully you get the idea
> anyway.
>
>>> On 3/14/17 10:16 PM, Rodrigo Damazio via Mercurial-devel wrote:
>>>
>>> On Tue, Mar 14, 2017 at 12:50 PM, David Soria Parra
>>> <dsp@experimentalworks.net> wrote:
>>>> On Sat, Mar 11, 2017 at 06:03:30PM -0800, Rodrigo Damazio Bovendorp via
>>>> Mercurial-devel wrote:
>>>>> # HG changeset patch
>>>>> # User Rodrigo Damazio <rdamazio@google.com>
>>>>> # Date 1489274373 28800
>>>>> #      Sat Mar 11 15:19:33 2017 -0800
>>>>> # Node ID 8c833b81a994e2d3304c3b06793f536355528aab
>>>>> # Parent  62939e0148f170b67ca8c7374f36c413b67fd387
>>>>> fancyopts: making config defaults actually override defaults
>>>>>
>>>> Overall this LGTM, and clearly makes defaults much better :).  My concern
>>>> is that we are encouraging the use of defaults again, while they are
>>>> deprecated. Defaults have inherent problems that they overwrite arguments
>>>> which might be mutable exclusive with others (e.g. --graph in incoming
>>>> and outgoing), or lead to undesired behavior if it's set by an admin. an
>>>> exmaple
>>>> is if you would specifiy defaults.update.check=True, the user will not
>>>> find an
>>>> --no-check option in the help message or anywhere else. This is not a
>>>> problem if
>>>> we assume defaults are alway set by the user and he knows about them.
>>>
>>> Thanks for the review.
>>>
>>> Yes, we discussed the update --check case specifically during Sprint:
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__public.etherpad-2Dmozilla.org_p_sprint-2Dhg4.2&d=DwIBaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=Jw8rundaE7TbmqBYd1txIQ&m=qgti-7e8saOlUI0k7ljLZ6HAh4SsKVlgoONeD60WZRA&s=usophD9VL71sbr6iMbVTWMVQPbQLoK4f-qe9si7v3rU&e=
>>> (search for "Flags and defaults breakout")
>>>
>>>
>>> Note that I copied the notes over the the wiki for posterity:
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_wiki_4.2sprint_Notes&d=DwIBaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=Jw8rundaE7TbmqBYd1txIQ&m=qgti-7e8saOlUI0k7ljLZ6HAh4SsKVlgoONeD60WZRA&s=5pz07una5PeOy9ugeIPoZIk4uGiP7gE5cwdweE6HcTI&e=
>>>
>>> If people do some cleanup passes and categorization, that would be useful.
>>> I may contribute here at some point as well.
>>>
>>>
>>> The conclusion was that this gains us the ability to do proper single-flag
>>> overrides, which is good, but doesn't solve all the issues. There are other
>>> changes we also want to make flags and defaults useful again:
>>> - make the passed-in flag values not be simple primitive types, but rather
>>> enhance them with addition information about where the value is coming from,
>>> so commands like update can decide that an explicit --clean overrides a
>>> system default of --check, and should only fail if both come from the same
>>> level
>>> - we want to add a --no- counterflag for every flag, not just booleans, as
>>> a way to unset it (useful for revision-specifying flags for instance)
>>> - we want to add environment variables to the stack of overrides along
>>> with the different levels of config files and command-line arguments
>>> - we want to try making all positional arguments map to flags (e.g. "hg
>>> update 123" would be equivalent to "hg update -r 123" by making 123 be
>>> passed to the command as the -r flag, also allowing config overrides for
>>> those)
>>> - we want to investigate why we support callables in flag defaults, and
>>> remove support if that's not useful to anyone
>>>
>>>
>>>
>>> While I like this initial direction, I think this needs to be turned into
>>> a series of smaller steps, and we need more discussion around whether we
>>> will revive the defaults section or keep it deprecated. I'll drop this from
>>> patchwork for now while we discuss.
>>
>> No problem. How/where/when/what would you like to discuss?
>>

I suggest we keep discussing on this thread. If there isn't enough 
opposition to "reviving" defaults, then maybe just keep this direction 
and submit as a few smaller patches.

That being said, I'm a bit opposed to un-deprecating defaults. Even if 
it makes sense for this new functionality, reusing it makes it harder to 
de-emphasize as a section, so the old behavior will remain front-and-center.

We recently added this "commands" section, though, and like defaults it 
gets ignored when HGPLAIN is on (with Martin's latest patch). I'd 
suggest that might be a better place for this functionality. Would you 
be okay with that? Are there reasons that it wouldn't make sense there?

My suggestion off the top of my head would be to do this this way:

[commands]
rebase.default.dest = DEST

That way, there's a separate "namespace" for command defaults within the 
commands section.
Augie Fackler - March 23, 2017, 3:22 p.m.
On Wed, Mar 22, 2017 at 11:51:30AM -0700, Martin von Zweigbergk via Mercurial-devel wrote:
> On Wed, Mar 22, 2017 at 10:14 AM, Rodrigo Damazio <rdamazio@google.com> wrote:
> > On Wed, Mar 22, 2017 at 3:50 AM, Ryan McElroy <rm@fb.com> wrote:
> >>
> >> Rodrigo: for some reason, patchwork thinks you are Martin. Any idea why?
> >> https://patchwork.mercurial-scm.org/patch/19133/
> >
> > No idea, but I'm using a script he created for mailing patches directly to
> > gmail's backend servers (to avoid reformatting) - Martin, any chance you
> > hardcoded your own address there? :)
>
> Nope. IIUC, patchwork has decided to associate any email from
> mercurial-devel@mercurial-scm.org with me, because it probably first
> received one from there from me. And the reason it is from
> mercurial-devel@mercurial-scm.org in the first place is because we
> (Google) set the DMARC/SPF/whatever headers to prevent spoofing of
> @google.com addresses, so the mailing list has no choice but to
> rewrite it as coming from the mailing list itself. I may very well
> have misunderstood how that works, but hopefully you get the idea
> anyway.

This matches my understanding of the collective mail transfer damage here, FWIW.
via Mercurial-devel - March 24, 2017, 7:15 a.m.
On Thu, Mar 23, 2017 at 3:07 AM, Ryan McElroy <rm@fb.com> wrote:

> On 3/22/17 6:51 PM, Martin von Zweigbergk wrote:
>
>> On Wed, Mar 22, 2017 at 10:14 AM, Rodrigo Damazio <rdamazio@google.com>
>> wrote:
>>
>>> On Wed, Mar 22, 2017 at 3:50 AM, Ryan McElroy <rm@fb.com> wrote:
>>>
>>>> Rodrigo: for some reason, patchwork thinks you are Martin. Any idea why?
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwo
>>>> rk.mercurial-2Dscm.org_patch_19133_&d=DwIBaQ&c=5VD0RTtNlTh3y
>>>> cd41b3MUw&r=Jw8rundaE7TbmqBYd1txIQ&m=qgti-7e8saOlUI0k7ljLZ6H
>>>> Ah4SsKVlgoONeD60WZRA&s=pKbjIO05blt_27t3IcMhFfzDFctfA-bMMaXrmGmiXJM&e=
>>>>
>>> No idea, but I'm using a script he created for mailing patches directly
>>> to
>>> gmail's backend servers (to avoid reformatting) - Martin, any chance you
>>> hardcoded your own address there? :)
>>>
>> Nope. IIUC, patchwork has decided to associate any email from
>> mercurial-devel@mercurial-scm.org with me, because it probably first
>> received one from there from me. And the reason it is from
>> mercurial-devel@mercurial-scm.org in the first place is because we
>> (Google) set the DMARC/SPF/whatever headers to prevent spoofing of
>> @google.com addresses, so the mailing list has no choice but to
>> rewrite it as coming from the mailing list itself. I may very well
>> have misunderstood how that works, but hopefully you get the idea
>> anyway.
>>
>> On 3/14/17 10:16 PM, Rodrigo Damazio via Mercurial-devel wrote:
>>>>
>>>> On Tue, Mar 14, 2017 at 12:50 PM, David Soria Parra
>>>> <dsp@experimentalworks.net> wrote:
>>>>
>>>>> On Sat, Mar 11, 2017 at 06:03:30PM -0800, Rodrigo Damazio Bovendorp via
>>>>> Mercurial-devel wrote:
>>>>>
>>>>>> # HG changeset patch
>>>>>> # User Rodrigo Damazio <rdamazio@google.com>
>>>>>> # Date 1489274373 28800
>>>>>> #      Sat Mar 11 15:19:33 2017 -0800
>>>>>> # Node ID 8c833b81a994e2d3304c3b06793f536355528aab
>>>>>> # Parent  62939e0148f170b67ca8c7374f36c413b67fd387
>>>>>> fancyopts: making config defaults actually override defaults
>>>>>>
>>>>>> Overall this LGTM, and clearly makes defaults much better :).  My
>>>>> concern
>>>>> is that we are encouraging the use of defaults again, while they are
>>>>> deprecated. Defaults have inherent problems that they overwrite
>>>>> arguments
>>>>> which might be mutable exclusive with others (e.g. --graph in incoming
>>>>> and outgoing), or lead to undesired behavior if it's set by an admin.
>>>>> an
>>>>> exmaple
>>>>> is if you would specifiy defaults.update.check=True, the user will not
>>>>> find an
>>>>> --no-check option in the help message or anywhere else. This is not a
>>>>> problem if
>>>>> we assume defaults are alway set by the user and he knows about them.
>>>>>
>>>>
>>>> Thanks for the review.
>>>>
>>>> Yes, we discussed the update --check case specifically during Sprint:
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__public.
>>>> etherpad-2Dmozilla.org_p_sprint-2Dhg4.2&d=DwIBaQ&c=5VD0RTtNl
>>>> Th3ycd41b3MUw&r=Jw8rundaE7TbmqBYd1txIQ&m=qgti-7e8saOlUI0k7lj
>>>> LZ6HAh4SsKVlgoONeD60WZRA&s=usophD9VL71sbr6iMbVTWMVQPbQLoK4f-
>>>> qe9si7v3rU&e=
>>>> (search for "Flags and defaults breakout")
>>>>
>>>>
>>>> Note that I copied the notes over the the wiki for posterity:
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mer
>>>> curial-2Dscm.org_wiki_4.2sprint_Notes&d=DwIBaQ&c=5VD0RTtNlTh
>>>> 3ycd41b3MUw&r=Jw8rundaE7TbmqBYd1txIQ&m=qgti-7e8saOlUI0k7ljLZ
>>>> 6HAh4SsKVlgoONeD60WZRA&s=5pz07una5PeOy9ugeIPoZIk4uGiP7gE5cwdweE6HcTI&e=
>>>>
>>>>
>>>> If people do some cleanup passes and categorization, that would be
>>>> useful.
>>>> I may contribute here at some point as well.
>>>>
>>>>
>>>> The conclusion was that this gains us the ability to do proper
>>>> single-flag
>>>> overrides, which is good, but doesn't solve all the issues. There are
>>>> other
>>>> changes we also want to make flags and defaults useful again:
>>>> - make the passed-in flag values not be simple primitive types, but
>>>> rather
>>>> enhance them with addition information about where the value is coming
>>>> from,
>>>> so commands like update can decide that an explicit --clean overrides a
>>>> system default of --check, and should only fail if both come from the
>>>> same
>>>> level
>>>> - we want to add a --no- counterflag for every flag, not just booleans,
>>>> as
>>>> a way to unset it (useful for revision-specifying flags for instance)
>>>> - we want to add environment variables to the stack of overrides along
>>>> with the different levels of config files and command-line arguments
>>>> - we want to try making all positional arguments map to flags (e.g. "hg
>>>> update 123" would be equivalent to "hg update -r 123" by making 123 be
>>>> passed to the command as the -r flag, also allowing config overrides for
>>>> those)
>>>> - we want to investigate why we support callables in flag defaults, and
>>>> remove support if that's not useful to anyone
>>>>
>>>>
>>>>
>>>> While I like this initial direction, I think this needs to be turned
>>>> into
>>>> a series of smaller steps, and we need more discussion around whether we
>>>> will revive the defaults section or keep it deprecated. I'll drop this
>>>> from
>>>> patchwork for now while we discuss.
>>>>
>>>
>>> No problem. How/where/when/what would you like to discuss?
>>>
>>>
> I suggest we keep discussing on this thread. If there isn't enough
> opposition to "reviving" defaults, then maybe just keep this direction and
> submit as a few smaller patches.
>
> That being said, I'm a bit opposed to un-deprecating defaults. Even if it
> makes sense for this new functionality, reusing it makes it harder to
> de-emphasize as a section, so the old behavior will remain front-and-center.
>
> We recently added this "commands" section, though, and like defaults it
> gets ignored when HGPLAIN is on (with Martin's latest patch). I'd suggest
> that might be a better place for this functionality. Would you be okay with
> that? Are there reasons that it wouldn't make sense there?
>
> My suggestion off the top of my head would be to do this this way:
>
> [commands]
> rebase.default.dest = DEST
>
> That way, there's a separate "namespace" for command defaults within the
> commands section.
>

That sounds fine to me if nobody else objects. I'll send an updated patch.

Patch

diff -r 62939e0148f1 -r 8c833b81a994 mercurial/dispatch.py
--- a/mercurial/dispatch.py	Wed Mar 08 18:11:41 2017 -0500
+++ b/mercurial/dispatch.py	Sat Mar 11 15:19:33 2017 -0800
@@ -470,10 +470,29 @@ 
                                          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 = map(util.expandpath, pycompat.shlexsplit(defaults)) + args
+            args = [util.expandpath(x) for x
+                    in 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("defaults", "%s.%s" % (cmd, optname)) or (
+                         ui.config("defaults", "%s.%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 62939e0148f1 -r 8c833b81a994 mercurial/fancyopts.py
--- a/mercurial/fancyopts.py	Wed Mar 08 18:11:41 2017 -0500
+++ b/mercurial/fancyopts.py	Sat Mar 11 15:19:33 2017 -0800
@@ -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 62939e0148f1 -r 8c833b81a994 tests/test-dispatch.t
--- a/tests/test-dispatch.t	Wed Mar 08 18:11:41 2017 -0500
+++ b/tests/test-dispatch.t	Sat Mar 11 15:19:33 2017 -0800
@@ -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
@@ -42,6 +45,54 @@ 
   a: no such file in rev 000000000000
   [1]
 
+new-style [defaults] and overrides
+
+  $ cp -f hgrc.bak $HGRCPATH
+  $ hg cat a
+  a
+  $ cat >> $HGRCPATH <<EOF
+  > [defaults]
+  > cat.r = null
+  > EOF
+  $ hg cat a
+  a: no such file in rev 000000000000
+  [1]
+
+  $ cp -f hgrc.bak $HGRCPATH
+  $ cat >> $HGRCPATH <<EOF
+  > [defaults]
+  > cat.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
+  > [defaults]
+  > status.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"
 
 OSError "No such file or directory" / "The system cannot find the path