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
