{"thread":{"id":"46971","subject":"Consider escaping special characters like 'less' does","startedAt":"2017-10-15T13:33:53Z","lastAt":"2017-10-23T00:45:30Z","messageCount":13,"participants":["Joris Valette","Jason Pyeron","Andreas Schwab","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"330399","messageId":"CA+o7MD8bhXWxy2M7z++jrq+k85SwzdEV6uwnUjV9=0_X99=VJg@mail.gmail.com","threadId":"46971","inReplyTo":null,"subject":"Consider escaping special characters like 'less' does","fromName":"Joris Valette","fromEmail":"joris.valette@gmail.com","sentAt":"2017-10-15T13:33:46Z","receivedAt":"2017-10-15T13:33:53Z","isPatch":false,"sender":{"key":"joris.valette@gmail.com","avatar":"https://gravatar.com/avatar/4eaf46dfa8e42ded42748c64f439042397acf0136495c374ce9951a2ad5cbce6?d=mp&s=160"},"body":"The pager 'less' escapes some characters when calling 'git diff'. This\nis what I might get:\n\n$ git diff --cached\ndiff --git a/some_file b/some_file\nnew file mode 100644\nindex 0000000..357323f\n--- /dev/null\n+++ b/some_file\n@@ -0,0 +1 @@\n+Hello\n\\ No newline at end of file\n\nThis example is a simple file encoded in UTF-8 *with BOM*.\nOn the other hand, the built-in git output shows this:\n\n$ git --no-pager diff --cached\ndiff --git a/some_file b/some_file\nnew file mode 100644\nindex 0000000..357323f\n--- /dev/null\n+++ b/some_file\n@@ -0,0 +1 @@\n+Hello\n\\ No newline at end of file\n\n\n\n\nYou can see 'less' shows explicitly, while it is implicit and\nhidden with git's built-in output.\nOther characters behave the same, like ZERO WIDTH SPACE , and\nI believe many others.\nThis is particularly annoying when there are other changes on the same\nline. Here I add ' world!' but also remove the BOM:\n\n$ git diff\ndiff --git a/some_file b/some_file\nindex 357323f..6769dd6 100644\n--- a/some_file\n+++ b/some_file\n@@ -1 +1 @@\n-Hello\n\\ No newline at end of file\n+Hello world!\n\\ No newline at end of file\n\nCompare with:\n\n$ git --no-pager diff\ndiff --git a/some_file b/some_file\nindex 357323f..6769dd6 100644\n--- a/some_file\n+++ b/some_file\n@@ -1 +1 @@\n-Hello\n\\ No newline at end of file\n+Hello world!\n\\ No newline at end of file\n\n\n\n\nIn the first example I can see both changes, in the second example the\nBOM removal is hidden and I won't notice it because there is indeed\nanother genuine change on the same line.\n\nI'll add a third example with CRLF line-terminators:\n\n$ git diff\ndiff --git a/some_file b/some_file\nindex 357323f..dfd6895 100644\n--- a/some_file\n+++ b/some_file\n@@ -1 +1 @@\n-Hello\n\\ No newline at end of file\n+Hello world!^M\n\nand:\n\n$ git --no-pager diff\ndiff --git a/some_file b/some_file\nindex 357323f..dfd6895 100644\n--- a/some_file\n+++ b/some_file\n@@ -1 +1 @@\n-Hello\n\\ No newline at end of file\n+Hello world!\n\n\n\n\nMy thoughts are that git should add this feature and tend towards\nless's behavior. A diff output should show as much as possible.\nI would happily use 'less' all the time, but unfortunately I don't\nthink I can. For instance, 'git add --patch', which I use quite often,\nprints through git's built-in output, and I miss less's features.\n\nJoris Valette.\n"},{"id":"330402","messageId":"773EDE3B5FDB48AD8FB0DB048CAEC8DE@blackfat","threadId":"46971","inReplyTo":"CA+o7MD8bhXWxy2M7z++jrq+k85SwzdEV6uwnUjV9=0_X99=VJg@mail.gmail.com","subject":"RE: Consider escaping special characters like 'less' does","fromName":"Jason Pyeron","fromEmail":"jpyeron@pdinc.us","sentAt":"2017-10-15T14:15:52Z","receivedAt":"2017-10-15T15:19:15Z","isPatch":false,"sender":{"key":"jpyeron@pdinc.us","avatar":"https://gravatar.com/avatar/c2e53452caa53d940768a1ffc9cf76196d851b9b534b7a39cd39852a70a0508f?d=mp&s=160"},"body":"> -----Original Message-----\n> From: Joris Valette\n> Sent: Sunday, October 15, 2017 9:34 AM\n> To: git@vger.kernel.org\n> Subject: Consider escaping special characters like 'less' does\n> \n> The pager 'less' escapes some characters when calling 'git diff'. This\n> is what I might get:\n> \n> $ git diff --cached\n> diff --git a/some_file b/some_file\n> new file mode 100644\n> index 0000000..357323f\n> --- /dev/null\n> +++ b/some_file\n> @@ -0,0 +1 @@\n> +Hello\n> \\ No newline at end of file\n> \n> This example is a simple file encoded in UTF-8 *with BOM*.\n> On the other hand, the built-in git output shows this:\n> \n> $ git --no-pager diff --cached\n> diff --git a/some_file b/some_file\n> new file mode 100644\n> index 0000000..357323f\n> --- /dev/null\n> +++ b/some_file\n> @@ -0,0 +1 @@\n> +?Hello\n> \\ No newline at end of file\n\nIt is your terminal, not git's fault that you get a ? rendered.\n\n$ git diff\ndiff --git a/test.txt b/test.txt\nindex af9f93a..294e397 100644\n--- a/test.txt\n+++ b/test.txt\n@@ -1 +1 @@\n-the quick brown fox jumps over the lazy dog\n+the quick brown fox jumps over the lazy dog\n\n$ git diff | hexdump.exe -C\n00000000 64 69 66 66 20 2d 2d 67 69 74 20 61 2f 74 65 73 |diff --git a/tes|\n00000010 74 2e 74 78 74 20 62 2f 74 65 73 74 2e 74 78 74 |t.txt b/test.txt|\n00000020 0a 69 6e 64 65 78 20 61 66 39 66 39 33 61 2e 2e |.index af9f93a..|\n00000030 32 39 34 65 33 39 37 20 31 30 30 36 34 34 0a 2d |294e397 100644.-|\n00000040 2d 2d 20 61 2f 74 65 73 74 2e 74 78 74 0a 2b 2b |-- a/test.txt.++|\n00000050 2b 20 62 2f 74 65 73 74 2e 74 78 74 0a 40 40 20 |+ b/test.txt.@@ |\n00000060 2d 31 20 2b 31 20 40 40 0a 2d 74 68 65 20 71 75 |-1 +1 @@.-the qu|\n00000070 69 63 6b 20 62 72 6f 77 6e 20 66 6f 78 20 6a 75 |ick brown fox ju|\n00000080 6d 70 73 20 6f 76 65 72 20 74 68 65 20 6c 61 7a |mps over the laz|\n\nNote: 0a is newline, 2b is + ef bb bf are the specials added to the file, 74 68 65 20 71 75 is 'the qu'\n\n00000090 79 20 64 6f 67 0a 2b ef bb bf 74 68 65 20 71 75 |y dog.+...the qu|\n000000a0 69 63 6b 20 62 72 6f 77 6e 20 66 6f 78 20 6a 75 |ick brown fox ju|\n000000b0 6d 70 73 20 6f 76 65 72 20 74 68 65 20 6c 61 7a |mps over the laz|\n000000c0 79 20 64 6f 67 0a |y dog.|\n000000c6\n\n$ git diff | cat\ndiff --git a/test.txt b/test.txt\nindex af9f93a..294e397 100644\n--- a/test.txt\n+++ b/test.txt\n@@ -1 +1 @@\n-the quick brown fox jumps over the lazy dog\n+the quick brown fox jumps over the lazy dog\n\n$ git --no-pager diff\ndiff --git a/test.txt b/test.txt\nindex af9f93a..294e397 100644\n--- a/test.txt\n+++ b/test.txt\n@@ -1 +1 @@\n-the quick brown fox jumps over the lazy dog\n+the quick brown fox jumps over the lazy dog\n\nAnd my UTF-8 capable terminal displays it fine.\n\nWhat do you get when you pipe to hexdump?\n\n-Jason\n\n"},{"id":"330404","messageId":"87zi8sxvkg.fsf@linux-m68k.org","threadId":"46971","inReplyTo":"773EDE3B5FDB48AD8FB0DB048CAEC8DE@blackfat","subject":"Re: Consider escaping special characters like 'less' does","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2017-10-15T15:46:55Z","receivedAt":"2017-10-15T15:47:04Z","isPatch":false,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"On Okt 15 2017, \"Jason Pyeron\" wrote:\n\n>> -----Original Message-----\n>> From: Joris Valette\n>> Sent: Sunday, October 15, 2017 9:34 AM\n>> To: git@vger.kernel.org\n>> Subject: Consider escaping special characters like 'less' does\n>> \n>> The pager 'less' escapes some characters when calling 'git diff'. This\n>> is what I might get:\n>> \n>> $ git diff --cached\n>> diff --git a/some_file b/some_file\n>> new file mode 100644\n>> index 0000000..357323f\n>> --- /dev/null\n>> +++ b/some_file\n>> @@ -0,0 +1 @@\n>> +Hello\n>> \\ No newline at end of file\n>> \n>> This example is a simple file encoded in UTF-8 *with BOM*.\n>> On the other hand, the built-in git output shows this:\n>> \n>> $ git --no-pager diff --cached\n>> diff --git a/some_file b/some_file\n>> new file mode 100644\n>> index 0000000..357323f\n>> --- /dev/null\n>> +++ b/some_file\n>> @@ -0,0 +1 @@\n>> +?Hello\n>> \\ No newline at end of file\n>\n> It is your terminal, not git's fault that you get a ? rendered.\n\nIt's your MUA's fault that you get a ?, the mail didn't contain any.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"330407","messageId":"CA+o7MD8fcD5-SFeQsX84Etu68ov7yy48OO4dV=gXMp5xY8s9Rg@mail.gmail.com","threadId":"46971","inReplyTo":"87zi8sxvkg.fsf@linux-m68k.org","subject":"Re: Consider escaping special characters like 'less' does","fromName":"Joris Valette","fromEmail":"joris.valette@gmail.com","sentAt":"2017-10-15T16:39:45Z","receivedAt":"2017-10-15T16:39:52Z","isPatch":false,"sender":{"key":"joris.valette@gmail.com","avatar":"https://gravatar.com/avatar/4eaf46dfa8e42ded42748c64f439042397acf0136495c374ce9951a2ad5cbce6?d=mp&s=160"},"body":"2017-10-15 17:46 GMT+02:00 Andreas Schwab :\n> On Okt 15 2017, \"Jason Pyeron\" wrote:\n>\n>>> -----Original Message-----\n>>> From: Joris Valette\n>>> Sent: Sunday, October 15, 2017 9:34 AM\n>>> To: git@vger.kernel.org\n>>> Subject: Consider escaping special characters like 'less' does\n>>>\n>>> The pager 'less' escapes some characters when calling 'git diff'. This\n>>> is what I might get:\n>>>\n>>> $ git diff --cached\n>>> diff --git a/some_file b/some_file\n>>> new file mode 100644\n>>> index 0000000..357323f\n>>> --- /dev/null\n>>> +++ b/some_file\n>>> @@ -0,0 +1 @@\n>>> +Hello\n>>> \\ No newline at end of file\n>>>\n>>> This example is a simple file encoded in UTF-8 *with BOM*.\n>>> On the other hand, the built-in git output shows this:\n>>>\n>>> $ git --no-pager diff --cached\n>>> diff --git a/some_file b/some_file\n>>> new file mode 100644\n>>> index 0000000..357323f\n>>> --- /dev/null\n>>> +++ b/some_file\n>>> @@ -0,0 +1 @@\n>>> +?Hello\n>>> \\ No newline at end of file\n>>\n>> It is your terminal, not git's fault that you get a ? rendered.\n>\n> It's your MUA's fault that you get a ?, the mail didn't contain any.\n\nActually, the original mail contained the special BOM sequence but\nit's generally invisible. His MUA shows it with a '?', mine doesn't\nshow anything, neither does Firefox on the mailing list page.\n\nThe question remains: could escaping be done?\n\nJoris\n"},{"id":"330410","messageId":"20171015200635.e2u4qkxlz2xwpfov@sigill.intra.peff.net","threadId":"46971","inReplyTo":"CA+o7MD8fcD5-SFeQsX84Etu68ov7yy48OO4dV=gXMp5xY8s9Rg@mail.gmail.com","subject":"Re: Consider escaping special characters like 'less' does","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-15T20:06:35Z","receivedAt":"2017-10-15T20:06:52Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Oct 15, 2017 at 06:39:45PM +0200, Joris Valette wrote:\n\n> > It's your MUA's fault that you get a ?, the mail didn't contain any.\n> \n> Actually, the original mail contained the special BOM sequence but\n> it's generally invisible. His MUA shows it with a '?', mine doesn't\n> show anything, neither does Firefox on the mailing list page.\n> \n> The question remains: could escaping be done?\n\nGit's diff generation will never do such escaping by default, because it\nmeans creating a patch that cannot be applied to get back the original\ncontent.\n\nThere _are_ already options to create diffs that cannot be applied, like\n--textconv. So it would be possible to add a similar option for\nescaping. But I don't think we really need or want a separate option,\nwhen you can already do one of:\n\n 1. If your files have special binary characters that are hard to see,\n you can use the existing textconv system to do whatever escaping\n you like. And then the Git will diff the result of the escaping,\n which means you get readable diffs when they change.\n\n 2. Put the raw output of git's diff through a filter that escapes. We\n already do this most of the time by piping through less. The most\n noticeable exception is \"add --patch\". There you can set up a\n program to filter as well. There's more information in a recent\n thread here:\n\n https://public-inbox.org/git/20171012184736.rglkbyryauwuvn2a@sigill.intra.peff.net/\n\nIt doesn't seem out of the question to me to have an out-of-the-box\ndefault for interactive.diffFilter which does some basic escaping (we\ncould even implement it inside the perl script for efficiency).\n\n-Peff\n"},{"id":"330411","messageId":"CA+o7MD84O5uTOz0xAGmd=xL+Hw8UsVCu5v_HEEeMWvAhSRQCeA@mail.gmail.com","threadId":"46971","inReplyTo":"20171015200635.e2u4qkxlz2xwpfov@sigill.intra.peff.net","subject":"Re: Consider escaping special characters like 'less' does","fromName":"Joris Valette","fromEmail":"joris.valette@gmail.com","sentAt":"2017-10-15T21:37:04Z","receivedAt":"2017-10-15T21:37:10Z","isPatch":false,"sender":{"key":"joris.valette@gmail.com","avatar":"https://gravatar.com/avatar/4eaf46dfa8e42ded42748c64f439042397acf0136495c374ce9951a2ad5cbce6?d=mp&s=160"},"body":"2017-10-15 22:06 GMT+02:00 Jeff King :\n> Git's diff generation will never do such escaping by default, because it\n> means creating a patch that cannot be applied to get back the original\n> content.\n\nIndeed this would be a problem. That's where my knowledge of git's\nsource code ends, but in that case, can't the output be discriminated\nagainst the command that was executed?\nCommand that outputs an applicable patch -> don't escape\nCommand that outputs a diff to see changes -> escape\n\n> There _are_ already options to create diffs that cannot be applied, like\n> --textconv. So it would be possible to add a similar option for\n> escaping. But I don't think we really need or want a separate option,\n> when you can already do one of:\n>\n> 1. If your files have special binary characters that are hard to see,\n> you can use the existing textconv system to do whatever escaping\n> you like. And then the Git will diff the result of the escaping,\n> which means you get readable diffs when they change.\n>\n> 2. Put the raw output of git's diff through a filter that escapes. We\n> already do this most of the time by piping through less. The most\n> noticeable exception is \"add --patch\". There you can set up a\n> program to filter as well. There's more information in a recent\n> thread here:\n>\n> https://public-inbox.org/git/20171012184736.rglkbyryauwuvn2a@sigill.intra.peff.net/\n>\n> It doesn't seem out of the question to me to have an out-of-the-box\n> default for interactive.diffFilter which does some basic escaping (we\n> could even implement it inside the perl script for efficiency).\n\nYes I read this thread, but I was left unsatisfied because I would\nlike something out-of-the-box.\nYour suggestion might be the best solution then: implement some\ndefault escaping for interactive.diffFilter.\n\nJoris\n"},{"id":"330484","messageId":"20171016221324.stn56gqnliunvcdv@sigill.intra.peff.net","threadId":"46971","inReplyTo":"CA+o7MD84O5uTOz0xAGmd=xL+Hw8UsVCu5v_HEEeMWvAhSRQCeA@mail.gmail.com","subject":"Re: Consider escaping special characters like 'less' does","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-16T22:13:24Z","receivedAt":"2017-10-16T22:13:32Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Oct 15, 2017 at 11:37:04PM +0200, Joris Valette wrote:\n\n> 2017-10-15 22:06 GMT+02:00 Jeff King :\n> > Git's diff generation will never do such escaping by default, because it\n> > means creating a patch that cannot be applied to get back the original\n> > content.\n> \n> Indeed this would be a problem. That's where my knowledge of git's\n> source code ends, but in that case, can't the output be discriminated\n> against the command that was executed?\n> Command that outputs an applicable patch -> don't escape\n> Command that outputs a diff to see changes -> escape\n\nSpeaking in a general sense, people use \"git diff\" for both purposes,\nand we can't necessarily tell which[1]. As a matter of fact, that's a\npotential problem with textconv filters as well (which are enabled by\ndefault for git-diff, but not for plumbing like diff-tree, format-patch,\netc). I think the feature isn't widely used enough for people to run\ninto problems (and they also use it only on things that they don't\n_expect_ to be able to make patches for, since they're binaries).\n\n[1] Of course we can come up with heuristics, like \"is stdout going to a\n a terminal\"? But in such a case we already kick in the pager, and it\n does the exact escaping you're asking for. :)\n\nFor a command like add--interactive, it knows which invocations are for\nshowing to the user and which are for applying (and it already uses\n\"--color\" selectively, for instance). So if there were an \"escape this\"\noption in git's diff generation, we could selectively pass it. But\nagain, I think the right solution there is not building the escaping\ninto Git, but just passing it through another filter.\n\n> > It doesn't seem out of the question to me to have an out-of-the-box\n> > default for interactive.diffFilter which does some basic escaping (we\n> > could even implement it inside the perl script for efficiency).\n> \n> Yes I read this thread, but I was left unsatisfied because I would\n> like something out-of-the-box.\n> Your suggestion might be the best solution then: implement some\n> default escaping for interactive.diffFilter.\n\nAlternatively, I suppose we could just always escape in\nadd--interactive. I'm trying to think of a case where somebody would\nreally want their diffFilter to see the raw bytes (or vice versa, to\nshow raw bytes produced by their filter), but I'm having trouble coming\nup with one.\n\nI.e., something like this would probably help your case without hurting\nanybody:\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 28b325d754..d44e5ea459 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -714,6 +714,16 @@ sub parse_diff {\n \t\tpush @{$hunk[-1]{DISPLAY}},\n \t\t\t(@colored ? $colored[$i] : $diff[$i]);\n \t}\n+\n+\tforeach my $hunk (@hunk) {\n+\t\tforeach my $line (@{$hunk->{DISPLAY}}) {\n+\t\t\t# all control chars minus newline and ESC (for color)\n+\t\t\tif ($line =~ s/[\\000-\\011\\013-\\032\\034-\\037]/?/g) {\n+\t\t\t\t$hunk->{CONTROLCHARS} = 1;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n \treturn @hunk;\n }\n \n@@ -1407,6 +1417,9 @@ sub patch_update_file {\n \t\tif ($hunk[$ix]{TYPE} eq 'hunk') {\n \t\t\t$other .= ',e';\n \t\t}\n+\t\tif ($hunk[$ix]->{CONTROLCHARS}) {\n+\t\t\tprint \"warning: control characters in hunk have been replaced by '?'\\n\";\n+\t\t}\n \t\tfor (@{$hunk[$ix]{DISPLAY}}) {\n \t\t\tprint;\n \t\t}\n\nI can't help but feel this is the tip of a larger iceberg, though. E.g.,\nwhat about characters outside of the terminal's correct encoding? Or\nbroken UTF-8 characters?\n\n-Peff\n"},{"id":"330487","messageId":"87po9m3e3e.fsf@linux-m68k.org","threadId":"46971","inReplyTo":"20171016221324.stn56gqnliunvcdv@sigill.intra.peff.net","subject":"Re: Consider escaping special characters like 'less' does","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2017-10-16T22:47:01Z","receivedAt":"2017-10-16T22:47:10Z","isPatch":false,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"On Okt 16 2017, Jeff King wrote:\n\n> I can't help but feel this is the tip of a larger iceberg, though. E.g.,\n> what about characters outside of the terminal's correct encoding? Or\n> broken UTF-8 characters?\n\nOr correctly encoded UTF-8 characters that look confusing? Or blobs\nwith embedded escape sequences?\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"330489","messageId":"20171016225816.mowrldmfkx6q7ape@sigill.intra.peff.net","threadId":"46971","inReplyTo":"87po9m3e3e.fsf@linux-m68k.org","subject":"Re: Consider escaping special characters like 'less' does","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-16T22:58:16Z","receivedAt":"2017-10-16T22:58:28Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 17, 2017 at 12:47:01AM +0200, Andreas Schwab wrote:\n\n> On Okt 16 2017, Jeff King wrote:\n> \n> > I can't help but feel this is the tip of a larger iceberg, though. E.g.,\n> > what about characters outside of the terminal's correct encoding? Or\n> > broken UTF-8 characters?\n> \n> Or correctly encoded UTF-8 characters that look confusing? Or blobs\n> with embedded escape sequences?\n\nYes, leaving ESC unfiltered here made me hesitate, too. We must allow it\nto show colors, but showing diffs with raw terminal codes can be quite\nconfusing.\n\nMy general advice on committing unprintable characters is: don't.\n\n-Peff\n"},{"id":"330497","messageId":"xmqqk1zur2yp.fsf@gitster.mtv.corp.google.com","threadId":"46971","inReplyTo":"20171016221324.stn56gqnliunvcdv@sigill.intra.peff.net","subject":"Re: Consider escaping special characters like 'less' does","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-17T01:13:34Z","receivedAt":"2017-10-17T01:13:41Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Alternatively, I suppose we could just always escape in\n> add--interactive. I'm trying to think of a case where somebody would\n> really want their diffFilter to see the raw bytes (or vice versa, to\n> show raw bytes produced by their filter), but I'm having trouble coming\n> up with one.\n\nYour patch below only implements the \"tame the raw bytes that come\nout of their filter\", which is quite agreeable.\n\n> I.e., something like this would probably help your case without hurting\n> anybody:\n> ...\n>\n> I can't help but feel this is the tip of a larger iceberg, though. E.g.,\n> what about characters outside of the terminal's correct encoding? Or\n> broken UTF-8 characters?\n\nHmph. If you use it as a \"built-in\" that is a fallback for\ndiffFilter, i.e. use it only when the end user does not have one,\nthen users can override whatever wrong thing the built-in logic does\nso... ;-)\n\n"},{"id":"330506","messageId":"20171017024540.erccnanmvz6t2aue@sigill.intra.peff.net","threadId":"46971","inReplyTo":"xmqqk1zur2yp.fsf@gitster.mtv.corp.google.com","subject":"Re: Consider escaping special characters like 'less' does","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-17T02:45:40Z","receivedAt":"2017-10-17T02:45:46Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 17, 2017 at 10:13:34AM +0900, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Alternatively, I suppose we could just always escape in\n> > add--interactive. I'm trying to think of a case where somebody would\n> > really want their diffFilter to see the raw bytes (or vice versa, to\n> > show raw bytes produced by their filter), but I'm having trouble coming\n> > up with one.\n> \n> Your patch below only implements the \"tame the raw bytes that come\n> out of their filter\", which is quite agreeable.\n\nYes. I think that is probably OK, especially given that we continue to\nallow terminal escapes (certainly some filters would want to use colors;\nI don't know if any would want to use more exotic control codes).\n\n> > I can't help but feel this is the tip of a larger iceberg, though. E.g.,\n> > what about characters outside of the terminal's correct encoding? Or\n> > broken UTF-8 characters?\n> \n> Hmph. If you use it as a \"built-in\" that is a fallback for\n> diffFilter, i.e. use it only when the end user does not have one,\n> then users can override whatever wrong thing the built-in logic does\n> so... ;-)\n\nYes, and maybe that is the best way to do it. It just seems like it is\nopening a can of worms about exactly which things should be filtered and\nhow.\n\nI also wondered if people would be annoyed that by using a filter, they\ndon't get the benefit of the escaping, unless their filter implements it\nseparately on top (and the original purpose of the filter option was for\nthings like diff-highlight and diff-so-fancy, which do not do such\nescaping).\n\n-Peff\n"},{"id":"330801","messageId":"37013BB09DFE493BB7E11E66A34657CE@blackfat","threadId":"46971","inReplyTo":"20171016221324.stn56gqnliunvcdv@sigill.intra.peff.net","subject":"RE: Consider escaping special characters like 'less' does","fromName":"Jason Pyeron","fromEmail":"jpyeron@pdinc.us","sentAt":"2017-10-22T12:27:20Z","receivedAt":"2017-10-22T12:27:29Z","isPatch":false,"sender":{"key":"jpyeron@pdinc.us","avatar":"https://gravatar.com/avatar/c2e53452caa53d940768a1ffc9cf76196d851b9b534b7a39cd39852a70a0508f?d=mp&s=160"},"body":"> -----Original Message-----\n> From: Jeff King\n> Sent: Monday, October 16, 2017 6:13 PM\n> To: Joris Valette\n> Cc: Andreas Schwab; Jason Pyeron; git@vger.kernel.org\n> Subject: Re: Consider escaping special characters like 'less' does\n> \n\n> \n> I.e., something like this would probably help your case \n> without hurting\n> anybody:\n> \n> diff --git a/git-add--interactive.perl b/git-add--interactive.perl\n> index 28b325d754..d44e5ea459 100755\n> --- a/git-add--interactive.perl\n> +++ b/git-add--interactive.perl\n> @@ -714,6 +714,16 @@ sub parse_diff {\n> \t\tpush @{$hunk[-1]{DISPLAY}},\n> \t\t\t(@colored ? $colored[$i] : $diff[$i]);\n> \t}\n> +\n> +\tforeach my $hunk (@hunk) {\n> +\t\tforeach my $line (@{$hunk->{DISPLAY}}) {\n> +\t\t\t# all control chars minus newline and \n> ESC (for color)\n> +\t\t\tif ($line =~ s/[\\000-\\011\\013-\\032\\034-\\037]/?/g) {\n\nWhat about CR [0x0D] ?\n\n> +\t\t\t\t$hunk->{CONTROLCHARS} = 1;\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> +\n> \treturn @hunk;\n> }\n> \n> @@ -1407,6 +1417,9 @@ sub patch_update_file {\n> \t\tif ($hunk[$ix]{TYPE} eq 'hunk') {\n> \t\t\t$other .= ',e';\n> \t\t}\n> +\t\tif ($hunk[$ix]->{CONTROLCHARS}) {\n> +\t\t\tprint \"warning: control characters in \n> hunk have been replaced by '?'\\n\";\n> +\t\t}\n> \t\tfor (@{$hunk[$ix]{DISPLAY}}) {\n> \t\t\tprint;\n> \t\t}\n> \n> I can't help but feel this is the tip of a larger iceberg, \n> though. E.g.,\n> what about characters outside of the terminal's correct encoding? Or\n> broken UTF-8 characters?\n> \n> -Peff\n> \n\n"},{"id":"330813","messageId":"20171023004523.temglxnlpuns2ejy@sigill.intra.peff.net","threadId":"46971","inReplyTo":"37013BB09DFE493BB7E11E66A34657CE@blackfat","subject":"Re: Consider escaping special characters like 'less' does","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-23T00:45:24Z","receivedAt":"2017-10-23T00:45:30Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Oct 22, 2017 at 08:27:20AM -0400, Jason Pyeron wrote:\n\n> > ESC (for color)\n> > +\t\t\tif ($line =~ s/[\\000-\\011\\013-\\032\\034-\\037]/?/g) {\n> \n> What about CR [0x0D] ?\n\nI assumed that CR was one of the things we'd want to avoid (and it was\nin fact what I used to test this). E.g., try:\n\n echo base >file\n git add file\n printf 'foo\\r+bar\\n' >file\n\nRunning through \"less\" shows something like:\n\n diff --git a/file b/file\n index df967b9..5b6ee80 100644\n --- a/file\n +++ b/file\n @@ -1 +1 @@\n -base\n +foo^M+bar\n\nbut \"git add -p\" shows:\n\n $ git add -p\n diff --git a/file b/file\n index df967b9..5b6ee80 100644\n --- a/file\n +++ b/file\n @@ -1 +1 @@\n -base\n +bar\n Stage this hunk [y,n,q,a,d,/,e,?]? \n\nFor systems where CRLF is the native line ending, we'd still expect to\nsee only LFs here when line-ending conversion is on (since the diff\nshows the canonical in-repo form).\n\nFor files where the CRs must remain for some reason, they'd show as a\n\"?\" at the end. The \"^M\" shown by \"less\" is a bit more informative. If\nwe really do want to pursue this direction, it might make sense to use\nmore descriptive placeholders.\n\n-Peff\n"}]}