RFC: Coloured output in bazaar core

[Russ Brown]

Martin Pool wrote:
> On Sun, Aug 3, 2008 at 2:54 PM, Russ Brown <pickscrape at gmail.com> wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> I had a discussion with lifeless a few days back about colored output
>> functionality in bazaar core.
>>
>> I have been working on adding colour to the diffstat plugin and wanted
>> to make it work in a way that would be consistent with any future core
>> support for it.
>>
>> The conclusion to the discussion was to start with a single
>> configuration value in bazaar.conf which would enable or disable colour
>> for all supporting commands, defaulting to off:
>>
>> [colour|color] = <Boolean>
> 
> It would be good to also support 'auto', to switch depending on the
> terminal capabilities.
> 

I did have this thought myself, but the general consensus on IRC was
that this should start as simple as possible and build up from there.
Auto should be relatively easy to add after the fact, so I'm not too
concerned about this.

>> I have just added support to the diffstat plugin to support this config
>> entry, though there is a bit more code in there than there would be if
>> support was in core (handling both spellings, parsing the boolean etc),
>> but it does work.
> 
> I'm not so keen on supporting both spellings; it adds more code and
> there is some potential for confusion.  (Are you going to allow both
> spellings in all places?  Does one override the other?)  At any rate
> for the initial merge I would just use the 'color' spelling
> everywhere, which is the standard in technical use.
> 

My original thoughts were just for 'colour' (being English myself), but
others in IRC suggested that both suggestions be acceptable. I can
certainly see the benefits to choosing just one. Talden is right in that
there is a difference between code and the UI, but I'm not sure if it is
worth allowing both in the UI either.

For what it's worth, git's colour configuration and options appear to
use the US spelling exclusively.

>> What would it take to get support for this option added to bzr core?
>> - From my limited experience of bzrlib, it looks to me like the following
>> would be a good start:
>>
>>  * A get_colours_on() method (or similarly named) somewhere in
>>   bzrlib.config which defaults to False. I'm not entirely sure which
>>   class (or classes) this needs adding to.
> 
> I would look at making this part of the UIFactory.
> 

Not something I've come across before: I'll look into it.

>> I think that would be all we would need to get started, but here are a
>> few ideas of additional things that could be added that would enhance
>> the feature further:
>>
>>  * New global options to bzr --no-colour and --no-color which the user
>>   can use to disable colour output temporarily for a single execution
>>   without having to mess with the config file
> 
> It might be clearer to put them just on the particular commands they
> affect?  I suppose many of them could eventually use colored output.
> 

This is what I originally suggested: lifeless seemed a little shocked
that people might want to configure colour to be on for some commands
and off for others. :)

Either way, a global 'color' setting would appear to work everywhere: if
nothing more specific is defined, do what it says. You can then add
options for commands like this:

color.diff = True

Which allows per-command configuration.

>>  * Some library that can be called upon to colour up a chunk of text.
> 
> That does seem pretty important :-)
> 

It does indeed. Diffstat currently includes a very small class that I
knocked up for this purpose. Being able to use one direct from bzrlib
would be ideal.

> Strictly speaking you probably want to use the terminfo library to get
> the escape codes through the terminfo library.  In practice I think
> almost every user will on a terminal that understands the standard
> escape codes (including xterms and the linux and bsd console) and this
> may be an unnecessary delay or complication.  Really you could start
> with either one.
> 
> This is only for Unix; on Windows there is an api to change the text
> color of course but I don't know how you would reach it from Python.
> But probably someone else here would.
> 

Portability is one thing I haven't really thought about: I'll do some
research on this (thanks Jonathan for the Twisted reference).

>> Any comments or additional ideas? If someone gives me a hint on where to
>> add the required code in bzrlib.config I'll submit a patch myself.
> 
> If I was writing this I would start by aiming for immediate
> gratification by just adding the --color option to diff and making
> that produce colored output, then think about how to permanently
> configure it on.  And of course there is the colordiff plugin you can
> look at.
> 

Diff is without doubt the obvious place to start...

Thanks for the comments!

-- 

Russ.