{"thread":{"id":"52037","subject":"[PATCH 0/1] diff-highlight: fix a whitespace nit","startedAt":"2019-10-15T03:31:32Z","lastAt":"2019-10-15T04:35:11Z","messageCount":4,"participants":["Norman Rasmussen via GitGitGadget","Jeff King","Norman Rasmussen"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"384062","messageId":"pull.397.git.1571110286.gitgitgadget@gmail.com","threadId":"52037","inReplyTo":null,"subject":"[PATCH 0/1] diff-highlight: fix a whitespace nit","fromName":"Norman Rasmussen via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-15T03:31:25Z","receivedAt":"2019-10-15T03:31:32Z","isPatch":true,"sender":{"key":"norman@rasmussen.co.za","avatar":"https://avatars.githubusercontent.com/u/14849?v=4"},"body":"This changes the indent from\n \"\"\nto\n \"\"\nso that the statement lines up with the rest of the block.\n\nSigned-off-by: Norman Rasmussen \n\nNorman Rasmussen (1):\n diff-highlight: fix a whitespace nit\n\n contrib/diff-highlight/DiffHighlight.pm | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\n\nbase-commit: 08da6496b61341ec45eac36afcc8f94242763468\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-397%2Fnormanr%2Fdiff-highlight-whitespace-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-397/normanr/diff-highlight-whitespace-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/397\n-- \ngitgitgadget\n"},{"id":"384063","messageId":"2823d4d360b2f53c1d63ba742808eaee80ede1e4.1571110286.git.gitgitgadget@gmail.com","threadId":"52037","inReplyTo":"pull.397.git.1571110286.gitgitgadget@gmail.com","subject":"[PATCH 1/1] diff-highlight: fix a whitespace nit","fromName":"Norman Rasmussen via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-15T03:31:26Z","receivedAt":"2019-10-15T03:31:36Z","isPatch":true,"sender":{"key":"norman@rasmussen.co.za","avatar":"https://avatars.githubusercontent.com/u/14849?v=4"},"body":"From: Norman Rasmussen \n\nThis changes the indent from\n \"\"\nto\n \"\"\nso that the statement lines up with the rest of the block.\n\nSigned-off-by: Norman Rasmussen \n---\n contrib/diff-highlight/DiffHighlight.pm | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm\nindex 7440aa1c46..e2589922a6 100644\n--- a/contrib/diff-highlight/DiffHighlight.pm\n+++ b/contrib/diff-highlight/DiffHighlight.pm\n@@ -72,7 +72,7 @@ sub handle_line {\n \t (?:$COLOR?\\|$COLOR?[ ])* # zero or more trailing \"|\"\n \t [ ]* # trailing whitespace for merges\n \t /x) {\n-\t my $graph_prefix = $&;\n+\t\tmy $graph_prefix = $&;\n \n \t\t# We must flush before setting graph indent, since the\n \t\t# new commit may be indented differently from what we\n-- \ngitgitgadget\n"},{"id":"384065","messageId":"20191015042006.GC32308@sigill.intra.peff.net","threadId":"52037","inReplyTo":"2823d4d360b2f53c1d63ba742808eaee80ede1e4.1571110286.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] diff-highlight: fix a whitespace nit","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-10-15T04:20:07Z","receivedAt":"2019-10-15T04:20:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 15, 2019 at 03:31:26AM +0000, Norman Rasmussen via GitGitGadget wrote:\n\n> From: Norman Rasmussen \n> \n> This changes the indent from\n> \"\"\n> to\n> \"\"\n> so that the statement lines up with the rest of the block.\n\nYep, that makes sense. Looks like I introduced the problem (most of my\nperl used to be written in a style that forbids tabs, so it may have\nsnuck in that way, but the rest of the file definitely follows Git's\nusual style of tabs).\n\n> diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm\n> index 7440aa1c46..e2589922a6 100644\n> --- a/contrib/diff-highlight/DiffHighlight.pm\n> +++ b/contrib/diff-highlight/DiffHighlight.pm\n> @@ -72,7 +72,7 @@ sub handle_line {\n> \t (?:$COLOR?\\|$COLOR?[ ])* # zero or more trailing \"|\"\n> \t [ ]* # trailing whitespace for merges\n> \t /x) {\n> -\t my $graph_prefix = $&;\n> +\t\tmy $graph_prefix = $&;\n\nThere are a few lines just above that have 8+ spaces. Arguably those\ncould be tabs, too, depending on your view of tabs. We usually do \"8\nspaces is a tab\" in the Git project, but the oft-repeated \"tabs to\nindent, spaces to align\" mantra would apply here (and I suspect you're\nusing a different tabwidth since you noticed this one case). So I'd just\nas soon leave them be, and take your patch as-is.\n\n-Peff\n"},{"id":"384066","messageId":"CAGF1phZk9_9Se++beXxDxQZ+Y6ucrFnBWJTVJfATMYyk4=Yn=Q@mail.gmail.com","threadId":"52037","inReplyTo":"20191015042006.GC32308@sigill.intra.peff.net","subject":"Re: [PATCH 1/1] diff-highlight: fix a whitespace nit","fromName":"Norman Rasmussen","fromEmail":"norman@rasmussen.co.za","sentAt":"2019-10-15T04:34:55Z","receivedAt":"2019-10-15T04:35:11Z","isPatch":true,"sender":{"key":"norman@rasmussen.co.za","avatar":"https://avatars.githubusercontent.com/u/14849?v=4"},"body":"On Mon, Oct 14, 2019 at 9:20 PM Jeff King wrote:\n> There are a few lines just above that have 8+ spaces. Arguably those\n> could be tabs, too, depending on your view of tabs. We usually do \"8\n> spaces is a tab\" in the Git project, but the oft-repeated \"tabs to\n> indent, spaces to align\" mantra would apply here (and I suspect you're\n> using a different tabwidth since you noticed this one case). So I'd just\n> as soon leave them be, and take your patch as-is.\n\nYep, the lines above are using the spaces to align the sections of the\nmulti-line if statement. This happens again for the return statements\nin highlight_pair and is_pair_interesting. So this is the only line\nthat doesn't stick to the rule (and probably because of editor\nauto-indenting). I have another change for the same change (which I'll\nsend once I've written tests) and I only noticed this line when I\nchanged my editor's default tabwidth after a while of coding.\n\n-- \n- Norman Rasmussen\n - Email: norman@rasmussen.co.za\n - Home page: http://norman.rasmussen.co.za/\n"}]}