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
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