Re: [PATCH RFC] ui: add support for a tweakdefaults knob

[Pierre-Yves David]

On 06/20/2017 03:53 AM, Augie Fackler wrote:



On Jun 19, 2017, at 11:32 AM, Pierre-Yves David 
 wrote:



I'm not sure what is your concerns here can you elaborate? We have about
200 known config option in core. That seems a manageable number of object.


Sorry, I just felt it's overengineered. I don't have no particular concern
about speed.


As centralized config growth feature it will needs one item for each
config option anyway (alias, default, documentation, etc...).
Do you have an alternative idea in mind?


Hmm. If we want these features, perhaps configitem() function would be the
best.


I think we want these features. David Demelier (+ at least I) want the alias 
feature for ConfigConsolidationPlan. Gregory Szorc (and others including me) 
wants the documentation features. If someone has concerns about these feature 
we should probably resolves them before pushing further forward.

I agree that without the future (more complex) feature, that would be a bit 
heavy (a simple list of 3 tuples would be fine) :-)


I’ve got some nits about the API as expressed in that series, but I see overall 
merit if it gives us a centralized way to manage config options (and more 
importantly documenting them in a consistent way).

I think I’d like to leave actually registering a config knob as optional for a 
cycle to see if it gets much adoption though - feels like a lot of churn to put 
people through all at once for what is (for now) unclear benefit, especially 
when you consider how disruptive 4.3 has already been...


I agree here, I'm not planning to make registration mandatory in 4.3. In 
all cases we'll start with at least one cycle of devel-warning about 
unregistered option and we might want to keep unregistered option 
supported for a long while (with a devel warning).


A key feature to push code toward config registration will be the 
documentation management.


Cheers,

--
Pierre-Yves David
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH RFC] ui: add support for a tweakdefaults knob

[Augie Fackler]

> On Jun 19, 2017, at 11:32 AM, Pierre-Yves David 
>  wrote:
> 
>>> 
>>> I'm not sure what is your concerns here can you elaborate? We have about
>>> 200 known config option in core. That seems a manageable number of object.
>> 
>> Sorry, I just felt it's overengineered. I don't have no particular concern
>> about speed.
>> 
>>> As centralized config growth feature it will needs one item for each
>>> config option anyway (alias, default, documentation, etc...).
>>> Do you have an alternative idea in mind?
>> 
>> Hmm. If we want these features, perhaps configitem() function would be the
>> best.
> 
> I think we want these features. David Demelier (+ at least I) want the alias 
> feature for ConfigConsolidationPlan. Gregory Szorc (and others including me) 
> wants the documentation features. If someone has concerns about these feature 
> we should probably resolves them before pushing further forward.
> 
> I agree that without the future (more complex) feature, that would be a bit 
> heavy (a simple list of 3 tuples would be fine) :-)

I’ve got some nits about the API as expressed in that series, but I see overall 
merit if it gives us a centralized way to manage config options (and more 
importantly documenting them in a consistent way).

I think I’d like to leave actually registering a config knob as optional for a 
cycle to see if it gets much adoption though - feels like a lot of churn to put 
people through all at once for what is (for now) unclear benefit, especially 
when you consider how disruptive 4.3 has already been...
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH RFC] ui: add support for a tweakdefaults knob

[Pierre-Yves David]

On 06/19/2017 04:58 PM, Yuya Nishihara wrote:

On Mon, 19 Jun 2017 16:04:41 +0200, Pierre-Yves David wrote:

There have been talk about such registry of option for quite some time
(including at the 4.2 central sprint). David Demelier ping me about
this[1] again 10 days ago and found some time to poke at it yesterday.

It turned out I ended up with something that work, I've started
patchbombing it[2]. Once this is in, we can update the tweakdefault flag
to use this.

[1] as part of his quest for
https://www.mercurial-scm.org/wiki/ConfigConsolidationPlan
[2]
https://www.mercurial-scm.org/pipermail/mercurial-devel/2017-June/099866.html


These patches look a good move, though I think the config registry idea is
bloated a bit. Suppose we have tons of config items, I don't think it's good
idea to register them per item.


I'm not sure what is your concerns here can you elaborate? We have about
200 known config option in core. That seems a manageable number of object.


Sorry, I just felt it's overengineered. I don't have no particular concern
about speed.


As centralized config growth feature it will needs one item for each
config option anyway (alias, default, documentation, etc...).
Do you have an alternative idea in mind?


Hmm. If we want these features, perhaps configitem() function would be the
best.


I think we want these features. David Demelier (+ at least I) want the 
alias feature for ConfigConsolidationPlan. Gregory Szorc (and others 
including me) wants the documentation features. If someone has concerns 
about these feature we should probably resolves them before pushing 
further forward.


I agree that without the future (more complex) feature, that would be a 
bit heavy (a simple list of 3 tuples would be fine) :-)


Cheers,

--
Pierre-Yves David
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH RFC] ui: add support for a tweakdefaults knob

[Yuya Nishihara]

On Mon, 19 Jun 2017 16:04:41 +0200, Pierre-Yves David wrote:
> >> There have been talk about such registry of option for quite some time
> >> (including at the 4.2 central sprint). David Demelier ping me about
> >> this[1] again 10 days ago and found some time to poke at it yesterday.
> >>
> >> It turned out I ended up with something that work, I've started
> >> patchbombing it[2]. Once this is in, we can update the tweakdefault flag
> >> to use this.
> >>
> >> [1] as part of his quest for
> >> https://www.mercurial-scm.org/wiki/ConfigConsolidationPlan
> >> [2]
> >> https://www.mercurial-scm.org/pipermail/mercurial-devel/2017-June/099866.html
> >
> > These patches look a good move, though I think the config registry idea is
> > bloated a bit. Suppose we have tons of config items, I don't think it's good
> > idea to register them per item.
> 
> I'm not sure what is your concerns here can you elaborate? We have about 
> 200 known config option in core. That seems a manageable number of object.

