Re: [PATCH v2 12/18] branch-diff: use color for the commit pairs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Johannes,

As many others have already said, thanks for this series!
I've used tbdiff a bit over the years, but having a builtin
will make it much more convenient (and the speed boost from
a C implementation will be a very nice bonus).

Johannes Schindelin wrote:
> @@ -430,6 +451,8 @@ int cmd_branch_diff(int argc, const char **argv, const char *prefix)
>  	struct string_list branch1 = STRING_LIST_INIT_DUP;
>  	struct string_list branch2 = STRING_LIST_INIT_DUP;
>  
> +	git_diff_basic_config("diff.color.frag", "magenta", NULL);
> +
>  	diff_setup(&diffopt);
>  	diffopt.output_format = DIFF_FORMAT_PATCH;
>  	diffopt.flags.suppress_diff_headers = 1;

Should this also (or only) check color.diff.frag?  I thought
that color.diff.* was preferred over diff.color.*, though
that doesn't seem to be entirely true in all parts of the
current codebase.

In testing this series it seems that setting color.diff
options to change the various colors read earlier in this
patch via diff_get_color_opt, as well as the 'frag' slot,
are ignored.  Setting them via diff.color.<slot> does work.

The later patch adding a man page documents branch-diff as
using `diff.color.*` and points to git-config(1), but the
config docs only list color.diff.

Is this a bug in the diff_get_color{,_opt}() tooling?
It's certainly not anything you've introduced here, of
course.  I just noticed that some custom color.diff settings
I've used weren't picked up by branch-diff, despite your
clear intention to respect colors from the config.

-- 
Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Abandon the search for Truth; settle for a good fantasy.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux