Patchwork [3,of,8] ui: add config knob for progressive mode

login
register
mail settings
Submitter Mathias De Maré
Date March 28, 2015, 10:51 a.m.
Message ID <ea4595977e4e882b7a35.1427539863@waste.org>
Download mbox | patch
Permalink /patch/8324/
State Rejected
Delegated to: Matt Mackall
Headers show

Comments

Mathias De Maré - March 28, 2015, 10:51 a.m.
# HG changeset patch
# User Mathias De Maré <mathias.demare@gmail.com>
# Date 1427227793 -3600
#      Tue Mar 24 21:09:53 2015 +0100
# Node ID ea4595977e4e882b7a355e7e7430989cee3ea000
# Parent  96a2ed2e01ef6033b74becfde0f79d5b61adbf11
ui: add config knob for progressive mode

This patch adds the ui.progressive config knob,
to allow enabling a number of progressive settings,
as proposed by Augie Fackler.
Additional settings (none added yet) can be added to progressive mode
by appending to _fixprogressive.
Augie Fackler - March 30, 2015, 2:26 p.m.
On Sat, Mar 28, 2015 at 05:51:03AM -0500, Mathias De Maré wrote:
> # HG changeset patch
> # User Mathias De Maré <mathias.demare@gmail.com>
> # Date 1427227793 -3600
> #      Tue Mar 24 21:09:53 2015 +0100
> # Node ID ea4595977e4e882b7a355e7e7430989cee3ea000
> # Parent  96a2ed2e01ef6033b74becfde0f79d5b61adbf11
> ui: add config knob for progressive mode
>
> This patch adds the ui.progressive config knob,
> to allow enabling a number of progressive settings,
> as proposed by Augie Fackler.
> Additional settings (none added yet) can be added to progressive mode
> by appending to _fixprogressive.
>
> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
> --- a/mercurial/help/config.txt
> +++ b/mercurial/help/config.txt
> @@ -1411,6 +1411,12 @@
>      If set to ``abort``, the command is aborted.
>      On Windows, this configuration option is ignored and the command aborted.
>
> +``progressive``
> +    Enable user-friendly features. True or False. Default is False;
> +    If set to True, specific functionality that improves user experience
> +    will be enabled, even if there is no complete backwards compatibility.
> +    Scripts should use the environment variable ``HGPLAIN`` to avoid impact.

We should probably have some comment here about how progressive-mode
is an explicitly moving target of our most-recommended configuration,
and that scripters should be sure to use the HGPLAIN variable.

