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

Hi Johannes,

Johannes Schindelin wrote:

Hi Todd,

On Sat, 5 May 2018, Todd Zullinger wrote:

quoted

quoted

@@ -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. :)

Thanks,

-- 
Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
A common mistake people make when trying to design something
completely foolproof is to underestimate the ingenuity of complete
fools.
    -- Douglas Adams