Message ID | 2823d4d360b2f53c1d63ba742808eaee80ede1e4.1571110286.git.gitgitgadget@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | diff-highlight: fix a whitespace nit | expand |
On Tue, Oct 15, 2019 at 03:31:26AM +0000, Norman Rasmussen via GitGitGadget wrote: > From: Norman Rasmussen <norman@rasmussen.co.za> > > This changes the indent from > "<tab><sp><sp><sp><sp><sp><sp><sp><sp>" > to > "<tab><tab>" > so that the statement lines up with the rest of the block. Yep, that makes sense. Looks like I introduced the problem (most of my perl used to be written in a style that forbids tabs, so it may have snuck in that way, but the rest of the file definitely follows Git's usual style of tabs). > diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm > index 7440aa1c46..e2589922a6 100644 > --- a/contrib/diff-highlight/DiffHighlight.pm > +++ b/contrib/diff-highlight/DiffHighlight.pm > @@ -72,7 +72,7 @@ sub handle_line { > (?:$COLOR?\|$COLOR?[ ])* # zero or more trailing "|" > [ ]* # trailing whitespace for merges > /x) { > - my $graph_prefix = $&; > + my $graph_prefix = $&; There are a few lines just above that have 8+ spaces. Arguably those could be tabs, too, depending on your view of tabs. We usually do "8 spaces is a tab" in the Git project, but the oft-repeated "tabs to indent, spaces to align" mantra would apply here (and I suspect you're using a different tabwidth since you noticed this one case). So I'd just as soon leave them be, and take your patch as-is. -Peff
On Mon, Oct 14, 2019 at 9:20 PM Jeff King <peff@peff.net> wrote: > There are a few lines just above that have 8+ spaces. Arguably those > could be tabs, too, depending on your view of tabs. We usually do "8 > spaces is a tab" in the Git project, but the oft-repeated "tabs to > indent, spaces to align" mantra would apply here (and I suspect you're > using a different tabwidth since you noticed this one case). So I'd just > as soon leave them be, and take your patch as-is. Yep, the lines above are using the spaces to align the sections of the multi-line if statement. This happens again for the return statements in highlight_pair and is_pair_interesting. So this is the only line that doesn't stick to the rule (and probably because of editor auto-indenting). I have another change for the same change (which I'll send once I've written tests) and I only noticed this line when I changed my editor's default tabwidth after a while of coding.
diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm index 7440aa1c46..e2589922a6 100644 --- a/contrib/diff-highlight/DiffHighlight.pm +++ b/contrib/diff-highlight/DiffHighlight.pm @@ -72,7 +72,7 @@ sub handle_line { (?:$COLOR?\|$COLOR?[ ])* # zero or more trailing "|" [ ]* # trailing whitespace for merges /x) { - my $graph_prefix = $&; + my $graph_prefix = $&; # We must flush before setting graph indent, since the # new commit may be indented differently from what we