Patchwork [1,of,2,V2] color: consolidate cut-and-paste code

login
register
mail settings
Submitter Jordi Gutiérrez Hermoso
Date March 10, 2015, 8:38 p.m.
Message ID <dd65a862c6a229840830.1426019923@Iris>
Download mbox | patch
Permalink /patch/7979/
State Superseded
Commit e98f2078e4675e1cff89158e13b4b8ac94c41902
Headers show

Comments

Jordi Gutiérrez Hermoso - March 10, 2015, 8:38 p.m.
# HG changeset patch
# User Jordi Gutiérrez Hermoso <jordigh@octave.org>
# Date 1426017448 14400
#      Tue Mar 10 15:57:28 2015 -0400
# Node ID dd65a862c6a2298408304d4eeb17a835b5e9d0e9
# Parent  02d7b5cd373bbb4e8263dad9bfbf9c4c3b0e4e3a
color: consolidate cut-and-paste code

This fixes a mild case of cut-and-paste code regarding failing to set
terminal modes. This is evident in the win32 comment that is misplaced
for the terminfo mode since cset ad6ad51cc0dd.

Instead, we refactor this C&P into a small local function.
Matt Mackall - March 10, 2015, 9:30 p.m.
On Tue, 2015-03-10 at 16:38 -0400, Jordi Gutiérrez Hermoso wrote:
> # HG changeset patch
> # User Jordi Gutiérrez Hermoso <jordigh@octave.org>
> # Date 1426017448 14400
> #      Tue Mar 10 15:57:28 2015 -0400
> # Node ID dd65a862c6a2298408304d4eeb17a835b5e9d0e9
> # Parent  02d7b5cd373bbb4e8263dad9bfbf9c4c3b0e4e3a
> color: consolidate cut-and-paste code
> 
> This fixes a mild case of cut-and-paste code regarding failing to set
> terminal modes. This is evident in the win32 comment that is misplaced
> for the terminfo mode since cset ad6ad51cc0dd.
> 
> Instead, we refactor this C&P into a small local function.
> 
> diff --git a/hgext/color.py b/hgext/color.py
> --- a/hgext/color.py
> +++ b/hgext/color.py
> @@ -251,12 +251,15 @@ def _modesetup(ui, coloropt):
>          else:
>              realmode = 'ansi'
>  
> +    def warn_fail_mode():

Adding more underbars in identifiers -> yeah.. nope.

This is exactly why I took your first patch even after you said you were
preparing a V2. It was fine as-is and there was a non-zero risk I would
have to reject your V2 because you'd needlessly made the bug fix
dependent on something I didn't want, thus delaying the fix.

Patch

diff --git a/hgext/color.py b/hgext/color.py
--- a/hgext/color.py
+++ b/hgext/color.py
@@ -251,12 +251,15 @@  def _modesetup(ui, coloropt):
         else:
             realmode = 'ansi'
 
+    def warn_fail_mode():
+        # only warn if color.mode was explicitly set
+        if mode == realmode:
+            ui.warn(_('warning: failed to set color mode to %s\n') % mode)
+
     if realmode == 'win32':
         _terminfo_params = {}
         if not w32effects:
-            if mode == 'win32':
-                # only warn if color.mode is explicitly set to win32
-                ui.warn(_('warning: failed to set color mode to %s\n') % mode)
+            warn_fail_mode()
             return None
         _effects.update(w32effects)
     elif realmode == 'ansi':
@@ -264,10 +267,8 @@  def _modesetup(ui, coloropt):
     elif realmode == 'terminfo':
         _terminfosetup(ui, mode)
         if not _terminfo_params:
-            if mode == 'terminfo':
-                ## FIXME Shouldn't we return None in this case too?
-                # only warn if color.mode is explicitly set to win32
-                ui.warn(_('warning: failed to set color mode to %s\n') % mode)
+            ## FIXME Shouldn't we return None in this case too?
+            warn_fail_mode()
             realmode = 'ansi'
     else:
         return None