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 Todd,

On Mon, 7 May 2018, Todd Zullinger wrote:

> Johannes Schindelin wrote:
> > 
> > On Sat, 5 May 2018, Todd Zullinger 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?
> > 
> > This code is not querying diff.color.frag, it is setting it. Without
> > any way to override it.
> > 
> > Having thought about it longer, and triggered by Peff's suggestion to
> > decouple the "reverse" part from the actual color, I fixed this by
> > 
> > - *not* setting .frag to magenta,
> > 
> > - using the reverse method also to mark outer *hunk headers* (not only
> > the outer -/+ markers).
> > 
> > - actually calling git_diff_ui_config()...
> 
> Excellent.  That seems to work nicely now, respecting the
> color.diff.<slot> config.
> 
> > The current work in progress can be pulled as `branch-diff` from
> > https://github.com/dscho/git, if I could ask you to test?
> 
> While the colors and 'branch --diff' usage seem to work
> nicely, I found that with 4ac3413cc8 ("branch-diff: left-pad
> patch numbers", 2018-05-05), 'git branch' itself is broken.
> 
> Running 'git branch' creates a branch named 'branch'.
> Calling 'git branch --list' shows only 'branch' as the only
> branch.
> 
> I didn't look too closely, but I'm guessing that the argv
> handling is leaving the 'branch' argument in place where it
> should be stripped?
> 
> This unsurprisingly breaks a large number of tests. :)

You will be delighted to learn that all of this is now moot, as I renamed
the command to `range-diff`, as this is what the wisdom of the crowd
chose.

Ciao,
Johannes



[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