On Sun, May 11, 2014 at 04:09:56PM +0000, Ævar Arnfjörð Bjarmason wrote:
> Change the display of hunks in hunk splitting mode to preserve the diff
> heading, which hasn't been done ever since the hunk splitting was
> initially added in v1.4.4.2-270-g835b2ae.
>
> Splitting the first hunk of this patch will now result in:
>
> Stage this hunk [y,n,q,a,d,/,j,J,g,s,e,?]? s
> Split into 2 hunks.
> @@ -792,7 +792,7 @@ sub hunk_splittable {
> [...]
>
> Instead of:
>
> Stage this hunk [y,n,q,a,d,/,j,J,g,s,e,?]? s
> Split into 2 hunks.
> @@ -792,7 +792,7 @@
> [...]
>
> This makes it easier to use the tool when you're splitting some giant
> hunk and can't remember in which function you are anymore.
This makes a lot of sense to me. I did notice two interesting quirks,
one of which might be worth addressing.
One, there is a slightly funny artifact in that the hunk header comes
from the top of the context line, and that top is a different position
for each of the split hunks. So in a file like:
header_A
content
header_B
one
two
three
four
you might have a diff like:
@@ ... @@ header_A
header_B
one
two
+ new line 1
three
+ new line 2
four
The hunk header for "new line 1" is "A", because "B" itself is part of
the context. But the hunk header for "new line 2", if it were an
independent hunk, would be "B". We print "A" because we copy it from the
original hunk.
It probably won't matter much in practice (and I can even see an
argument that "A" is the "right" answer). And figuring out "B" here
would be prohibitively difficult, I would think, as it would require
applying the funcname rules internal to git-diff to a hunk that git-diff
itself never actually sees.
Since the output from your patch is strictly better than what we saw
before, I think there is no reason we cannot leave such an improvement
to later (or never).
> The diff is somewhat larger than I initially expected because in order
> to display the headings in the same color scheme as the output from
> git-diff(1) itself I had to split up the code that would previously
> color diff output that previously consisted entirely of the fraginfo,
> but now consists of the fraginfo and the diff heading (the latter of
> which isn't colored).
The func heading is not colored by default, but you can configure it to
be so with color.diff.func. I double-checked the behavior with your
patch: you end up with the uncolored header in the split hunks, because
it is parsed from the uncolored line. Which is not bad, but I think we
can trivially do better, just by adding back in the color as we do with
the fraginfo.
Like:
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index ed1e564..ac5763d 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -29,6 +29,10 @@ my ($fraginfo_color) =
$diff_use_color ? (
$repo->get_color('color.diff.frag', 'cyan'),
) : ();
+my ($funcname_color) =
+ $diff_use_color ? (
+ $repo->get_color('color.diff.func', ''),
+ ) : ();
my ($diff_plain_color) =
$diff_use_color ? (
$repo->get_color('color.diff.plain', ''),
@@ -902,7 +906,7 @@ sub split_hunk {
unshift @{$hunk->{DISPLAY}}, join(
"",
$diff_use_color ? colored($fraginfo_color, $fraginfo) : $fraginfo,
- $heading,
+ $diff_use_color ? colored($funcname_color, $heading) : $heading,
"\n"
);
}
I didn't prepare a commit message because I think it should probably
just be squashed in.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html