Sorry, I just felt it's overengineered. I don't have no particular concern
about speed.

> As centralized config growth feature it will needs one item for each 
> config option anyway (alias, default, documentation, etc...).
> Do you have an alternative idea in mind?

Hmm. If we want these features, perhaps configitem() function would be the
best.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH RFC] ui: add support for a tweakdefaults knob

[Yuya Nishihara]

On Sun, 18 Jun 2017 21:59:28 -0400, Augie Fackler wrote:
> 
> > On Jun 17, 2017, at 01:01, Yuya Nishihara  wrote:
> > 
> >>> I have no idea how we should process values which are only set in 
> >>> untrusted
> >>> config. Using hasconfig(untrusted=True) might be a bit safer, but there 
> >>> would
> >>> still be inconsistency.
> >> 
> >> I thought untrusted was the default? Or do you just want it explicit?
> > 
> > untrusted is False by default. I think this and the repo config problems can
> > be mitigated by not setting tweaked values to _ocfg.
> > 
> >   if not tcfg.hasitem(section, name):
> >   tcfg.set(section, name, value, "")
> >   if not ucfg.hasitem(section, name):
> >   ucfg.set(section, name, value, "")
> >   fixconfig()
> 
> Oh, I see the problem now. I'm not sure how to address that. It was 
> intentional that tweakdefaults is only respected if it's a trusted config 
> entry, so all its items can be treated as trusted.

I agree it's probably okay to ignore an untrusted tweakdefaults option.

> I think your fix sounds reasonable for the setting of the config items. 
> Should I roll a v3 that moves the tweakdefaults() call to dispatch and make 
> it work this way instead of on (ab)using ui.setconfig?

Not using setconfig() sounds nice, too.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH RFC] ui: add support for a tweakdefaults knob

[David Soria Parra]

On Sat, Jun 17, 2017 at 12:14:22AM +0900, Yuya Nishihara wrote:
> On Wed, 14 Jun 2017 21:37:21 -0400, Augie Fackler wrote:
> > # HG changeset patch
> > # User Augie Fackler 
> > # Date 1497488194 14400
> > #  Wed Jun 14 20:56:34 2017 -0400
> > # Node ID 0e5ea7a86a8021d02218c35b07366ac6081ab3fb
> > # Parent  3abba5bc34546951b11b1bd3f5e5c77b90d950d1
> > ui: add support for a tweakdefaults knob
> 
> Queued this, thanks.
> 
I briefly glanced over this. I like the direciton, but prefer our initial
proposal of using ui.compat= with multiple compat knobs. I can revive my patch
for this, but I was initially holding back to wait for Jun's immutable config
work. ui.compat would allow us to allow to add multiple configuration points,
e.g. ui.compat=hg4.0, ui.compat=latest. I feel that the name `tweakdefaults` is
not perfectly suitable if this becomes a one-stop-shop for changing behavior
rather than just setting defaults.

Besides these minors things, I am hugely in favor of this.

___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH RFC] ui: add support for a tweakdefaults knob

[Augie Fackler]

> On Jun 18, 2017, at 21:59, Augie Fackler  wrote:
> 
>> 
>> On Jun 17, 2017, at 01:01, Yuya Nishihara  wrote:
>> 
 I have no idea how we should process values which are only set in untrusted
 config. Using hasconfig(untrusted=True) might be a bit safer, but there 
 would
 still be inconsistency.
>>> 
>>> I thought untrusted was the default? Or do you just want it explicit?
>> 
>> untrusted is False by default. I think this and the repo config problems can
>> be mitigated by not setting tweaked values to _ocfg.
>> 
>>  if not tcfg.hasitem(section, name):
>>  tcfg.set(section, name, value, "")
>>  if not ucfg.hasitem(section, name):
>>  ucfg.set(section, name, value, "")
>>  fixconfig()
> 
> Oh, I see the problem now. I'm not sure how to address that. It was 
> intentional that tweakdefaults is only respected if it's a trusted config 
> entry, so all its items can be treated as trusted.
> 
> I think your fix sounds reasonable for the setting of the config items. 
> Should I roll a v3 that moves the tweakdefaults() call to dispatch and make 
> it work this way instead of on (ab)using ui.setconfig?

I just noticed the tweakdefaults patch is pretty far back in the stack at this 
point. I can do a followup if the items I've mentioned here sound good 
(tomorrow, since it's time for me to sleep now...)

> 
> Thanks!
> Augie

___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH RFC] ui: add support for a tweakdefaults knob

[Pierre-Yves David]

On 06/15/2017 05:48 PM, Yuya Nishihara wrote:

On Thu, 15 Jun 2017 11:21:33 -0400, Augie Fackler wrote:

On Jun 15, 2017, at 11:19, Yuya Nishihara  wrote:
On Wed, 14 Jun 2017 21:37:21 -0400, Augie Fackler wrote:

# HG changeset patch
# User Augie Fackler 
# Date 1497488194 14400
#  Wed Jun 14 20:56:34 2017 -0400
# Node ID 0e5ea7a86a8021d02218c35b07366ac6081ab3fb
# Parent  3abba5bc34546951b11b1bd3f5e5c77b90d950d1
ui: add support for a tweakdefaults knob


+1


+def _maybetweakdefaults(self):
+if not self.configbool('ui', 'tweakdefaults'):
+return
+if self._tweaked or self.plain('tweakdefaults'):
+return
+
+# Note: it is SUPER IMPORTANT that you set self._tweaked to
+# True *before* any calls to setconfig(), otherwise you'll get
+# infinite recursion between setconfig and this method.
+#
+# TODO: We should extract an inner method in setconfig() to
+# avoid this weirdness.
+self._tweaked = True
+tmpcfg = config.config()
+tmpcfg.parse('', tweakrc)
+for section in tmpcfg:
+for name, value in tmpcfg.items(section):
+if not self.hasconfig(section, name):
+self.setconfig(section, name, value, "")


