Patchwork [V3] ui: add new config flag for interface selection

login
register
mail settings
Submitter Laurent Charignon
Date Feb. 5, 2016, 4:17 p.m.
Message ID <ac519fea89b3e1a5c1ed.1454689020@dev5073.prn1.facebook.com>
Download mbox | patch
Permalink /patch/13007/
State Superseded
Delegated to: Augie Fackler
Headers show

Comments

Laurent Charignon - Feb. 5, 2016, 4:17 p.m.
# HG changeset patch
# User Laurent Charignon <lcharignon@fb.com>
# Date 1454688960 28800
#      Fri Feb 05 08:16:00 2016 -0800
# Node ID ac519fea89b3e1a5c1edafd403fb78fe229d0a11
# Parent  01a5143cd25f285f8c745a92986cd7186bb32c90
ui: add new config flag for interface selection

This patch introduces a new config flag ui.interface to select the interface
for interactive commands. It currently only applies to chunks selection.
The config can be overridden on a per feature basis with the flag
ui.interface.<feature>.

features for the moment can only be 'chunkselector', moving forward we expect
to have 'histedit' and other commands there.

If an incorrect value is given to ui.interface we print a warning and use the
default interface: text. If HGPLAIN is specified we also use the default
interface: text.
Augie Fackler - Feb. 5, 2016, 9:08 p.m.
On Fri, Feb 05, 2016 at 08:17:00AM -0800, Laurent Charignon wrote:
> # HG changeset patch
> # User Laurent Charignon <lcharignon@fb.com>
> # Date 1454688960 28800
> #      Fri Feb 05 08:16:00 2016 -0800
> # Node ID ac519fea89b3e1a5c1edafd403fb78fe229d0a11
> # Parent  01a5143cd25f285f8c745a92986cd7186bb32c90
> ui: add new config flag for interface selection
>
> This patch introduces a new config flag ui.interface to select the interface
> for interactive commands. It currently only applies to chunks selection.
> The config can be overridden on a per feature basis with the flag
> ui.interface.<feature>.
>
> features for the moment can only be 'chunkselector', moving forward we expect
> to have 'histedit' and other commands there.
>
> If an incorrect value is given to ui.interface we print a warning and use the
> default interface: text. If HGPLAIN is specified we also use the default
> interface: text.
>
> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
> --- a/mercurial/help/config.txt
> +++ b/mercurial/help/config.txt
> @@ -1587,6 +1587,17 @@
>  ``interactive``
>      Allow to prompt the user. (default: True)
>
> +``interface``
> +    Select the default interface for interactive features (default: text).
> +    Possible values are 'text' and 'curses'.
> +
> +``interface.chunkselector``
> +    Select the interface for chunkselection.
> +    Possible values are 'text' and 'crecord'.

Why isn't 'curses' a valid option here? I'm fine with advertising
crecord, but if you really mean it that picking curses means "best
available curses UI and we'll keep crecord forever even if it becomes
horrible by comparison", then you should be able to say
ui.interface=text and ui.interface.chunkselector=curses and it should
behave like you'd expect.

(I'm still intensely skeptical of the notion that we'd ever ship
/multiple/ curses chunkselectors, but I'm not willing to die on that
hill.)

