[PATCH RFC] color: fix --color=always when output is piped

[Brodie Rao]

On Thu, May 26, 2011 at 1:27 PM, Peter Arrenbrecht
<peter.arrenbrecht at gmail.com> wrote:
> On Thu, May 26, 2011 at 9:57 PM, Matt Mackall <mpm at selenic.com> wrote:
>> On Wed, 2011-05-25 at 18:49 +0200, Peter Arrenbrecht wrote:
>>> # HG changeset patch
>>> # User Peter Arrenbrecht <peter.arrenbrecht at gmail.com>
>>> # Date 1306342080 -7200
>>> # Node ID 30850bc97c5c1f6ba8bb0d07bc0d57fb32a0b1dd
>>> # Parent  9d31f762cdf263329aa80e0b8d9ecc38a2b3bf44
>>> color: fix --color=always when output is piped
>>
>> What was broken?
>>
>> Is this the terminfo vs less thing?
>
> Yes. Now --color=always really does emit colors into the pipe for less.
> -parren

To be clear, this is not related to the issue someone was running into
recently where terminfo mode emitted control codes less couldn't
understand. This just unbreaks --color=always.

The patch looks good to me.

>>> Also restructures the mode determination as discussed on IRC.
>>> I tried to simplify the code too, while keeping the original
>>> semantics except for the case of --color=always.
>>
>>> diff --git a/hgext/color.py b/hgext/color.py
>>> --- a/hgext/color.py
>>> +++ b/hgext/color.py
>>> @@ -114,7 +114,7 @@
>>>              'blue_background': 44, 'purple_background': 45,
>>>              'cyan_background': 46, 'white_background': 47}
>>>
>>> -def _terminfosetup(ui):
>>> +def _terminfosetup(ui, mode):
>>>      '''Initialize terminfo data and the terminal if we're in terminfo mode.'''
>>>
>>>      global _terminfo_params
>>> @@ -122,7 +122,6 @@
>>>      if not _terminfo_params:
>>>          return
>>>      # Otherwise, see what the config file says.
>>> -    mode = ui.config('color', 'mode', 'auto')
>>>      if mode not in ('auto', 'terminfo'):
>>>          return
>>>
>>> @@ -149,6 +148,51 @@
>>>            "ECMA-48 color\n"))
>>>          _terminfo_params = {}
>>>
>>> +def _modesetup(ui, opts):
>>> +    global _terminfo_params
>>> +
>>> +    coloropt = opts['color']
>>> +    auto = coloropt == 'auto'
>>> +    always = not auto and util.parsebool(coloropt)
>>> +    if not always and not auto:
>>> +        return None
>>> +
>>> +    formatted = always or (os.environ.get('TERM') != 'dumb' and ui.formatted())
>>> +
>>> +    mode = ui.config('color', 'mode', 'auto')
>>> +    realmode = mode
>>> +    if mode == 'auto':
>>> +        if os.name == 'nt' and 'TERM' not in os.environ:
>>> +            # looks line a cmd.exe console, use win32 API or nothing
>>> +            realmode = 'win32'
>>> +        elif not formatted:
>>> +            realmode = 'ansi'
>>> +        else:
>>> +            realmode = 'terminfo'
>>> +
>>> +    if realmode == 'win32':
>>> +        if not w32effects and mode == 'win32':
>>> +            # only warn if color.mode is explicitly set to win32
>>> +            ui.warn(_('warning: failed to set color mode to %s\n') % mode)
>>> +            return None
>>> +        _effects.update(w32effects)
>>> +    elif realmode == 'ansi':
>>> +        _terminfo_params = {}
>>> +    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)
>>> +            realmode = 'ansi'
>>> +    else:
>>> +        return None
>>> +
>>> +    if always or (auto and formatted):
>>> +        return realmode
>>> +    return None
>>> +
>>>  try:
>>>      import curses
>>>      # Mapping from effect name to terminfo attribute name or color number.
>>> @@ -301,40 +345,9 @@
>>>      global _terminfo_params
>>>      if ui.plain():
>>>          return
>>> -
>>> -    formatted = (os.environ.get('TERM') != 'dumb' and ui.formatted())
>>> -    mode = ui.config('color', 'mode', 'auto')
>>> -    if mode == 'auto':
>>> -        if os.name == 'nt' and 'TERM' not in os.environ:
>>> -            # looks line a cmd.exe console, use win32 API or nothing
>>> -            mode = w32effects and 'win32' or 'none'
>>> -        else:
>>> -            if not formatted:
>>> -                _terminfo_params = False
>>> -            else:
>>> -                _terminfosetup(ui)
>>> -                if not _terminfo_params:
>>> -                    mode = 'ansi'
>>> -                else:
>>> -                    mode = 'terminfo'
>>> -    if mode == 'win32':
>>> -        if w32effects is None:
>>> -            # only warn if color.mode is explicitly set to win32
>>> -            ui.warn(_('warning: failed to set color mode to %s\n') % mode)
>>> -            return
>>> -        _effects.update(w32effects)
>>> -    elif mode == 'ansi':
>>> -        _terminfo_params = {}
>>> -    elif mode == 'terminfo':
>>> -        _terminfosetup(ui)
>>> -    else:
>>> -        return
>>>      def colorcmd(orig, ui_, opts, cmd, cmdfunc):
>>> -        coloropt = opts['color']
>>> -        auto = coloropt == 'auto'
>>> -        always = util.parsebool(coloropt)
>>> -        if (always or
>>> -            (always is None and auto and formatted)):
>>> +        mode = _modesetup(ui_, opts)
>>> +        if mode:
>>>              colorui._colormode = mode
>>>              colorui.__bases__ = (ui_.__class__,)
>>>              ui_.__class__ = colorui
>>> _______________________________________________
>>> Mercurial-devel mailing list
>>> Mercurial-devel at selenic.com
>>> http://selenic.com/mailman/listinfo/mercurial-devel
>>
>>
>> --
>> Mathematics is the supreme nostalgia of our time.
>>
>>
>>
>