Maybe we want tmpcfg -> {ucfg, tcfg} -> ocfg layers, but it wouldn't be
doable right now since tmpcfg should be inserted *after* [uto]cfg are
loaded.


Yeah. Nobody seems motivated enough to do that work, which I totally get, so I did "done" 
instead of "clean". If it ever gets to be enough of a pain we can refactor, and we can 
maybe fix it up if we ever redo configuration to be a stack of immutable objects.


Nobody except for Jun. I'll review this patch more carefully tomorrow and
probably queue it.


A registry of config option including default value would also fit that 
need. the tweak default knob could update these default in (a copy of) 
the config registry. That would skip the explicit update of the config 
content (and associated concerns).


There have been talk about such registry of option for quite some time 
(including at the 4.2 central sprint). David Demelier ping me about 
this[1] again 10 days ago and found some time to poke at it yesterday.


It turned out I ended up with something that work, I've started 
patchbombing it[2]. Once this is in, we can update the tweakdefault flag 
to use this.


[1] as part of his quest for 
https://www.mercurial-scm.org/wiki/ConfigConsolidationPlan
[2] 
https://www.mercurial-scm.org/pipermail/mercurial-devel/2017-June/099866.html


Cheers,

--
Pierre-Yves David
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH RFC] ui: add support for a tweakdefaults knob

[Yuya Nishihara]

On Fri, 16 Jun 2017 19:26:00 -0400, Augie Fackler wrote:
> > On Jun 16, 2017, at 11:14, Yuya Nishihara  wrote:
> > On Wed, 14 Jun 2017 21:37:21 -0400, Augie Fackler wrote:
> >> # HG changeset patch
> >> # User Augie Fackler 
> >> # Date 1497488194 14400
> >> #  Wed Jun 14 20:56:34 2017 -0400
> >> # Node ID 0e5ea7a86a8021d02218c35b07366ac6081ab3fb
> >> # Parent  3abba5bc34546951b11b1bd3f5e5c77b90d950d1
> >> ui: add support for a tweakdefaults knob
> > 
> > Queued this, thanks.
> > 
> >> @@ -241,8 +257,29 @@ class ui(object):
> >> u.fixconfig(section=section)
> >> else:
> >> raise error.ProgrammingError('unknown rctype: %s' % t)
> >> +u._maybetweakdefaults()
> >> return u
> >> 
> >> +def _maybetweakdefaults(self):
> >> +if not self.configbool('ui', 'tweakdefaults'):
> >> +return
> >> +if self._tweaked or self.plain('tweakdefaults'):
> >> +return
> > 
> > tweakdefaults can't be disabled by --config if it's enabled by rc files.
> > Maybe this could be fixed by moving ui._maybetweakdefaults() to dispatch.py
> > where color.setup() is called.
> 
> Good idea! do you want to send a followup, or should I?

I haven't started that yet. Perhaps we should add some test-config.t tests
before doing further tweaks.

> >> +# Note: it is SUPER IMPORTANT that you set self._tweaked to
> >> +# True *before* any calls to setconfig(), otherwise you'll get
> >> +# infinite recursion between setconfig and this method.
> >> +#
> >> +# TODO: We should extract an inner method in setconfig() to
> >> +# avoid this weirdness.
> >> +self._tweaked = True
> >> +tmpcfg = config.config()
> >> +tmpcfg.parse('', tweakrc)
> >> +for section in tmpcfg:
> >> +for name, value in tmpcfg.items(section):
> >> +if not self.hasconfig(section, name):
> >> +self.setconfig(section, name, value, 
> >> "")
> > 
> > I have no idea how we should process values which are only set in untrusted
> > config. Using hasconfig(untrusted=True) might be a bit safer, but there 
> > would
> > still be inconsistency.
> 
> I thought untrusted was the default? Or do you just want it explicit?

untrusted is False by default. I think this and the repo config problems can
be mitigated by not setting tweaked values to _ocfg.

   if not tcfg.hasitem(section, name):
   tcfg.set(section, name, value, "")
   if not ucfg.hasitem(section, name):
   ucfg.set(section, name, value, "")
   fixconfig()
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH RFC] ui: add support for a tweakdefaults knob

[Augie Fackler]

> On Jun 16, 2017, at 11:14, Yuya Nishihara  wrote:
> 
> On Wed, 14 Jun 2017 21:37:21 -0400, Augie Fackler wrote:
>> # HG changeset patch
>> # User Augie Fackler 
>> # Date 1497488194 14400
>> #  Wed Jun 14 20:56:34 2017 -0400
>> # Node ID 0e5ea7a86a8021d02218c35b07366ac6081ab3fb
>> # Parent  3abba5bc34546951b11b1bd3f5e5c77b90d950d1
>> ui: add support for a tweakdefaults knob
> 
> Queued this, thanks.
> 
>> @@ -241,8 +257,29 @@ class ui(object):
>> u.fixconfig(section=section)
>> else:
>> raise error.ProgrammingError('unknown rctype: %s' % t)
>> +u._maybetweakdefaults()
>> return u
>> 
>> +def _maybetweakdefaults(self):
>> +if not self.configbool('ui', 'tweakdefaults'):
>> +return
>> +if self._tweaked or self.plain('tweakdefaults'):
>> +return
> 
> tweakdefaults can't be disabled by --config if it's enabled by rc files.
> Maybe this could be fixed by moving ui._maybetweakdefaults() to dispatch.py
> where color.setup() is called.

Good idea! do you want to send a followup, or should I?

