Patchwork [3,of,6] move: move the 'label' method to the core class

login
register
mail settings
Submitter Pierre-Yves David
Date Feb. 22, 2017, 4:54 p.m.
Message ID <9a9e8b28b717e30ed430.1487782474@nodosa.octopoid.net>
Download mbox | patch
Permalink /patch/18717/
State Superseded
Delegated to: Yuya Nishihara
Headers show

Comments

Pierre-Yves David - Feb. 22, 2017, 4:54 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
# Date 1487590284 -3600
#      Mon Feb 20 12:31:24 2017 +0100
# Node ID 9a9e8b28b717e30ed43030cc64f115a4356193de
# Parent  f53e95ec986616a48a3a8e67e5a9674b8a684d6e
# EXP-Topic color
move: move the 'label' method to the core class

We move 'label' method from the custom 'colorui' class to the core 'ui' class.
This bring us closer to supporting color in core natively. Core already have a
'label' method that was a no-op. We update its content with the logic from the
extension. Behavior is unchanged when colormode = None.
Yuya Nishihara - Feb. 23, 2017, 2:09 p.m.
On Wed, 22 Feb 2017 17:54:34 +0100, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
> # Date 1487590284 -3600
> #      Mon Feb 20 12:31:24 2017 +0100
> # Node ID 9a9e8b28b717e30ed43030cc64f115a4356193de
> # Parent  f53e95ec986616a48a3a8e67e5a9674b8a684d6e
> # EXP-Topic color
> move: move the 'label' method to the core class
> 
> We move 'label' method from the custom 'colorui' class to the core 'ui' class.
> This bring us closer to supporting color in core natively. Core already have a
> 'label' method that was a no-op. We update its content with the logic from the
> extension. Behavior is unchanged when colormode = None.

It's nice to move color functions into core and get rid of the colorui class,
but I also think we should avoid making ui.py bloated. So, instead of moving
everything into ui.py, can we put them in color.py as module-level functions?

For instance,

  def label(self, msg, label):
      if self._colormode:
          msg = color.labeledstr(self._colormode, msg, label)
      return msg

  def write(self, *args, **opts):
      ...
      if self._colormode:
          color.writelabeled(self._colormode, self.fout, *args, **opts)
Pierre-Yves David - Feb. 24, 2017, 4:34 p.m.
On 02/23/2017 03:09 PM, Yuya Nishihara wrote:
> On Wed, 22 Feb 2017 17:54:34 +0100, Pierre-Yves David wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
>> # Date 1487590284 -3600
>> #      Mon Feb 20 12:31:24 2017 +0100
>> # Node ID 9a9e8b28b717e30ed43030cc64f115a4356193de
>> # Parent  f53e95ec986616a48a3a8e67e5a9674b8a684d6e
>> # EXP-Topic color
>> move: move the 'label' method to the core class
>>
>> We move 'label' method from the custom 'colorui' class to the core 'ui' class.
>> This bring us closer to supporting color in core natively. Core already have a
>> 'label' method that was a no-op. We update its content with the logic from the
>> extension. Behavior is unchanged when colormode = None.
>
> It's nice to move color functions into core and get rid of the colorui class,
> but I also think we should avoid making ui.py bloated. So, instead of moving
> everything into ui.py, can we put them in color.py as module-level functions?

That make sense, I've moved the label computation in the 'color' module 
since it make heavy refence of color specific parts. On the other hand, 
the new "write" part are not too color specific. They only add reference 
and call to "label", something already part of the top level color API 
so I think i make sense to keep it here.

Finally, poking at turning the unix part into the same thing as win32 
made me realize that may translation was wrote, win32 really need to 
access ui.write* , not just ui.f*.write. V2 will extract the lower level 
function writing to the stream so that we can reuse it properly.

Cheers,

>
> For instance,
>
>   def label(self, msg, label):
>       if self._colormode:
>           msg = color.labeledstr(self._colormode, msg, label)
>       return msg
>
>   def write(self, *args, **opts):
>       ...
>       if self._colormode:
>           color.writelabeled(self._colormode, self.fout, *args, **opts)
>

Patch

diff -r f53e95ec9866 -r 9a9e8b28b717 hgext/color.py
--- a/hgext/color.py	Mon Feb 20 12:13:23 2017 +0100
+++ b/hgext/color.py	Mon Feb 20 12:31:24 2017 +0100
@@ -328,32 +328,6 @@  class colorui(uimod.ui):
             return super(colorui, self).write_err(
                 *[self.label(a, label) for a in args], **opts)
 
-    def label(self, msg, label):
-        if self._colormode is None:
-            return super(colorui, self).label(msg, label)
-
-        if self._colormode == 'debug':
-            if label and msg:
-                if msg[-1] == '\n':
-                    return "[%s|%s]\n" % (label, msg[:-1])
-                else:
-                    return "[%s|%s]" % (label, msg)
-            else:
-                return msg
-
-        effects = []
-        for l in label.split():
-            s = color._styles.get(l, '')
-            if s:
-                effects.append(s)
-            elif color.valideffect(l):
-                effects.append(l)
-        effects = ' '.join(effects)
-        if effects:
-            return '\n'.join([color._render_effects(line, effects)
-                              for line in msg.split('\n')])
-        return msg
-
 def uisetup(ui):
     if ui.plain():
         return
diff -r f53e95ec9866 -r 9a9e8b28b717 mercurial/ui.py
--- a/mercurial/ui.py	Mon Feb 20 12:13:23 2017 +0100
+++ b/mercurial/ui.py	Mon Feb 20 12:31:24 2017 +0100
@@ -26,6 +26,7 @@  from .i18n import _
 from .node import hex
 
 from . import (
+    color,
     config,
     encoding,
     error,
@@ -1361,13 +1362,31 @@  class ui(object):
     def label(self, msg, label):
         '''style msg based on supplied label
 
-        Like ui.write(), this just returns msg unchanged, but extensions
-        and GUI tools can override it to allow styling output without
-        writing it.
+        If some color mode is enabled, this will added the necessary control
+        character to apply such color. In addition 'debug' color mode adds
+        markup showing which label affect a piece of text.
 
         ui.write(s, 'label') is equivalent to
         ui.write(ui.label(s, 'label')).
         '''
+        if self._colormode == 'debug':
+            if label and msg:
+                if msg[-1] == '\n':
+                    msg = "[%s|%s]\n" % (label, msg[:-1])
+                else:
+                    msg = "[%s|%s]" % (label, msg)
+        elif self._colormode is not None:
+            effects = []
+            for l in label.split():
+                s = color._styles.get(l, '')
+                if s:
+                    effects.append(s)
+                elif color.valideffect(l):
+                    effects.append(l)
+            effects = ' '.join(effects)
+            if effects:
+                msg = '\n'.join([color._render_effects(line, effects)
+                                 for line in msg.split('\n')])
         return msg
 
     def develwarn(self, msg, stacklevel=1, config=None):