Other than that, and potential bikeshedding on the name of
`progressive` (I don't feel strongly either way about that), I'm +1 on
the series, modulo the behavioral fix on patch 1.

> +
>  ``quiet``
>      Reduce the amount of output printed. True or False. Default is False.
>
> diff --git a/mercurial/ui.py b/mercurial/ui.py
> --- a/mercurial/ui.py
> +++ b/mercurial/ui.py
> @@ -79,6 +79,7 @@
>          # _bufferstates: Should the temporary capture includes stderr
>          self._bufferstates = []
>          self.quiet = self.verbose = self.debugflag = self.tracebackflag = False
> +        self.progressive = False
>          self._reportuntrusted = True
>          self._ocfg = config.config() # overlay
>          self._tcfg = config.config() # trusted
> @@ -111,6 +112,8 @@
>              for f in scmutil.rcpath():
>                  self.readconfig(f, trust=True)
>
> +        self._fixprogressive(self.progressive)
> +
>      def copy(self):
>          return self.__class__(self)
>
> @@ -158,7 +161,7 @@
>
>          if self.plain():
>              for k in ('debug', 'fallbackencoding', 'quiet', 'slash',
> -                      'logtemplate', 'style',
> +                      'logtemplate', 'progressive', 'style',
>                        'traceback', 'verbose'):
>                  if k in cfg['ui']:
>                      del cfg['ui'][k]
> @@ -207,6 +210,7 @@
>              self._reportuntrusted = self.debugflag or self.configbool("ui",
>                  "report_untrusted", True)
>              self.tracebackflag = self.configbool('ui', 'traceback', False)
> +            self.progressive = self.configbool('ui', 'progressive', False)
>
>          if section in (None, 'trusted'):
>              # update trust information
> @@ -452,6 +456,11 @@
>          '''tell whether section exists in config.'''
>          return section in self._data(untrusted)
>
> +    def hasconfig(self, section, config, untrusted=False):
> +        '''tell whether config item exists in config.'''
> +        return self.has_section(section, untrusted) \
> +            and config in self._data(untrusted)[section]
> +
>      def configitems(self, section, untrusted=False):
>          items = self._data(untrusted).items(section)
>          if self.debugflag and not untrusted and self._reportuntrusted:
> @@ -467,6 +476,10 @@
>              for name, value in self.configitems(section, untrusted):
>                  yield section, name, value
>
> +    def _fixprogressive(self, progressive):
> +        if not progressive:
> +            return
> +
>      def plain(self, feature=None):
>          '''is plain mode active?
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Pierre-Yves David - March 30, 2015, 7:28 p.m.
On 03/30/2015 07:26 AM, Augie Fackler wrote:
> On Sat, Mar 28, 2015 at 05:51:03AM -0500, Mathias De Maré wrote:
>> # HG changeset patch
>> # User Mathias De Maré <mathias.demare@gmail.com>
>> # Date 1427227793 -3600
>> #      Tue Mar 24 21:09:53 2015 +0100
>> # Node ID ea4595977e4e882b7a355e7e7430989cee3ea000
>> # Parent  96a2ed2e01ef6033b74becfde0f79d5b61adbf11
>> ui: add config knob for progressive mode
>>
>> This patch adds the ui.progressive config knob,
>> to allow enabling a number of progressive settings,
>> as proposed by Augie Fackler.
>> Additional settings (none added yet) can be added to progressive mode
>> by appending to _fixprogressive.
>>
>> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
>> --- a/mercurial/help/config.txt
>> +++ b/mercurial/help/config.txt
>> @@ -1411,6 +1411,12 @@
>>       If set to ``abort``, the command is aborted.
>>       On Windows, this configuration option is ignored and the command aborted.
>>
>> +``progressive``
>> +    Enable user-friendly features. True or False. Default is False;
>> +    If set to True, specific functionality that improves user experience
>> +    will be enabled, even if there is no complete backwards compatibility.
>> +    Scripts should use the environment variable ``HGPLAIN`` to avoid impact.
>
> We should probably have some comment here about how progressive-mode
> is an explicitly moving target of our most-recommended configuration,
> and that scripters should be sure to use the HGPLAIN variable.
>
> Other than that, and potential bikeshedding on the name of
> `progressive` (I don't feel strongly either way about that), I'm +1 on
> the series, modulo the behavioral fix on patch 1.

I would probably goes for "experiemental.progressiveui" until at least 
the sprint. This will give us a couple of week to test the thing before 
committing to it with a public config. And I expect sprint bike 
shredding to be much more efficient.
Sean Farley - March 30, 2015, 7:47 p.m.
Pierre-Yves David <pierre-yves.david@ens-lyon.org> writes:

> On 03/30/2015 07:26 AM, Augie Fackler wrote:
>> On Sat, Mar 28, 2015 at 05:51:03AM -0500, Mathias De Maré wrote:
>>> # HG changeset patch
>>> # User Mathias De Maré <mathias.demare@gmail.com>
>>> # Date 1427227793 -3600
>>> #      Tue Mar 24 21:09:53 2015 +0100
>>> # Node ID ea4595977e4e882b7a355e7e7430989cee3ea000
>>> # Parent  96a2ed2e01ef6033b74becfde0f79d5b61adbf11
>>> ui: add config knob for progressive mode
>>>
>>> This patch adds the ui.progressive config knob,
>>> to allow enabling a number of progressive settings,
>>> as proposed by Augie Fackler.
>>> Additional settings (none added yet) can be added to progressive mode
>>> by appending to _fixprogressive.
>>>
>>> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
>>> --- a/mercurial/help/config.txt
>>> +++ b/mercurial/help/config.txt
>>> @@ -1411,6 +1411,12 @@
>>>       If set to ``abort``, the command is aborted.
>>>       On Windows, this configuration option is ignored and the command aborted.
>>>
>>> +``progressive``
>>> +    Enable user-friendly features. True or False. Default is False;
>>> +    If set to True, specific functionality that improves user experience
>>> +    will be enabled, even if there is no complete backwards compatibility.
>>> +    Scripts should use the environment variable ``HGPLAIN`` to avoid impact.
>>
>> We should probably have some comment here about how progressive-mode
>> is an explicitly moving target of our most-recommended configuration,
>> and that scripters should be sure to use the HGPLAIN variable.
>>
>> Other than that, and potential bikeshedding on the name of
>> `progressive` (I don't feel strongly either way about that), I'm +1 on
>> the series, modulo the behavioral fix on patch 1.
>
> I would probably goes for "experiemental.progressiveui" until at least 
> the sprint. This will give us a couple of week to test the thing before 
> committing to it with a public config. And I expect sprint bike 
> shredding to be much more efficient.

Just chiming in that instead of naming this variable 'friendly' or
'progressive,' why not call it what it is: 'backwards_incompatible' or
something similar?

That being said, I have no real preference one way or the either.
Matt Mackall - March 31, 2015, 11:42 a.m.
On Sat, 2015-03-28 at 05:51 -0500, Mathias De Maré wrote:
> # HG changeset patch
> # User Mathias De Maré <mathias.demare@gmail.com>
> # Date 1427227793 -3600
> #      Tue Mar 24 21:09:53 2015 +0100
> # Node ID ea4595977e4e882b7a355e7e7430989cee3ea000
> # Parent  96a2ed2e01ef6033b74becfde0f79d5b61adbf11
> ui: add config knob for progressive mode

> +        self.progressive = False

This would probable be better as a function of the same name.

> @@ -452,6 +456,11 @@
>          '''tell whether section exists in config.'''
>          return section in self._data(untrusted)
>  
> +    def hasconfig(self, section, config, untrusted=False):
> +        '''tell whether config item exists in config.'''
> +        return self.has_section(section, untrusted) \
> +            and config in self._data(untrusted)[section]
> +

You accidentally grew an unused function?
Mathias De Maré - April 1, 2015, 3:16 p.m.
On Tue, Mar 31, 2015 at 1:42 PM, Matt Mackall <mpm@selenic.com> wrote:

> On Sat, 2015-03-28 at 05:51 -0500, Mathias De Maré wrote:
> > # HG changeset patch
> > # User Mathias De Maré <mathias.demare@gmail.com>
> > # Date 1427227793 -3600
> > #      Tue Mar 24 21:09:53 2015 +0100
> > # Node ID ea4595977e4e882b7a355e7e7430989cee3ea000
> > # Parent  96a2ed2e01ef6033b74becfde0f79d5b61adbf11
> > ui: add config knob for progressive mode
>
> > +        self.progressive = False
>
> This would probable be better as a function of the same name.
>
Good idea, I'll change this.

>
> > @@ -452,6 +456,11 @@
> >          '''tell whether section exists in config.'''
> >          return section in self._data(untrusted)
> >
> > +    def hasconfig(self, section, config, untrusted=False):
> > +        '''tell whether config item exists in config.'''
> > +        return self.has_section(section, untrusted) \
> > +            and config in self._data(untrusted)[section]
> > +
>
> You accidentally grew an unused function?
>
hasconfig is used in all of the following patches that add functionality to
ui.progressive. That's why I put it with the general ui.progressive patch.
I'll move it to the first patch that uses the function instead.

>
> --
> Mathematics is the supreme nostalgia of our time.
>
>
>
Pierre-Yves David - April 4, 2015, 12:30 a.m.
On 04/01/2015 08:16 AM, Mathias De Maré wrote:
> hasconfig is used in all of the following patches that add functionality
> to ui.progressive. That's why I put it with the general ui.progressive
> patch.
> I'll move it to the first patch that uses the function instead.

Or move it to its own patch with a description explaining it will be 
used by the next one.

Patch

diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
--- a/mercurial/help/config.txt
+++ b/mercurial/help/config.txt
@@ -1411,6 +1411,12 @@ 
     If set to ``abort``, the command is aborted.
     On Windows, this configuration option is ignored and the command aborted.
 
+``progressive``
+    Enable user-friendly features. True or False. Default is False;
+    If set to True, specific functionality that improves user experience
+    will be enabled, even if there is no complete backwards compatibility.
+    Scripts should use the environment variable ``HGPLAIN`` to avoid impact.
+
 ``quiet``
     Reduce the amount of output printed. True or False. Default is False.
 
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -79,6 +79,7 @@ 
         # _bufferstates: Should the temporary capture includes stderr
         self._bufferstates = []
         self.quiet = self.verbose = self.debugflag = self.tracebackflag = False
+        self.progressive = False
         self._reportuntrusted = True
         self._ocfg = config.config() # overlay
         self._tcfg = config.config() # trusted
@@ -111,6 +112,8 @@ 
             for f in scmutil.rcpath():
                 self.readconfig(f, trust=True)
 
+        self._fixprogressive(self.progressive)
+
     def copy(self):
         return self.__class__(self)
 
@@ -158,7 +161,7 @@ 
 
         if self.plain():
             for k in ('debug', 'fallbackencoding', 'quiet', 'slash',
-                      'logtemplate', 'style',
+                      'logtemplate', 'progressive', 'style',
                       'traceback', 'verbose'):
                 if k in cfg['ui']:
                     del cfg['ui'][k]
@@ -207,6 +210,7 @@ 
             self._reportuntrusted = self.debugflag or self.configbool("ui",
                 "report_untrusted", True)
             self.tracebackflag = self.configbool('ui', 'traceback', False)
+            self.progressive = self.configbool('ui', 'progressive', False)
 
         if section in (None, 'trusted'):
             # update trust information
@@ -452,6 +456,11 @@ 
         '''tell whether section exists in config.'''
         return section in self._data(untrusted)
 
+    def hasconfig(self, section, config, untrusted=False):
+        '''tell whether config item exists in config.'''
+        return self.has_section(section, untrusted) \
+            and config in self._data(untrusted)[section]
+
     def configitems(self, section, untrusted=False):
         items = self._data(untrusted).items(section)
         if self.debugflag and not untrusted and self._reportuntrusted:
@@ -467,6 +476,10 @@ 
             for name, value in self.configitems(section, untrusted):
                 yield section, name, value
 
+    def _fixprogressive(self, progressive):
+        if not progressive:
+            return
+
     def plain(self, feature=None):
         '''is plain mode active?