[REVIEW] Add basic --color support to bzr diff
Russ Brown
pickscrape at gmail.com
Sat Aug 9 21:47:59 BST 2008
Aaron Bentley wrote:
> Russ Brown wrote:
>> I've reached a point where colour output is working, and before I
>> continue I'd like some feedback to make sure I am on the right track.
>
> Why have you chosen to reimplement color diff rather than adapting the
> code from bzrtools cdiff?
>
I think I probably titled this email incorrectly and gave the wrong
impression. The primary goal of this endeavour is to add a useful
framework for coloured output to bzr in the generic sense, so it can be
used by all commands and plugins equally. The diff command was just a
starting point: I could have started with status, for example. My goal
is not simply to replace cdiff.
I actually did use some of the code from cdiff, but much of it is very
much geared to colouring diff specifically, while this patch is focused
on adding colour support to bzr core that can be easily used from any
command.
> The result is less featureful, since it doesn't allow color
> customization, doesn't support style checks, and doesn't auto-detect
> color support.
>
First of all, I'm not finished yet. This is just a preview so I can get
some feedback before I continue (or just junk what I've done).
Particularly, I commented about the current lack of colour customisation
in the code and the email, and also the platform and capability
detection. I was in fact going to make use of cdiff's colour support
detection and also add win32 support too. Again, this isn't finished.
Furthermore, in the discussions I've had about this on the mailing list
and IRC I was told to not worry about things such as configuration and
auto detection at this stage: just get the basics in place first, which
is what I am doing.
Style checks are beyond the scope of adding coloured output. This isn't
about putting cdiff in core, but adding command colour support to core.
> If you're going to replace cdiff, please replace it with something
> *better*, not worse.
>
That's hardly a fair comparison: this is incomplete code.
>> I have gone with adding the --color option to diff: I agree with the
>> discussion about always/auto/never, but I think that is more applicable
>> to the config file side of things than the command line options.
>
> We usually configure commands using aliases, so only existing
> commandline options are typically used to configure commands.
>
Fair enough. In that case I'll allow --color to take always/auto/never.
> Aaron
More information about the bazaar
mailing list