RFC: Coloured output in bazaar core

Mon Aug 4 13:39:40 BST 2008

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I would just argue that it changes the command line options as well.
Specifically, the standard options are usually:

foo --color=never/always/auto

Which is different from

foo --color
foo --no-color

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

As does "ls --color" it seems grep allows both --color and --colour.

The biggest problem is that if you make it a *branch* config, then you
can override it in a per-branch config. Which runs into problems of
having "color=always" in branch.conf, and "colour=never" in branch.conf.
ConfigObj is also helpful about indicating when you have the same option
written twice, but can't be of help when they are spelled differently.

As for the Bazaar internals, they should all be "get_color", I believe I
remember Martin indicating that US spelling is preferred for all internals.

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

I would recommend using "always/never/auto" rather than True/False,
because that seems to be common in a lot of other commands. You don't
have to *implement* auto yet, but using True/False doesn't quite match
'auto'.
Especially because if you start with True/False you are likely to be
treating it as a boolean internally. (You'll certainly want a function
that ends up returning a boolean, though. So maybe it will just be the
layering.)

I do agree that you are unlikely to want color for some commands, but
not others. Though, of course "bzr diff --no-color > file.txt". (Which
is why --color=auto is nicer.)

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

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkiW+IwACgkQJdeBCYSNAAP/xwCgxUWWxZS8AJ3nEDp5MKvBWCH4
a6cAoNaOE7t7YPsg5RMnUl/wbCo0ZalM
=3wv6
-----END PGP SIGNATURE-----