> 
>> +# Note: it is SUPER IMPORTANT that you set self._tweaked to
>> +# True *before* any calls to setconfig(), otherwise you'll get
>> +# infinite recursion between setconfig and this method.
>> +#
>> +# TODO: We should extract an inner method in setconfig() to
>> +# avoid this weirdness.
>> +self._tweaked = True
>> +tmpcfg = config.config()
>> +tmpcfg.parse('', tweakrc)
>> +for section in tmpcfg:
>> +for name, value in tmpcfg.items(section):
>> +if not self.hasconfig(section, name):
>> +self.setconfig(section, name, value, "")
> 
> I have no idea how we should process values which are only set in untrusted
> config. Using hasconfig(untrusted=True) might be a bit safer, but there would
> still be inconsistency.

I thought untrusted was the default? Or do you just want it explicit?

> 
> Another problem is self._ocfg can shadow repo config which isn't loaded yet
> when _maybetweakdefaults() is called.

___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH RFC] ui: add support for a tweakdefaults knob

[Yuya Nishihara]

On Wed, 14 Jun 2017 21:37:21 -0400, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler 
> # Date 1497488194 14400
> #  Wed Jun 14 20:56:34 2017 -0400
> # Node ID 0e5ea7a86a8021d02218c35b07366ac6081ab3fb
> # Parent  3abba5bc34546951b11b1bd3f5e5c77b90d950d1
> ui: add support for a tweakdefaults knob

Queued this, thanks.

> @@ -241,8 +257,29 @@ class ui(object):
>  u.fixconfig(section=section)
>  else:
>  raise error.ProgrammingError('unknown rctype: %s' % t)
> +u._maybetweakdefaults()
>  return u
>  
> +def _maybetweakdefaults(self):
> +if not self.configbool('ui', 'tweakdefaults'):
> +return
> +if self._tweaked or self.plain('tweakdefaults'):
> +return

tweakdefaults can't be disabled by --config if it's enabled by rc files.
Maybe this could be fixed by moving ui._maybetweakdefaults() to dispatch.py
where color.setup() is called.

> +# Note: it is SUPER IMPORTANT that you set self._tweaked to
> +# True *before* any calls to setconfig(), otherwise you'll get
> +# infinite recursion between setconfig and this method.
> +#
> +# TODO: We should extract an inner method in setconfig() to
> +# avoid this weirdness.
> +self._tweaked = True
> +tmpcfg = config.config()
> +tmpcfg.parse('', tweakrc)
> +for section in tmpcfg:
> +for name, value in tmpcfg.items(section):
> +if not self.hasconfig(section, name):
> +self.setconfig(section, name, value, "")

I have no idea how we should process values which are only set in untrusted
config. Using hasconfig(untrusted=True) might be a bit safer, but there would
still be inconsistency.

Another problem is self._ocfg can shadow repo config which isn't loaded yet
when _maybetweakdefaults() is called.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH RFC] ui: add support for a tweakdefaults knob

[Martin von Zweigbergk via Mercurial-devel]

On Thu, Jun 15, 2017 at 8:19 AM, Yuya Nishihara  wrote:
> On Wed, 14 Jun 2017 21:37:21 -0400, Augie Fackler wrote:
>> # HG changeset patch
>> # User Augie Fackler 
>> # Date 1497488194 14400
>> #  Wed Jun 14 20:56:34 2017 -0400
>> # Node ID 0e5ea7a86a8021d02218c35b07366ac6081ab3fb
>> # Parent  3abba5bc34546951b11b1bd3f5e5c77b90d950d1
>> ui: add support for a tweakdefaults knob
>
> +1

I forgot to say, but definitely +1 from me too. Thanks!

>
>> +def _maybetweakdefaults(self):
>> +if not self.configbool('ui', 'tweakdefaults'):
>> +return
>> +if self._tweaked or self.plain('tweakdefaults'):
>> +return
>> +
>> +# Note: it is SUPER IMPORTANT that you set self._tweaked to
>> +# True *before* any calls to setconfig(), otherwise you'll get
>> +# infinite recursion between setconfig and this method.
>> +#
>> +# TODO: We should extract an inner method in setconfig() to
>> +# avoid this weirdness.
>> +self._tweaked = True
>> +tmpcfg = config.config()
>> +tmpcfg.parse('', tweakrc)
>> +for section in tmpcfg:
>> +for name, value in tmpcfg.items(section):
>> +if not self.hasconfig(section, name):
>> +self.setconfig(section, name, value, "")
>
> Maybe we want tmpcfg -> {ucfg, tcfg} -> ocfg layers, but it wouldn't be
> doable right now since tmpcfg should be inserted *after* [uto]cfg are
> loaded.

I don't think I followed that, but another possible way it could work
is for tweakdefaults to behave as if the settings were updated inline,
so tweakdefaults=yes in system config would behave as if all the
tweaks happened in the system config, etc. Anyway, let's leave that
polish (whatever form it takes) for a later. I think we can live with
that little BC breakage (like we did with the environment variable
overrides by Jun).

> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH RFC] ui: add support for a tweakdefaults knob

[Martin von Zweigbergk via Mercurial-devel]

