{"thread":{"id":"52534","subject":"[PATCH 0/1] diff-highlight: highlight range-diff","startedAt":"2019-12-29T15:49:55Z","lastAt":"2020-01-28T07:52:16Z","messageCount":3,"participants":["Jack Bates via GitGitGadget","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"389033","messageId":"pull.684.git.git.1577634590.gitgitgadget@gmail.com","threadId":"52534","inReplyTo":null,"subject":"[PATCH 0/1] diff-highlight: highlight range-diff","fromName":"Jack Bates via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-29T15:49:49Z","receivedAt":"2019-12-29T15:49:55Z","isPatch":true,"sender":{"key":"bk874k@nottheoilrig.com","avatar":null},"body":"Piping git range-diff through diff-highlight currently has no effect, for\ntwo reasons:\n\n 1. There are ANSI escapes before and after the @@ hunk headers (when color\n is enabled) which diff-highlight fails to match. One solution is to\n match both escapes (/^$COLOR*\\@\\@$COLOR* /). This patch drops the\n trailing space from the existing pattern instead.\n \n \n 2. Unlike git log, git range-diff diffs are indented, which diff-highlight\n also fails to match. This patch allows hunk headers preceded by any\n amount of whitespace, and then skips past that indentation when parsing\n subsequent lines, by reusing the machinery that handles the --graph\n output.\n \n \n\nSigned-off-by: Jack Bates jack@nottheoilrig.com [jack@nottheoilrig.com]\n\nJack Bates (1):\n diff-highlight: highlight range-diff\n\n contrib/diff-highlight/DiffHighlight.pm | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\n\nbase-commit: 0a76bd7381ec0dbb7c43776eb6d1ac906bca29e6\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-684%2Fjablko%2Fpatch-1-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-684/jablko/patch-1-v1\nPull-Request: https://github.com/git/git/pull/684\n-- \ngitgitgadget\n"},{"id":"389034","messageId":"547b3087ac2e8e05af86b4b9972cef8c20da1cdf.1577634590.git.gitgitgadget@gmail.com","threadId":"52534","inReplyTo":"pull.684.git.git.1577634590.gitgitgadget@gmail.com","subject":"[PATCH 1/1] diff-highlight: highlight range-diff","fromName":"Jack Bates via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-29T15:49:50Z","receivedAt":"2019-12-29T15:49:58Z","isPatch":true,"sender":{"key":"bk874k@nottheoilrig.com","avatar":null},"body":"From: Jack Bates \n\nPiping `git range-diff` through diff-highlight currently has no effect,\nfor two reasons:\n\n 1. There are ANSI escapes before and after the `@@` hunk headers (when\n color is enabled) which diff-highlight fails to match. One solution\n is to match both escapes (`/^$COLOR*\\@\\@$COLOR* /`). This patch\n drops the trailing space from the existing pattern instead.\n\n 2. Unlike `git log`, `git range-diff` diffs are indented, which\n diff-highlight also fails to match. This patch allows hunk headers\n preceded by any amount of whitespace, and then skips past that\n indentation when parsing subsequent lines, by reusing the machinery\n that handles the --graph output.\n\nSigned-off-by: Jack Bates \n---\n contrib/diff-highlight/DiffHighlight.pm | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm\nindex e2589922a6..74f53e53c9 100644\n--- a/contrib/diff-highlight/DiffHighlight.pm\n+++ b/contrib/diff-highlight/DiffHighlight.pm\n@@ -90,7 +90,8 @@ sub handle_line {\n \n \tif (!$in_hunk) {\n \t\t$line_cb->($orig);\n-\t\t$in_hunk = /^$COLOR*\\@\\@ /;\n+\t\t$in_hunk = /^( *)$COLOR*\\@\\@/;\n+\t\t$graph_indent = length($1);\n \t}\n \telsif (/^$COLOR*-/) {\n \t\tpush @removed, $orig;\n-- \ngitgitgadget\n"},{"id":"390614","messageId":"20200128075214.GB564804@coredump.intra.peff.net","threadId":"52534","inReplyTo":"547b3087ac2e8e05af86b4b9972cef8c20da1cdf.1577634590.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] diff-highlight: highlight range-diff","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-28T07:52:14Z","receivedAt":"2020-01-28T07:52:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Dec 29, 2019 at 03:49:50PM +0000, Jack Bates via GitGitGadget wrote:\n\n> From: Jack Bates \n> \n> Piping `git range-diff` through diff-highlight currently has no effect,\n> for two reasons:\n\nSorry for the slow review; this got overlooked over the holidays.\n\n> 1. There are ANSI escapes before and after the `@@` hunk headers (when\n> color is enabled) which diff-highlight fails to match. One solution\n> is to match both escapes (`/^$COLOR*\\@\\@$COLOR* /`). This patch\n> drops the trailing space from the existing pattern instead.\n\nHmm. Matching $COLOR on either side would be stricter. In particular,\nwith your patch I think we'd match \"@@@\", undoing 3dbfe2b8ae\n(diff-highlight: avoid highlighting combined diffs, 2016-08-31).\n\n> 2. Unlike `git log`, `git range-diff` diffs are indented, which\n> diff-highlight also fails to match. This patch allows hunk headers\n> preceded by any amount of whitespace, and then skips past that\n> indentation when parsing subsequent lines, by reusing the machinery\n> that handles the --graph output.\n\nThis also means we'd start trying to highlight diffs that are inside\ncommit messages. That might not be the end of the world.\n\nYou can see some examples in git.git by doing:\n\n git log | /path/to/original/diff-highlight >old\n git log | /path/to/your/new/diff-highlight >new\n diff -u old new\n\n> diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm\n> index e2589922a6..74f53e53c9 100644\n> --- a/contrib/diff-highlight/DiffHighlight.pm\n> +++ b/contrib/diff-highlight/DiffHighlight.pm\n> @@ -90,7 +90,8 @@ sub handle_line {\n> \n> \tif (!$in_hunk) {\n> \t\t$line_cb->($orig);\n> -\t\t$in_hunk = /^$COLOR*\\@\\@ /;\n> +\t\t$in_hunk = /^( *)$COLOR*\\@\\@/;\n\nThere's a similar regex a few lines below to decide of we should remain\nin a hunk. Would you need to modify that, too?\n\n> +\t\t$graph_indent = length($1);\n\nThis throws away the existing $graph_indent. I know we wouldn't have a\nrange-diff mixed with a graph, but I think it breaks normal graph usage.\nAt least \"make test\" in contrib/diff-highlight seems to complain, and\nadding \"--graph -p\" to the \"git log\" invocations above shows that we're\nnot highlighting a bunch of cases (perhaps all?).\n\n-Peff\n"}]}