> +    This config overrides the interface specified by ui.interface.
> +    If ui.interface and ui.interface.chunkselector aren't set, we use the text
> +    interface.
> +
>  ``logtemplate``
>      Template string for commands that print changesets.
>
> diff --git a/mercurial/ui.py b/mercurial/ui.py
> --- a/mercurial/ui.py
> +++ b/mercurial/ui.py
> @@ -699,6 +699,63 @@
>              return False
>          return util.isatty(fh)
>
> +    def interface(self, feature):
> +        '''what interface to use for interactive console features?
> +
> +        The interface is controlled by the value of `ui.interface` but also by
> +        the value of feature-specific configuration. For example:
> +
> +        ui.interface.histedit = text
> +        ui.interface.chunkselector = crecord
> +
> +        Here the features are "histedit" and "chunkselector".
> +
> +        The configuration above means that the default interfaces for commands
> +        is curses, the interface for histedit is text and the interface for
> +        selecting chunk is crecord (a curses interface).
> +
> +        Consider the following exemple:
> +        ui.interface = curses
> +        ui.interface.histedit = text
> +
> +        Then histedit will use the text interface and chunkselector will use
> +        the default curses interface (crecord at the moment).
> +        '''
> +        if self.plain():
> +            return "text"
> +
> +        alloweddefaultinterfaces = ("text", "curses")
> +        defaultfeaturesinterfaces = {
> +            "chunkselector": {
> +                "text": "text",
> +                "curses": "crecord"
> +            }
> +        }
> +
> +        # Default interface
> +        defaultinterface = "text"
> +        i = self.config("ui", "interface", None)
> +        if i is not None:
> +            if i not in alloweddefaultinterfaces:
> +                self.warn(_("invalid value for ui.interface: %s") % i)
> +                self.warn(_("(using default interface instead %s)")
> +                          % defaultinterface)
> +            else:
> +                defaultinterface = i
> +
> +        # Feature-specific interface
> +        if feature not in defaultfeaturesinterfaces.keys():
> +            # This is a programming error, not a user facing error, it is
> +            # ok to Abort here as we should never hit this case
> +            raise ValueError("Unknown feature requested %s" % feature)
> +
> +        f = self.config("ui", "interface.%s" % feature, None)
> +        if f is None:
> +            return defaultfeaturesinterfaces[feature][defaultinterface]
> +        else:
> +            return f
> +
> +
>      def interactive(self):
>          '''is interactive input allowed?
>
> diff --git a/tests/test-commit-interactive-curses.t b/tests/test-commit-interactive-curses.t
> --- a/tests/test-commit-interactive-curses.t
> +++ b/tests/test-commit-interactive-curses.t
> @@ -223,3 +223,34 @@
>    hello world
>
>
> +Check ui.interface logic for the chunkselector
> +
> +The default interface is text
> +  $ chunkselectorinterface() {
> +  > python <<EOF
> +  > from mercurial import hg, ui, parsers;\
> +  > repo = hg.repository(ui.ui(), ".");\
> +  > print repo.ui.interface("chunkselector")
> +  > EOF
> +  > }
> +  $ chunkselectorinterface
> +  text
> +
> +The default curses interface is crecord
> +  $ cat <<EOF >> $HGRCPATH
> +  > [ui]
> +  > interface = curses
> +  > EOF
> +  $ chunkselectorinterface
> +  crecord
> +
> +It is possible to override the default interface with a feature specific
> +interface
> +  $ cat <<EOF >> $HGRCPATH
> +  > [ui]
> +  > interface = text
> +  > interface.chunkselector = crecord
> +  > EOF
> +
> +  $ chunkselectorinterface
> +  crecord
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Pierre-Yves David - Feb. 10, 2016, 1:35 p.m.
On 02/05/2016 09:08 PM, Augie Fackler wrote:
> On Fri, Feb 05, 2016 at 08:17:00AM -0800, Laurent Charignon wrote:
>> # HG changeset patch
>> # User Laurent Charignon <lcharignon@fb.com>
>> # Date 1454688960 28800
>> #      Fri Feb 05 08:16:00 2016 -0800
>> # Node ID ac519fea89b3e1a5c1edafd403fb78fe229d0a11
>> # Parent  01a5143cd25f285f8c745a92986cd7186bb32c90
>> ui: add new config flag for interface selection
>>
>> This patch introduces a new config flag ui.interface to select the interface
>> for interactive commands. It currently only applies to chunks selection.
>> The config can be overridden on a per feature basis with the flag
>> ui.interface.<feature>.
>>
>> features for the moment can only be 'chunkselector', moving forward we expect
>> to have 'histedit' and other commands there.
>>
>> If an incorrect value is given to ui.interface we print a warning and use the
>> default interface: text. If HGPLAIN is specified we also use the default
>> interface: text.
>>
>> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
>> --- a/mercurial/help/config.txt
>> +++ b/mercurial/help/config.txt
>> @@ -1587,6 +1587,17 @@
>>   ``interactive``
>>       Allow to prompt the user. (default: True)
>>
>> +``interface``
>> +    Select the default interface for interactive features (default: text).
>> +    Possible values are 'text' and 'curses'.
>> +
>> +``interface.chunkselector``
>> +    Select the interface for chunkselection.
>> +    Possible values are 'text' and 'crecord'.
>
> Why isn't 'curses' a valid option here? I'm fine with advertising
> crecord, but if you really mean it that picking curses means "best
> available curses UI and we'll keep crecord forever even if it becomes
> horrible by comparison", then you should be able to say
> ui.interface=text and ui.interface.chunkselector=curses and it should
> behave like you'd expect.
>
> (I'm still intensely skeptical of the notion that we'd ever ship
> /multiple/ curses chunkselectors, but I'm not willing to die on that
> hill.)

+1 on having "curse" as the primary option value.

It is also consistent with possible value for 'ui.interface'

Patch

diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
--- a/mercurial/help/config.txt
+++ b/mercurial/help/config.txt
@@ -1587,6 +1587,17 @@ 
 ``interactive``
     Allow to prompt the user. (default: True)
 
+``interface``
+    Select the default interface for interactive features (default: text).
+    Possible values are 'text' and 'curses'.
+
+``interface.chunkselector``
+    Select the interface for chunkselection.
+    Possible values are 'text' and 'crecord'.
+    This config overrides the interface specified by ui.interface.
+    If ui.interface and ui.interface.chunkselector aren't set, we use the text
+    interface.
+
 ``logtemplate``
     Template string for commands that print changesets.
 
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -699,6 +699,63 @@ 
             return False
         return util.isatty(fh)
 
+    def interface(self, feature):
+        '''what interface to use for interactive console features?
+
+        The interface is controlled by the value of `ui.interface` but also by
+        the value of feature-specific configuration. For example:
+
+        ui.interface.histedit = text
+        ui.interface.chunkselector = crecord
+
+        Here the features are "histedit" and "chunkselector".
+
+        The configuration above means that the default interfaces for commands
+        is curses, the interface for histedit is text and the interface for
+        selecting chunk is crecord (a curses interface).
+
+        Consider the following exemple:
+        ui.interface = curses
+        ui.interface.histedit = text
+
+        Then histedit will use the text interface and chunkselector will use
+        the default curses interface (crecord at the moment).
+        '''
+        if self.plain():
+            return "text"
+
+        alloweddefaultinterfaces = ("text", "curses")
+        defaultfeaturesinterfaces = {
+            "chunkselector": {
+                "text": "text",
+                "curses": "crecord"
+            }
+        }
+
+        # Default interface
+        defaultinterface = "text"
+        i = self.config("ui", "interface", None)
+        if i is not None:
+            if i not in alloweddefaultinterfaces:
+                self.warn(_("invalid value for ui.interface: %s") % i)
+                self.warn(_("(using default interface instead %s)")
+                          % defaultinterface)
+            else:
+                defaultinterface = i
+
+        # Feature-specific interface
+        if feature not in defaultfeaturesinterfaces.keys():
+            # This is a programming error, not a user facing error, it is
+            # ok to Abort here as we should never hit this case
+            raise ValueError("Unknown feature requested %s" % feature)
+
+        f = self.config("ui", "interface.%s" % feature, None)
+        if f is None:
+            return defaultfeaturesinterfaces[feature][defaultinterface]
+        else:
+            return f
+
+
     def interactive(self):
         '''is interactive input allowed?
 
diff --git a/tests/test-commit-interactive-curses.t b/tests/test-commit-interactive-curses.t
--- a/tests/test-commit-interactive-curses.t
+++ b/tests/test-commit-interactive-curses.t
@@ -223,3 +223,34 @@ 
   hello world
 
 
+Check ui.interface logic for the chunkselector
+
+The default interface is text
+  $ chunkselectorinterface() {
+  > python <<EOF
+  > from mercurial import hg, ui, parsers;\
+  > repo = hg.repository(ui.ui(), ".");\
+  > print repo.ui.interface("chunkselector")
+  > EOF
+  > }
+  $ chunkselectorinterface
+  text
+
+The default curses interface is crecord
+  $ cat <<EOF >> $HGRCPATH
+  > [ui]
+  > interface = curses
+  > EOF
+  $ chunkselectorinterface
+  crecord
+
+It is possible to override the default interface with a feature specific
+interface
+  $ cat <<EOF >> $HGRCPATH
+  > [ui]
+  > interface = text
+  > interface.chunkselector = crecord
+  > EOF
+
+  $ chunkselectorinterface
+  crecord