On Wed, Jun 14, 2017 at 6:37 PM, Augie Fackler  wrote:
> # HG changeset patch
> # User Augie Fackler 
> # Date 1497488194 14400
> #  Wed Jun 14 20:56:34 2017 -0400
> # Node ID 0e5ea7a86a8021d02218c35b07366ac6081ab3fb
> # Parent  3abba5bc34546951b11b1bd3f5e5c77b90d950d1
> ui: add support for a tweakdefaults knob
>
> We've been talking for years about a one-stop config knob to opt in to
> better behavior. There have been a lot of ideas thrown around, but
> they all seem to be too complicated to get anyone to actually do the
> work.. As such, this patch is the stupidest thing that can possibly
> work in the name of getting a good feature to users.
>
> Right now it's just three config settings that I think are generally
> uncontroversial, but I expect to add more soon. That will likely
> include adding new config knobs for the express purpose of adding them
> to tweakdefaults.
>
> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
> --- a/mercurial/help/config.txt
> +++ b/mercurial/help/config.txt
> @@ -2071,6 +2071,15 @@ User interface controls.
>  on all exceptions, even those recognized by Mercurial (such as
>  IOError or MemoryError). (default: False)
>
> +``tweakdefaults``
> +
> +By default Mercurial's behavior changes very little from release
> +to release, but over time the recommended config settings
> +shift. Enable this config to opt in to get automatic tweaks to
> +Mercurial's behavior over time. This config setting will have no
> +effet if ``HGPLAIN` is set or ``HGPLAINEXCEPT`` is set and does
> +not include ``tweakdefaults``. (default: False)
> +
>  ``username``
>  The committer of a changeset created when running "commit".
>  Typically a person's name and email address, e.g. ``Fred Widget
> diff --git a/mercurial/ui.py b/mercurial/ui.py
> --- a/mercurial/ui.py
> +++ b/mercurial/ui.py
> @@ -43,6 +43,20 @@ urlreq = util.urlreq
>  _keepalnum = ''.join(c for c in map(pycompat.bytechr, range(256))
>   if not c.isalnum())
>
> +# The config knobs that will be altered (if unset) by ui.tweakdefaults.
> +tweakrc = """
> +[ui]
> +# The rollback command is dangerous. As a rule, don't use it.
> +rollback = False
> +
> +[commands]
> +# Make `hg status` emit cwd-relative paths by default.
> +status.relative = yes
> +
> +[diff]
> +git = 1
> +"""
> +
>  samplehgrcs = {
>  'user':
>  """# example user config (see 'hg help config' for more info)
> @@ -182,6 +196,7 @@ class ui(object):
>  self.fin = src.fin
>  self.pageractive = src.pageractive
>  self._disablepager = src._disablepager
> +self._tweaked = src._tweaked
>
>  self._tcfg = src._tcfg.copy()
>  self._ucfg = src._ucfg.copy()
> @@ -205,6 +220,7 @@ class ui(object):
>  self.fin = util.stdin
>  self.pageractive = False
>  self._disablepager = False
> +self._tweaked = False
>
>  # shared read-only environment
>  self.environ = encoding.environ
> @@ -241,8 +257,29 @@ class ui(object):
>  u.fixconfig(section=section)
>  else:
>  raise error.ProgrammingError('unknown rctype: %s' % t)
> +u._maybetweakdefaults()
>  return u
>
> +def _maybetweakdefaults(self):
> +if not self.configbool('ui', 'tweakdefaults'):
> +return
> +if self._tweaked or self.plain('tweakdefaults'):
> +return
> +
> +# Note: it is SUPER IMPORTANT that you set self._tweaked to
> +# True *before* any calls to setconfig(), otherwise you'll get
> +# infinite recursion between setconfig and this method.
> +#
> +# TODO: We should extract an inner method in setconfig() to
> +# avoid this weirdness.
> +self._tweaked = True
> +tmpcfg = config.config()
> +tmpcfg.parse('', tweakrc)
> +for section in tmpcfg:
> +for name, value in tmpcfg.items(section):
> +if not self.hasconfig(section, name):
> +self.setconfig(section, name, value, "")
> +
>  def copy(self):
>  return self.__class__(self)
>
> @@ -387,6 +424,7 @@ class ui(object):
>  for cfg in (self._ocfg, self._tcfg, self._ucfg):
>  cfg.set(section, name, value, source)
>  self.fixconfig(section=section)
> +self._maybetweakdefaults()
>
>  def _data(self, untrusted):
>  return untrusted and self._ucfg or self._tcfg
> diff --git a/tests/test-status.t b/tests/test-status.t
> --- a/tests/test-status.t
> +++ b/tests/test-status.t

As I said on IRC, I'd consider testing this in test-config.t and only
checking the output of "hg (show)config". Since it will be completely
transparent to the users (such as the status command), I think that's
probably enough. Or at least it probably means we can test less in the

Re: [PATCH RFC] ui: add support for a tweakdefaults knob

[Yuya Nishihara]

On Thu, 15 Jun 2017 11:21:33 -0400, Augie Fackler wrote:
> > On Jun 15, 2017, at 11:19, Yuya Nishihara  wrote:
> > On Wed, 14 Jun 2017 21:37:21 -0400, Augie Fackler wrote:
> >> # HG changeset patch
> >> # User Augie Fackler 
> >> # Date 1497488194 14400
> >> #  Wed Jun 14 20:56:34 2017 -0400
> >> # Node ID 0e5ea7a86a8021d02218c35b07366ac6081ab3fb
> >> # Parent  3abba5bc34546951b11b1bd3f5e5c77b90d950d1
> >> ui: add support for a tweakdefaults knob
> > 
> > +1
> > 
> >> +def _maybetweakdefaults(self):
> >> +if not self.configbool('ui', 'tweakdefaults'):
> >> +return
> >> +if self._tweaked or self.plain('tweakdefaults'):
> >> +return
> >> +
> >> +# Note: it is SUPER IMPORTANT that you set self._tweaked to
> >> +# True *before* any calls to setconfig(), otherwise you'll get
> >> +# infinite recursion between setconfig and this method.
> >> +#
> >> +# TODO: We should extract an inner method in setconfig() to
> >> +# avoid this weirdness.
> >> +self._tweaked = True
> >> +tmpcfg = config.config()
> >> +tmpcfg.parse('', tweakrc)
> >> +for section in tmpcfg:
> >> +for name, value in tmpcfg.items(section):
> >> +if not self.hasconfig(section, name):
> >> +self.setconfig(section, name, value, 
> >> "")
> > 
> > Maybe we want tmpcfg -> {ucfg, tcfg} -> ocfg layers, but it wouldn't be
> > doable right now since tmpcfg should be inserted *after* [uto]cfg are
> > loaded.
> 
> Yeah. Nobody seems motivated enough to do that work, which I totally get, so 
> I did "done" instead of "clean". If it ever gets to be enough of a pain we 
> can refactor, and we can maybe fix it up if we ever redo configuration to be 
> a stack of immutable objects.

Nobody except for Jun. I'll review this patch more carefully tomorrow and
probably queue it.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH RFC] ui: add support for a tweakdefaults knob

[Augie Fackler]

> On Jun 15, 2017, at 11:19, Yuya Nishihara  wrote:
> 
> On Wed, 14 Jun 2017 21:37:21 -0400, Augie Fackler wrote:
>> # HG changeset patch
>> # User Augie Fackler 
>> # Date 1497488194 14400
>> #  Wed Jun 14 20:56:34 2017 -0400
>> # Node ID 0e5ea7a86a8021d02218c35b07366ac6081ab3fb
>> # Parent  3abba5bc34546951b11b1bd3f5e5c77b90d950d1
>> ui: add support for a tweakdefaults knob
> 
> +1
> 
>> +def _maybetweakdefaults(self):
>> +if not self.configbool('ui', 'tweakdefaults'):
>> +return
>> +if self._tweaked or self.plain('tweakdefaults'):
>> +return
>> +
>> +# Note: it is SUPER IMPORTANT that you set self._tweaked to
>> +# True *before* any calls to setconfig(), otherwise you'll get
>> +# infinite recursion between setconfig and this method.
>> +#
>> +# TODO: We should extract an inner method in setconfig() to
>> +# avoid this weirdness.
>> +self._tweaked = True
>> +tmpcfg = config.config()
>> +tmpcfg.parse('', tweakrc)
>> +for section in tmpcfg:
>> +for name, value in tmpcfg.items(section):
>> +if not self.hasconfig(section, name):
>> +self.setconfig(section, name, value, "")
> 
> Maybe we want tmpcfg -> {ucfg, tcfg} -> ocfg layers, but it wouldn't be
> doable right now since tmpcfg should be inserted *after* [uto]cfg are
> loaded.

Yeah. Nobody seems motivated enough to do that work, which I totally get, so I 
did "done" instead of "clean". If it ever gets to be enough of a pain we can 
refactor, and we can maybe fix it up if we ever redo configuration to be a 
stack of immutable objects.

___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH RFC] ui: add support for a tweakdefaults knob

[Yuya Nishihara]

On Wed, 14 Jun 2017 21:37:21 -0400, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler 
> # Date 1497488194 14400
> #  Wed Jun 14 20:56:34 2017 -0400
> # Node ID 0e5ea7a86a8021d02218c35b07366ac6081ab3fb
> # Parent  3abba5bc34546951b11b1bd3f5e5c77b90d950d1
> ui: add support for a tweakdefaults knob

+1

> +def _maybetweakdefaults(self):
> +if not self.configbool('ui', 'tweakdefaults'):
> +return
> +if self._tweaked or self.plain('tweakdefaults'):
> +return
> +
> +# Note: it is SUPER IMPORTANT that you set self._tweaked to
> +# True *before* any calls to setconfig(), otherwise you'll get
> +# infinite recursion between setconfig and this method.
> +#
> +# TODO: We should extract an inner method in setconfig() to
> +# avoid this weirdness.
> +self._tweaked = True
> +tmpcfg = config.config()
> +tmpcfg.parse('', tweakrc)
> +for section in tmpcfg:
> +for name, value in tmpcfg.items(section):
> +if not self.hasconfig(section, name):
> +self.setconfig(section, name, value, "")

Maybe we want tmpcfg -> {ucfg, tcfg} -> ocfg layers, but it wouldn't be
doable right now since tmpcfg should be inserted *after* [uto]cfg are
loaded.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH RFC] ui: add support for a tweakdefaults knob

[Augie Fackler]

> On Jun 15, 2017, at 01:14, Martin von Zweigbergk  
> wrote:
> 
> At what config level is the tweaking done?

It's done really late, because it's plausible that the user-level or repo-level 
hgrc would contain ui.tweakdefaults=True. It checks for all the config entries 
it wants to set, and if anything is already set to a value it doesn't make any 
changes.

> I didn't understand the
> code well enough to tell.


> Concretely, let's say I have
> ui.tweakdefaults=yes in my system setting and
> command.status.relative=no in my local settings would I get relative
> status or not? I would assume not.

You would not get relative status.

> How about the inverse
> (command.status.relative=no in my system settings and
> ui.tweakdefaults=yes in my local settings)?

You would also not get relative status.

> I'm not sure what I would
> expect in this case, but probably to not get relative status. (It's a
> weird example because there's no reason to set
> command.status.relative=no in the system settings, but there are
> probably more reasonable examples for non-boolean options).

Yeah, it's definitely rough around the edges, but I think it's better than not 
having the feature.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH RFC] ui: add support for a tweakdefaults knob

[Martin von Zweigbergk via Mercurial-devel]

On Wed, Jun 14, 2017 at 6:37 PM, Augie Fackler  wrote:
> # HG changeset patch
> # User Augie Fackler 
> # Date 1497488194 14400
> #  Wed Jun 14 20:56:34 2017 -0400
> # Node ID 0e5ea7a86a8021d02218c35b07366ac6081ab3fb
> # Parent  3abba5bc34546951b11b1bd3f5e5c77b90d950d1
> ui: add support for a tweakdefaults knob
>
> We've been talking for years about a one-stop config knob to opt in to
> better behavior. There have been a lot of ideas thrown around, but
> they all seem to be too complicated to get anyone to actually do the
> work.. As such, this patch is the stupidest thing that can possibly
> work in the name of getting a good feature to users.
>
> Right now it's just three config settings that I think are generally
> uncontroversial, but I expect to add more soon. That will likely
> include adding new config knobs for the express purpose of adding them
> to tweakdefaults.
>
> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
> --- a/mercurial/help/config.txt
> +++ b/mercurial/help/config.txt
> @@ -2071,6 +2071,15 @@ User interface controls.
>  on all exceptions, even those recognized by Mercurial (such as
>  IOError or MemoryError). (default: False)
>
> +``tweakdefaults``
> +
> +By default Mercurial's behavior changes very little from release
> +to release, but over time the recommended config settings
> +shift. Enable this config to opt in to get automatic tweaks to
> +Mercurial's behavior over time. This config setting will have no
> +effet if ``HGPLAIN` is set or ``HGPLAINEXCEPT`` is set and does
> +not include ``tweakdefaults``. (default: False)
> +
>  ``username``
>  The committer of a changeset created when running "commit".
>  Typically a person's name and email address, e.g. ``Fred Widget
> diff --git a/mercurial/ui.py b/mercurial/ui.py
> --- a/mercurial/ui.py
> +++ b/mercurial/ui.py
> @@ -43,6 +43,20 @@ urlreq = util.urlreq
>  _keepalnum = ''.join(c for c in map(pycompat.bytechr, range(256))
>   if not c.isalnum())
>
> +# The config knobs that will be altered (if unset) by ui.tweakdefaults.
> +tweakrc = """
> +[ui]
> +# The rollback command is dangerous. As a rule, don't use it.
> +rollback = False
> +
> +[commands]
> +# Make `hg status` emit cwd-relative paths by default.
> +status.relative = yes
> +
> +[diff]
> +git = 1
> +"""
> +
>  samplehgrcs = {
>  'user':
>  """# example user config (see 'hg help config' for more info)
> @@ -182,6 +196,7 @@ class ui(object):
>  self.fin = src.fin
>  self.pageractive = src.pageractive
>  self._disablepager = src._disablepager
> +self._tweaked = src._tweaked
>
>  self._tcfg = src._tcfg.copy()
>  self._ucfg = src._ucfg.copy()
> @@ -205,6 +220,7 @@ class ui(object):
>  self.fin = util.stdin
>  self.pageractive = False
>  self._disablepager = False
> +self._tweaked = False
>
>  # shared read-only environment
>  self.environ = encoding.environ
> @@ -241,8 +257,29 @@ class ui(object):
>  u.fixconfig(section=section)
>  else:
>  raise error.ProgrammingError('unknown rctype: %s' % t)
> +u._maybetweakdefaults()
>  return u
>
> +def _maybetweakdefaults(self):
> +if not self.configbool('ui', 'tweakdefaults'):
> +return
> +if self._tweaked or self.plain('tweakdefaults'):
> +return
> +
> +# Note: it is SUPER IMPORTANT that you set self._tweaked to
> +# True *before* any calls to setconfig(), otherwise you'll get
> +# infinite recursion between setconfig and this method.
> +#
> +# TODO: We should extract an inner method in setconfig() to
> +# avoid this weirdness.
> +self._tweaked = True
> +tmpcfg = config.config()
> +tmpcfg.parse('', tweakrc)
> +for section in tmpcfg:
> +for name, value in tmpcfg.items(section):
> +if not self.hasconfig(section, name):
> +self.setconfig(section, name, value, "")
> +
>  def copy(self):
>  return self.__class__(self)
>
> @@ -387,6 +424,7 @@ class ui(object):
>  for cfg in (self._ocfg, self._tcfg, self._ucfg):
>  cfg.set(section, name, value, source)
>  self.fixconfig(section=section)
> +self._maybetweakdefaults()
>
>  def _data(self, untrusted):
>  return untrusted and self._ucfg or self._tcfg
> diff --git a/tests/test-status.t b/tests/test-status.t
> --- a/tests/test-status.t
> +++ b/tests/test-status.t
> @@ -107,6 +107,29 @@ combining patterns with root and pattern
>? a/in_a
>? b/in_b
>
> +tweaking defaults works
> +  $ hg status --cwd a --config ui.tweakdefaults=yes
> +  ? 1/in_a_1
> +  ? in_a
> +  ? ../b/1/in_b_1
> +  ? ../b/2/in_b_2
> +  ? ../b/in_b
> +  ? ../in_root
> +  $ 

Re: [PATCH RFC] ui: add support for a tweakdefaults knob

[Siddharth Agarwal]

On 6/14/17 6:37 PM, Augie Fackler wrote:

# HG changeset patch
# User Augie Fackler 
# Date 1497488194 14400
#  Wed Jun 14 20:56:34 2017 -0400
# Node ID 0e5ea7a86a8021d02218c35b07366ac6081ab3fb
# Parent  3abba5bc34546951b11b1bd3f5e5c77b90d950d1
ui: add support for a tweakdefaults knob


+1 -- this gives us a great path forward.



We've been talking for years about a one-stop config knob to opt in to
better behavior. There have been a lot of ideas thrown around, but
they all seem to be too complicated to get anyone to actually do the
work.. As such, this patch is the stupidest thing that can possibly
work in the name of getting a good feature to users.

Right now it's just three config settings that I think are generally
uncontroversial, but I expect to add more soon. That will likely
include adding new config knobs for the express purpose of adding them
to tweakdefaults.

diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
--- a/mercurial/help/config.txt
+++ b/mercurial/help/config.txt
@@ -2071,6 +2071,15 @@ User interface controls.
  on all exceptions, even those recognized by Mercurial (such as
  IOError or MemoryError). (default: False)
  
+``tweakdefaults``

+
+By default Mercurial's behavior changes very little from release
+to release, but over time the recommended config settings
+shift. Enable this config to opt in to get automatic tweaks to
+Mercurial's behavior over time. This config setting will have no
+effet if ``HGPLAIN` is set or ``HGPLAINEXCEPT`` is set and does
+not include ``tweakdefaults``. (default: False)
+
  ``username``
  The committer of a changeset created when running "commit".
  Typically a person's name and email address, e.g. ``Fred Widget
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -43,6 +43,20 @@ urlreq = util.urlreq
  _keepalnum = ''.join(c for c in map(pycompat.bytechr, range(256))
   if not c.isalnum())
  
+# The config knobs that will be altered (if unset) by ui.tweakdefaults.

+tweakrc = """
+[ui]
+# The rollback command is dangerous. As a rule, don't use it.
+rollback = False
+
+[commands]
+# Make `hg status` emit cwd-relative paths by default.
+status.relative = yes
+
+[diff]
+git = 1
+"""
+
  samplehgrcs = {
  'user':
  """# example user config (see 'hg help config' for more info)
@@ -182,6 +196,7 @@ class ui(object):
  self.fin = src.fin
  self.pageractive = src.pageractive
  self._disablepager = src._disablepager
+self._tweaked = src._tweaked
  
  self._tcfg = src._tcfg.copy()

  self._ucfg = src._ucfg.copy()
@@ -205,6 +220,7 @@ class ui(object):
  self.fin = util.stdin
  self.pageractive = False
  self._disablepager = False
+self._tweaked = False
  
  # shared read-only environment

  self.environ = encoding.environ
@@ -241,8 +257,29 @@ class ui(object):
  u.fixconfig(section=section)
  else:
  raise error.ProgrammingError('unknown rctype: %s' % t)
+u._maybetweakdefaults()
  return u
  
+def _maybetweakdefaults(self):

+if not self.configbool('ui', 'tweakdefaults'):
+return
+if self._tweaked or self.plain('tweakdefaults'):
+return
+
+# Note: it is SUPER IMPORTANT that you set self._tweaked to
+# True *before* any calls to setconfig(), otherwise you'll get
+# infinite recursion between setconfig and this method.
+#
+# TODO: We should extract an inner method in setconfig() to
+# avoid this weirdness.
+self._tweaked = True
+tmpcfg = config.config()
+tmpcfg.parse('', tweakrc)
+for section in tmpcfg:
+for name, value in tmpcfg.items(section):
+if not self.hasconfig(section, name):
+self.setconfig(section, name, value, "")
+
  def copy(self):
  return self.__class__(self)
  
@@ -387,6 +424,7 @@ class ui(object):

  for cfg in (self._ocfg, self._tcfg, self._ucfg):
  cfg.set(section, name, value, source)
  self.fixconfig(section=section)
+self._maybetweakdefaults()
  
  def _data(self, untrusted):

  return untrusted and self._ucfg or self._tcfg
diff --git a/tests/test-status.t b/tests/test-status.t
--- a/tests/test-status.t
+++ b/tests/test-status.t
@@ -107,6 +107,29 @@ combining patterns with root and pattern
? a/in_a
? b/in_b
  
+tweaking defaults works

+  $ hg status --cwd a --config ui.tweakdefaults=yes
+  ? 1/in_a_1
+  ? in_a
+  ? ../b/1/in_b_1
+  ? ../b/2/in_b_2
+  ? ../b/in_b
+  ? ../in_root
+  $ HGPLAIN=1 hg status --cwd a --config ui.tweakdefaults=yes
+  ? a/1/in_a_1 (glob)
+  ? a/in_a (glob)
+  ? b/1/in_b_1 (glob)
+  ? b/2/in_b_2 (glob)
+  ? b/in_b (glob)
+  ? in_root
+  

Re: [PATCH RFC] ui: add support for a tweakdefaults knob

[Sean Farley]

Sean Farley  writes:

> Augie Fackler  writes:
>
>> # HG changeset patch
>> # User Augie Fackler 
>> # Date 1497488194 14400
>> #  Wed Jun 14 20:56:34 2017 -0400
>> # Node ID 0e5ea7a86a8021d02218c35b07366ac6081ab3fb
>> # Parent  3abba5bc34546951b11b1bd3f5e5c77b90d950d1
>> ui: add support for a tweakdefaults knob
>>
>> We've been talking for years about a one-stop config knob to opt in to
>> better behavior. There have been a lot of ideas thrown around, but
>> they all seem to be too complicated to get anyone to actually do the
>> work.. As such, this patch is the stupidest thing that can possibly
>> work in the name of getting a good feature to users.
>>
>> Right now it's just three config settings that I think are generally
>> uncontroversial, but I expect to add more soon. That will likely
>> include adding new config knobs for the express purpose of adding them
>> to tweakdefaults.
>
> https://media.giphy.com/media/Mes24baGbS5Og/giphy.gif

In case it wasn't clear, I am excited about this patch. The name is fine
to me since it makes it very clear on what it does.


signature.asc
Description: PGP signature
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH RFC] ui: add support for a tweakdefaults knob

[Sean Farley]

Augie Fackler  writes:

> # HG changeset patch
> # User Augie Fackler 
> # Date 1497488194 14400
> #  Wed Jun 14 20:56:34 2017 -0400
> # Node ID 0e5ea7a86a8021d02218c35b07366ac6081ab3fb
> # Parent  3abba5bc34546951b11b1bd3f5e5c77b90d950d1
> ui: add support for a tweakdefaults knob
>
> We've been talking for years about a one-stop config knob to opt in to
> better behavior. There have been a lot of ideas thrown around, but
> they all seem to be too complicated to get anyone to actually do the
> work.. As such, this patch is the stupidest thing that can possibly
> work in the name of getting a good feature to users.
>
> Right now it's just three config settings that I think are generally
> uncontroversial, but I expect to add more soon. That will likely
> include adding new config knobs for the express purpose of adding them
> to tweakdefaults.

https://media.giphy.com/media/Mes24baGbS5Og/giphy.gif


signature.asc
Description: PGP signature
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel