[PATCH] fancyopts: making config defaults actually override defaults

Rodrigo Damazio rdamazio at google.com
Fri Mar 24 03:15:36 EDT 2017


On Thu, Mar 23, 2017 at 3:07 AM, Ryan McElroy <rm at 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 at google.com>
>> wrote:
>>
>>> On Wed, Mar 22, 2017 at 3:50 AM, Ryan McElroy <rm at 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 at mercurial-scm.org with me, because it probably first
>> received one from there from me. And the reason it is from
>> mercurial-devel at 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 at 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 at 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170324/3c43f487/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4847 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170324/3c43f487/attachment.bin>


More information about the Mercurial-devel mailing list