[PATCH 3 of 3] [RFC] dispatch: setup color before pager

Thu Mar 23 23:11:02 EDT 2017

On Tue, 21 Mar 2017 23:10:36 -0400, Gregory Szorc  
<gregory.szorc at gmail.com> wrote:

> On Tue, Mar 21, 2017 at 6:11 PM, Matt Harbison <mharbison72 at gmail.com>
> wrote:
>
>> (+CC: indygreg, since he added this functionality in 4e02418b4236)
>>
>> On Tue, 21 Mar 2017 17:36:29 -0400, Augie Fackler <raf at durin42.com>  
>> wrote:
>>
>> On Mon, Mar 20, 2017 at 12:53:19AM -0400, Matt Harbison wrote:
>>>
>>>> # HG changeset patch
>>>> # User Matt Harbison <matt_harbison at yahoo.com>
>>>> # Date 1489985511 14400
>>>> #      Mon Mar 20 00:51:51 2017 -0400
>>>> # Node ID 2a323109f8e7ec5548dd506c2cff05f27fa20c1e
>>>> # Parent  6e72bb689e57c985b138c580acb4cad8053dc180
>>>> [RFC] dispatch: setup color before pager
>>>>
>>>
>>> Oy, these two I just dunno. I'll defer to other Windows experts.
>>>
>>
>> Heh. You have no idea how sad my Sunday was :-/
>>
>> Just to be clear, these last two can't be taken as-is.  The output on
>> Windows without a pager is black text on a green background.
>>
>> I did a little more playing, and if we don't do something, this will be  
>> a
>> regression.  In 4.1, both of these commands colorize in MSYS:
>>
>> [extensions]
>> color =
>>
>> [pager]
>> pager = less
>>
>> $ hg log -l 5 --config extensions.pager= --config color.pagermode=ansi
>>
>> $ hg log -l 5 --config color.pagermode=ansi
>>
>>
>> Now, neither of them do (both are now paged).  You have to set
>> '--pager=no' to get the color back, or add '--config color.mode=ansi'.   
>> But
>> the latter breaks the output when disabling pager.  And even if that
>> magically worked, that doesn't help with switching between MSYS and
>> cmd.exe, which is why I'd love to make 'auto' smarter.  (I'd rather not
>> resort to %include hacks).
>>
>
> So the whole reason behind 4e02418b4236 (as documented in the commit
> message) is that different pagers support different escape sequences. At
> the time, I couldn't figure out an elegant way to detect the pager's
> capabilities at run time. So, I added a config option that could be  
> defined
> in certain environments (like a controlled msys environment where `less`
> was installed as the pager such as the one that Mozilla uses to build
> Firefox) so that pager+color would "just work" in both cmd.exe and an  
> msys
> environment. More info at
> https://bugzilla.mozilla.org/show_bug.cgi?id=545432#c59 and below (search
> for my name).
>
> Core Mercurial should focus on behavior in a stock cmd.exe environment  
> and
> be best-effort - but not broken - for cygwin, msys, etc. This is what
> a78888d98606 aimed to achieve. If someone is running a non-cmd.exe
> terminal, I think it is up to the packager to enable a reasonable default
> (such as Mozilla setting color.pagermode in our MozillaBuild msys
> environment).

That seems reasonable.  But color.pagermode seems broken for auto pager,  
now that everything has transitioned to the core.  It works on stable (if  
the color/pager load order is right).

4.1:

1) Color wrapped dispatch._runcommand() and pager wrapped color's wrapper.
2) Running a command will:
    a) Call the pager wrapper, set ui.pageractive, and spawn the pager if
       True
    b) Call the color wrapper, and examine ui.pageractive to see if it cares
       about color.pagermode
    c) Call the actual command

Default branch:

1) Pager settings are forced on or off in dispatch.  '--pager=on' fires up
    a pager; 'auto' pager mode defers spawning a pager.
2) Color is setup (examining ui.pageractive), and color.mode=auto is
    resolved
3) Run the command
    a) spawn a pager the pager, setting ui.pageractive=True

I think this means that the process of spawning the pager needs to be able  
to call color.setup() again, so that it can reexamine ui.pageractive.   
Otherwise, the only way 'ui.pageractive' is ever true when color is  
getting setup is if --pager != 'auto|off'.  IDK the implications of a  
second setup, and I'm perplexed by all of the various ui objects that  
exist.  I didn't check yet to see if there's any possibility to output  
before the calls to ui.pager() now.  (I wonder if there are instances in  
largefiles with all of its overrides.)

Thoughts?

> Also, if you want a new challenge, the Windows console apparently  
> supports
> 24-bit color (
> https://blogs.msdn.microsoft.com/commandline/2016/09/22/24-bit-color-in-the-windows-console/)
> and ANSI+VT-100 escape sequences now.
> https://github.com/symfony/symfony/pull/18385 and
> https://github.com/Microsoft/BashOnWindows/issues/1173 are somewhat
> informative. We could likely detect the Windows version and active  
> terminal
> to call SetConsoleMode with ENABLE_VIRTUAL_TERMINAL_PROCESSING. Then
> pager.color=ansi should "just work" on Windows.

Thanks for the tip- I forgot about this.  I'm not on Windows 10, so it  
isn't a high priority for me, but I'll look into it if I can find some  
spare time.

>>
>> IIRC, the inter-operation between these two extensions was why the  
>> ability
>> to register an afterload callback was added.  I'll keep looking at this,
>> but I suspect we need a color+pager expert more than a Windows expert.
>
>
> I think that afterload callback was always buggy and should be fixed or
> removed :/
>
>
>>
>>
>>
>>>> With the previous order, color was None for commands invoked with
>>>> --pager=yes at
>>>> the time the pager was spawned, which meant the switch over to ANSI  
>>>> mode
>>>> on
>>>> Windows didn't occur.  That left the color.win32print() in 'auto'  
>>>> mode,
>>>> which
>>>> meant no color in the pager output.  Non Windows platforms are  
>>>> capable of
>>>> printing color with this option.
>>>>
>>>> diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
>>>> --- a/mercurial/dispatch.py
>>>> +++ b/mercurial/dispatch.py
>>>> @@ -760,6 +760,13 @@
>>>>              for ui_ in uis:
>>>>                  ui_.setconfig('ui', 'interactive', 'off', '-y')
>>>>
>>>> +        # setup color handling
>>>> +        coloropt = options['color']
>>>> +        for ui_ in uis:
>>>> +            if coloropt:
>>>> +                ui_.setconfig('ui', 'color', coloropt, '--color')
>>>> +            color.setup(ui_)
>>>> +
>>>>          if util.parsebool(options['pager']):
>>>>              ui.pager('internal-always-' + cmd)
>>>>          elif options['pager'] != 'auto':
>>>> @@ -769,13 +776,6 @@
>>>>              for ui_ in uis:
>>>>                  ui_.insecureconnections = True
>>>>
>>>> -        # setup color handling
>>>> -        coloropt = options['color']
>>>> -        for ui_ in uis:
>>>> -            if coloropt:
>>>> -                ui_.setconfig('ui', 'color', coloropt, '--color')
>>>> -            color.setup(ui_)
>>>> -
>>>>          if options['version']:
>>>>              return commands.version_(ui)
>>>>          if options['help']:
>>>> _______________________________________________
>>>> Mercurial-devel mailing list
>>>> Mercurial-devel at mercurial-scm.org
>>>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>>>>