{"thread":{"id":"49886","subject":"BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF","startedAt":"2018-11-23T18:18:45Z","lastAt":"2018-12-06T18:41:24Z","messageCount":23,"participants":["Frank Schäfer","Johannes Sixt","Torsten Bögershausen","brian m. carlson","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"363976","messageId":"1f068f3a-d636-9b9c-f501-51d33412b964@googlemail.com","threadId":"49886","inReplyTo":null,"subject":"BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF","fromName":"Frank Schäfer","fromEmail":"fschaefer.oss@googlemail.com","sentAt":"2018-11-23T18:19:41Z","receivedAt":"2018-11-23T18:18:45Z","isPatch":false,"sender":{"key":"fschaefer.oss@googlemail.com","avatar":null},"body":"The CR marker ^M doesn't show up in '-' lines of diffs when the ending\nof the removed line is CR+LF.\nIt shows up as expected in '-' lines when the ending of the removed line\nis CR only.\nIt also always shows up as expected in '+' lines.\n\nThese are the diffs of the 6 possible line ending changes:\n\nLF to CR+LF:\n@@ -1,2 +1,2 @@\n-aaa\n+aaa^M\n bbb\n \nCR+LF to LF:\n@@ -1,2 +1,2 @@\n-aaa            => BUG: should be -aaa^M\n+aaa\n bbb\n\nCR to CR+LF:\n@@ -1 +1,2 @@\n-aaa^Mbbb\n+aaa^M\n+bbb\n\nCR+LF to CR:\n@@ -1,2 +1 @@\n-aaa            => BUG: should be -aaa^M\n-bbb\n+aaa^Mbbb\n\nLF to CR:\n@@ -1,2 +1 @@\n-aaa\n-bbb\n+aaa^Mbbb\n\nCR to LF:\n@@ -1 +1,2 @@\n-aaa^Mbbb\n+aaa\n+bbb\n\nTested with version 2.19.1.\n\nRegards,\nFrank Schäfer\n\n"},{"id":"363977","messageId":"4dca3f64-4851-6d48-8266-dfe55f597739@kdbg.org","threadId":"49886","inReplyTo":"1f068f3a-d636-9b9c-f501-51d33412b964@googlemail.com","subject":"Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2018-11-23T21:47:36Z","receivedAt":"2018-11-23T21:47:41Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 23.11.18 um 19:19 schrieb Frank Schäfer:\n> The CR marker ^M doesn't show up in '-' lines of diffs when the ending\n> of the removed line is CR+LF.\n> It shows up as expected in '-' lines when the ending of the removed line\n> is CR only.\n> It also always shows up as expected in '+' lines.\n\nIs your repository configured to (1) highlight whitespace errors in diff \noutput and (2) to leave CRLF alone in text files?\n\nIf so, then it is just a side-effect of this combination, an illusion, \nso to say: The CR in the CRLF combo is trailing whitespace. The 'git \ndiff' marks it by inserting an escape sequence to switch the color \nbefore ^M and another escape sequence to reset to color after ^M. This \nbreaks the CRLF combination apart, so that the pager does not process it \nas a combined CRLF sequence; it displays the lone CR as ^M.\n\nIt is easy to achieve the opposite effect, i.e., that ^M is not \ndisplayed. For example with these lines in .git/info/attributes or \n.gitattributes:\n\n*.cpp \nwhitespace=trailing-space,cr-at-eol,indent-with-non-tab,space-before-tab\n*.h whitespace=trailing-space,cr-at-eol,indent-with-non-tab,space-before-tab\n\nNote the cr-at-eol. (There may be shorter versions to achieve a similar \neffect.)\n\n-- Hannes\n"},{"id":"364005","messageId":"edadf857-2d4b-f058-5e07-286afb312901@googlemail.com","threadId":"49886","inReplyTo":"4dca3f64-4851-6d48-8266-dfe55f597739@kdbg.org","subject":"Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF","fromName":"Frank Schäfer","fromEmail":"fschaefer.oss@googlemail.com","sentAt":"2018-11-24T14:51:26Z","receivedAt":"2018-11-24T14:50:30Z","isPatch":false,"sender":{"key":"fschaefer.oss@googlemail.com","avatar":null},"body":"Am 23.11.18 um 22:47 schrieb Johannes Sixt:\n> Am 23.11.18 um 19:19 schrieb Frank Schäfer:\n>> The CR marker ^M doesn't show up in '-' lines of diffs when the ending\n>> of the removed line is CR+LF.\n>> It shows up as expected in '-' lines when the ending of the removed line\n>> is CR only.\n>> It also always shows up as expected in '+' lines.\n>\n> Is your repository configured to (1) highlight whitespace errors in\n> diff output and (2) to leave CRLF alone in text files?\nI'm using the default configuration, so whitespace is set to\ntrailing-space, but cr-at-eol is not set.\n\n>\n> If so, then it is just a side-effect of this combination, an illusion,\n> so to say: The CR in the CRLF combo is trailing whitespace. The 'git\n> diff' marks it by inserting an escape sequence to switch the color\n> before ^M and another escape sequence to reset to color after ^M. This\n> breaks the CRLF combination apart, so that the pager does not process\n> it as a combined CRLF sequence; it displays the lone CR as ^M.\nUrghh... so that needs to be fixed.\nWhy does it work correctly with '+' lines ?\n\n>\n> It is easy to achieve the opposite effect, i.e., that ^M is not\n> displayed. For example with these lines in .git/info/attributes or\n> .gitattributes:\n>\n> *.cpp\n> whitespace=trailing-space,cr-at-eol,indent-with-non-tab,space-before-tab\n> *.h\n> whitespace=trailing-space,cr-at-eol,indent-with-non-tab,space-before-tab\n>\n> Note the cr-at-eol. (There may be shorter versions to achieve a\n> similar effect.)\nWith cr-at-eol, ^M indeed doesn't show up anymore in '+' lines with\nCR+LF line endings. That's correct/expected.\n'-' lines with CR+LF line endings are displayed correctly in this case, too.\nHowever, ^M still shows up in '+' and '-' lines with CR line endings.\n\nHmm... is CR-only line termination supported at all ?\nE.g. 'eol' can be set to 'lf' or 'crlf' but not 'cr'...\n\nRegards,\nFrank\n\n>\n> -- Hannes\n"},{"id":"364007","messageId":"20181124152504.GA14440@tor.lan","threadId":"49886","inReplyTo":"edadf857-2d4b-f058-5e07-286afb312901@googlemail.com","subject":"Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2018-11-24T15:25:04Z","receivedAt":"2018-11-24T15:25:08Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Sat, Nov 24, 2018 at 03:51:26PM +0100, Frank Schäfer wrote:\n[]\n> \n> Hmm... is CR-only line termination supported at all ?\n> E.g. 'eol' can be set to 'lf' or 'crlf' but not 'cr'...\n> \n\nNo, CR-only is not supported, because:\nNobody was implementing it, and that is probably because\nthe only question abou CR-only (at least what I remember)\nwas a when an old Mac OS (not the Mac OS X)\nwas used (which used to use CR instead of LF).\n\nAnd, such feature may be implemented by writing a filter,\nreplace CR with LF as \"clean\" and \"LF\" with \"CR\" for smudge.\n\n"},{"id":"364013","messageId":"80ffe850-b966-a37b-09bd-44e04d769944@kdbg.org","threadId":"49886","inReplyTo":"edadf857-2d4b-f058-5e07-286afb312901@googlemail.com","subject":"Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2018-11-24T22:07:26Z","receivedAt":"2018-11-24T22:07:31Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 24.11.18 um 15:51 schrieb Frank Schäfer:\n> Am 23.11.18 um 22:47 schrieb Johannes Sixt:\n>> Am 23.11.18 um 19:19 schrieb Frank Schäfer:\n>>> The CR marker ^M doesn't show up in '-' lines of diffs when the ending\n>>> of the removed line is CR+LF.\n>>> It shows up as expected in '-' lines when the ending of the removed line\n>>> is CR only.\n>>> It also always shows up as expected in '+' lines.\n>>\n>> Is your repository configured to (1) highlight whitespace errors in\n>> diff output and (2) to leave CRLF alone in text files?\n> I'm using the default configuration, so whitespace is set to\n> trailing-space, but cr-at-eol is not set.\n> \n>>\n>> If so, then it is just a side-effect of this combination, an illusion,\n>> so to say: The CR in the CRLF combo is trailing whitespace. The 'git\n>> diff' marks it by inserting an escape sequence to switch the color\n>> before ^M and another escape sequence to reset to color after ^M. This\n>> breaks the CRLF combination apart, so that the pager does not process\n>> it as a combined CRLF sequence; it displays the lone CR as ^M.\n> Urghh... so that needs to be fixed.\n> Why does it work correctly with '+' lines ?\n\nI don't think that there is anything to fix. If you have a file with \nCRLF in it, but you did not declare to Git that CRLF is the expected \nend-of-line indicator, then the CR *is* trailing whitespace (because the \nline ends at LF), and 'git diff' highlights it.\n\n-- Hannes\n"},{"id":"364034","messageId":"2858f03b-89a7-be52-501f-55b6d281bebc@googlemail.com","threadId":"49886","inReplyTo":"80ffe850-b966-a37b-09bd-44e04d769944@kdbg.org","subject":"Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF","fromName":"Frank Schäfer","fromEmail":"fschaefer.oss@googlemail.com","sentAt":"2018-11-25T14:03:11Z","receivedAt":"2018-11-25T14:02:16Z","isPatch":false,"sender":{"key":"fschaefer.oss@googlemail.com","avatar":null},"body":"Am 24.11.18 um 23:07 schrieb Johannes Sixt:\n> I don't think that there is anything to fix. If you have a file with\n> CRLF in it, but you did not declare to Git that CRLF is the expected\n> end-of-line indicator, then the CR *is* trailing whitespace (because\n> the line ends at LF), and 'git diff' highlights it. \n\nSure, it's correct to highlight it.\nBut it doesn't highlight it in removed lines, just in added lines.\nI can see no good reason why removed and added lines should be treated\ndifferently.\n\nLet me give you two real-life examples:\n \n1) If CR+LF line termination is used in a file, changing the content of\na line (but not its termination) currently produces a diff like\n\n-something\n+something_new^M\n\nwhich causes the user to think he has changed the line ending (added a\nCR) although he didn't.\n\n2) If someone/something unintentionally changes the line termination\nfrom CR+LF to LF, it doesn't show up in the diff:\n\n-something\n+something\n\nI don't think this behavior makes sense.\nAt least it's IMHO not what users expect to see.\nThey want to see what's really going on, not to get confused.\n\nRegards,\nFrank\n"},{"id":"364038","messageId":"30442f9c-a1cb-4635-d8e3-a301d94a56fd@kdbg.org","threadId":"49886","inReplyTo":"2858f03b-89a7-be52-501f-55b6d281bebc@googlemail.com","subject":"Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2018-11-25T21:39:44Z","receivedAt":"2018-11-25T21:39:49Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 25.11.18 um 15:03 schrieb Frank Schäfer:\n> Am 24.11.18 um 23:07 schrieb Johannes Sixt:\n>> I don't think that there is anything to fix. If you have a file with\n>> CRLF in it, but you did not declare to Git that CRLF is the expected\n>> end-of-line indicator, then the CR *is* trailing whitespace (because\n>> the line ends at LF), and 'git diff' highlights it.\n> \n> Sure, it's correct to highlight it.\n> But it doesn't highlight it in removed lines, just in added lines.\n> I can see no good reason why removed and added lines should be treated\n> differently.\n\nBut incorrect whitespace is never highlighted in removed lines, why \nshould CR be an exception?\n\n> 1) If CR+LF line termination is used in a file, changing the content of\n> a line (but not its termination) currently produces a diff like\n> \n> -something\n> +something_new^M\n> \n> which causes the user to think he has changed the line ending (added a\n> CR) although he didn't.\n\nBut this is not limited to CR at EOL:\n\n-something\n+something_new\n\nwill also show the incorrect whitespace highlighted only for the + line.\n\n> 2) If someone/something unintentionally changes the line termination\n> from CR+LF to LF, it doesn't show up in the diff:\n> \n> -something\n> +something\n\nSame here for other cases, for example\n\n-something\n+something\n\nwill not have on obvious indicator that whitespace was corrected.\n\nIf you are worried about a change in EOL style, you should better listen \nto your other tools. Either it is important, or it is not. If it is, \nthey will report it to you. If it isn't, why care?\n\n-- Hannes\n"},{"id":"364045","messageId":"20181125235023.GS890086@genre.crustytoothpaste.net","threadId":"49886","inReplyTo":"2858f03b-89a7-be52-501f-55b6d281bebc@googlemail.com","subject":"Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-11-25T23:50:24Z","receivedAt":"2018-11-25T23:50:33Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Sun, Nov 25, 2018 at 03:03:11PM +0100, Frank Schäfer wrote:\n> Am 24.11.18 um 23:07 schrieb Johannes Sixt:\n> > I don't think that there is anything to fix. If you have a file with\n> > CRLF in it, but you did not declare to Git that CRLF is the expected\n> > end-of-line indicator, then the CR *is* trailing whitespace (because\n> > the line ends at LF), and 'git diff' highlights it. \n> \n> Sure, it's correct to highlight it.\n> But it doesn't highlight it in removed lines, just in added lines.\n> I can see no good reason why removed and added lines should be treated\n> differently.\n\nThe default behavior is to highlight whitespace errors only in new\nlines, because the assumption is that while you don't want to introduce\nnew errors, you can't do anything about old mistakes without rewriting\nhistory.\n\nI agree that in many circumstances, such as code review, this may be\nundesirable. In the past, I've done code reviews where I may let\nexisting trailing whitespace go but am strict about not introducing\nnew trailing whitespace, and being able to see both is helpful.\n\nIf you want to see whitespace errors in both the old and the new, use\n--ws-error-highlight or set diff.wsErrorHighlight.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"364050","messageId":"xmqqr2f8zdt5.fsf@gitster-ct.c.googlers.com","threadId":"49886","inReplyTo":"30442f9c-a1cb-4635-d8e3-a301d94a56fd@kdbg.org","subject":"Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-26T04:02:30Z","receivedAt":"2018-11-26T04:02:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> But incorrect whitespace is never highlighted in removed lines, why\n> should CR be an exception?\n> ...\n> Same here for other cases, for example\n> \n> -something\n> +something\n>\n> will not have on obvious indicator that whitespace was corrected.\n\nAll correct, but misses one point in Frank's original report, which\nobserved\n\n\t-something\n\t+something_new^M\n\nwith ^M highlighted for whitespace error. The highlighting is\ncorrect. But notice lack of caret-em on the preimage line?\n\nIt turns out that we show something like this\n\n -something CR LF\n\nfor the preimage line, while showing something like this\n\n +something_new CR LF\n\nfor the postimage line. \n\nBecause CR on the postimage line, thanks to highlighting, appears\nalone separate from the LF, it is shown as two-letter caret-em\nsequence to the user.\n\nOn the other hand, because CR and LF appear next to each other on\nthe preimage line, the pager and/or the terminal behaves as if CR is\nnot even there and that is where Frank's complaint comes from, I think.\n\nThe code is doing the right thing by showing CR, but it is hidden by\nthe pager and/or the terminal.\n"},{"id":"364093","messageId":"f06b734a-fc8e-221a-c983-f2ab9daba17f@kdbg.org","threadId":"49886","inReplyTo":"xmqqzhtwzghr.fsf@gitster-ct.c.googlers.com","subject":"Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2018-11-26T19:49:33Z","receivedAt":"2018-11-26T19:49:39Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 26.11.18 um 04:04 schrieb Junio C Hamano:\n> It appears to me that what Frank sees is not \"^M highlighted for\n> whitespace breakage appears only on postimage lines, while ^M is\n> shown but not highlighted on preimage lines\". My reading of the\n> above is \"Not just without highlight, ^M is just *NOT* shown on the\n> preimage line\".\n> \n> That does not sound right. I would understand it if both lines\n> showed ^M at the end, and only the one on the postimage line had it\n> highlighted as a trailing-whitespace.\n\nI agree that this is a (small?) weakness. But...\n\n> When we are producing a colored output, we know we are *not* writing\n> for machines, so one way to do it would be to turn CR at the end of\n> the line into two letter \"^\" \"M\" sequence on our end, without relying\n> on the terminal and/or the pager. I dunno.\n\n... this goes too far, IMO. It is the pager's task to decode control \ncharacters.\n\n-- Hannes\n"},{"id":"364105","messageId":"xmqqva4jv2kc.fsf@gitster-ct.c.googlers.com","threadId":"49886","inReplyTo":"f06b734a-fc8e-221a-c983-f2ab9daba17f@kdbg.org","subject":"Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-26T23:31:15Z","receivedAt":"2018-11-26T23:31:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Am 26.11.18 um 04:04 schrieb Junio C Hamano:\n>>\n>> That does not sound right. I would understand it if both lines\n>> showed ^M at the end, and only the one on the postimage line had it\n>> highlighted as a trailing-whitespace.\n>\n> I agree that this is a (small?) weakness. But...\n>\n>> When we are producing a colored output, we know we are *not* writing\n>> for machines, so one way to do it would be to turn CR at the end of\n>> the line into two letter \"^\" \"M\" sequence on our end, without relying\n>> on the terminal and/or the pager. I dunno.\n>\n> ... this goes too far, IMO. It is the pager's task to decode control\n> characters.\n\nIt was tongue-in-cheek suggestion to split a CR into caret-em on our\nend, but we'd get essentially the same visual effect if we added a\nrule:\n\n\tWhen producing a colored output (not limited to whitespace\n\terror coloring of diff output), insert before a CR\n\tthat comes immediately before a LF.\n\nThen, what Frank saw in the troublesome output would become\n\n\t -something CR LF\n\t +something_new CR LF\n\nand we'll let the existing pager+terminal magic turn that trailing\nCR on the preimage line into caret-em, just like the trailing CR on\nthe postimage line is already shown as caret-em with the current\noutput.\n\nAnd a good thing is that I do not think that new rule is doing any\ndecode of control chars on our end. We are just producing colored\noutput normally.\n"},{"id":"364147","messageId":"3e24a770-47fc-50e4-d757-1e4a28dcd019@kdbg.org","threadId":"49886","inReplyTo":"xmqqva4jv2kc.fsf@gitster-ct.c.googlers.com","subject":"Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2018-11-27T18:15:31Z","receivedAt":"2018-11-27T18:15:37Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 27.11.18 um 00:31 schrieb Junio C Hamano:\n> Johannes Sixt writes:\n>> Am 26.11.18 um 04:04 schrieb Junio C Hamano:\n>> ... this goes too far, IMO. It is the pager's task to decode control\n>> characters.\n> \n> It was tongue-in-cheek suggestion to split a CR into caret-em on our\n> end, but we'd get essentially the same visual effect if we added a\n> rule:\n> \n> \tWhen producing a colored output (not limited to whitespace\n> \terror coloring of diff output), insert before a CR\n> \tthat comes immediately before a LF.\n> \n> Then, what Frank saw in the troublesome output would become\n> \n> \t -something CR LF\n> \t +something_new CR LF\n> \n> and we'll let the existing pager+terminal magic turn that trailing\n> CR on the preimage line into caret-em, just like the trailing CR on\n> the postimage line is already shown as caret-em with the current\n> output.\n\nI wouldn't want that to happen for all output (context lines, - lines, + \nlines): I really am not interested to see all the CRs in my CRLF files.\n\n> And a good thing is that I do not think that new rule is doing any\n> decode of control chars on our end. We are just producing colored\n> output normally.\n\nBut we already have it, as Brian pointed out:\n\n git diff --ws-error-highlight=old,new\n\nor by setting diff.wsErrorHighlight accordingly.\n\n-- Hannes\n"},{"id":"364157","messageId":"b69f9626-7a6f-521a-1b4e-8b21dc214d93@googlemail.com","threadId":"49886","inReplyTo":"xmqqva4jv2kc.fsf@gitster-ct.c.googlers.com","subject":"Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF","fromName":"Frank Schäfer","fromEmail":"fschaefer.oss@googlemail.com","sentAt":"2018-11-27T20:06:55Z","receivedAt":"2018-11-27T20:06:00Z","isPatch":false,"sender":{"key":"fschaefer.oss@googlemail.com","avatar":null},"body":"\nAm 27.11.18 um 00:31 schrieb Junio C Hamano:\n> Johannes Sixt writes:\n>\n>> Am 26.11.18 um 04:04 schrieb Junio C Hamano:\n>>> That does not sound right. I would understand it if both lines\n>>> showed ^M at the end, and only the one on the postimage line had it\n>>> highlighted as a trailing-whitespace.\n>> I agree that this is a (small?) weakness. But...\n>>\n>>> When we are producing a colored output, we know we are *not* writing\n>>> for machines, so one way to do it would be to turn CR at the end of\n>>> the line into two letter \"^\" \"M\" sequence on our end, without relying\n>>> on the terminal and/or the pager. I dunno.\n>> ... this goes too far, IMO. It is the pager's task to decode control\n>> characters.\n> It was tongue-in-cheek suggestion to split a CR into caret-em on our\n> end, but we'd get essentially the same visual effect if we added a\n> rule:\n>\n> \tWhen producing a colored output (not limited to whitespace\n> \terror coloring of diff output), insert before a CR\n> \tthat comes immediately before a LF.\n>\n> Then, what Frank saw in the troublesome output would become\n>\n> \t -something CR LF\n> \t +something_new CR LF\n>\n> and we'll let the existing pager+terminal magic turn that trailing\n> CR on the preimage line into caret-em, just like the trailing CR on\n> the postimage line is already shown as caret-em with the current\n> output.\n>\n> And a good thing is that I do not think that new rule is doing any\n> decode of control chars on our end. We are just producing colored\n> output normally.\n\nHmm... I think I now understand what caused the confusion here.\nIt was my mistake: I didn't consider that EOL characters are whitespace\ncharacters, too. :/\n\nNevertheless, I still think that eol (CR, LF) and \"normal\" whitespace\n(space, tab) should be distinguished and marked/displayed differently,\nbecause they are playing different roles.\nYour suggestion seems to be a good solution for that.\n\nRegards,\nFrank\n\n\n\n"},{"id":"364159","messageId":"fefed4ac-33ce-20f0-d777-510c01919f95@googlemail.com","threadId":"49886","inReplyTo":"3e24a770-47fc-50e4-d757-1e4a28dcd019@kdbg.org","subject":"Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF","fromName":"Frank Schäfer","fromEmail":"fschaefer.oss@googlemail.com","sentAt":"2018-11-27T20:09:47Z","receivedAt":"2018-11-27T20:08:51Z","isPatch":false,"sender":{"key":"fschaefer.oss@googlemail.com","avatar":null},"body":"\nAm 27.11.18 um 19:15 schrieb Johannes Sixt:\n> Am 27.11.18 um 00:31 schrieb Junio C Hamano:\n>> Johannes Sixt writes:\n>>> Am 26.11.18 um 04:04 schrieb Junio C Hamano:\n>>> ... this goes too far, IMO. It is the pager's task to decode control\n>>> characters.\n>>\n>> It was tongue-in-cheek suggestion to split a CR into caret-em on our\n>> end, but we'd get essentially the same visual effect if we added a\n>> rule:\n>>\n>>     When producing a colored output (not limited to whitespace\n>>     error coloring of diff output), insert before a CR\n>>     that comes immediately before a LF.\n>>\n>> Then, what Frank saw in the troublesome output would become\n>>\n>>      -something CR LF\n>>      +something_new CR LF\n>>\n>> and we'll let the existing pager+terminal magic turn that trailing\n>> CR on the preimage line into caret-em, just like the trailing CR on\n>> the postimage line is already shown as caret-em with the current\n>> output.\n>\n> I wouldn't want that to happen for all output (context lines, - lines,\n> + lines): I really am not interested to see all the CRs in my CRLF files.\n>\n>> And a good thing is that I do not think that new rule is doing any\n>> decode of control chars on our end.  We are just producing colored\n>> output normally.\n>\n> But we already have it, as Brian pointed out:\n>\n>    git diff --ws-error-highlight=old,new\n>\n> or by setting diff.wsErrorHighlight accordingly.\n... but that also turns on displaying normal space/tab errors in removed\nlines.\nSo to make both of us happy, we would need separate switches.\n\nAny chance this can be done easily enough ?\n\nRegards,\nFrank\n\n"},{"id":"364279","messageId":"xmqqk1kwr5tp.fsf@gitster-ct.c.googlers.com","threadId":"49886","inReplyTo":"3e24a770-47fc-50e4-d757-1e4a28dcd019@kdbg.org","subject":"Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-29T02:11:14Z","receivedAt":"2018-11-29T02:11:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Am 27.11.18 um 00:31 schrieb Junio C Hamano:\n>> Johannes Sixt writes:\n>>> Am 26.11.18 um 04:04 schrieb Junio C Hamano:\n>>> ... this goes too far, IMO. It is the pager's task to decode control\n>>> characters.\n>>\n>> It was tongue-in-cheek suggestion to split a CR into caret-em on our\n>> end, but we'd get essentially the same visual effect if we added a\n>> rule:\n>>\n>> \tWhen producing a colored output (not limited to whitespace\n>> \terror coloring of diff output), insert before a CR\n>> \tthat comes immediately before a LF.\n\nThis was misspoken a bit. Let's revise it to\n\n \tWhen producing a colored output (not limited to whitespace\n \terror coloring of diff output) for contents that are not\n \tmarked as eol=crlf (and other historical means), insert\n \t before a CR that comes immediately before a LF.\n\nto exempt CR in CRLF sequence that the contents and the user agree\nto be part of the end-of-line marker.\n\n> I wouldn't want that to happen for all output (context lines, - lines,\n> + lines): I really am not interested to see all the CRs in my CRLF\n> files.\n\nSo your CRLF files will not give caret-em.\n\n>> And a good thing is that I do not think that new rule is doing any\n>> decode of control chars on our end. We are just producing colored\n>> output normally.\n>\n> But we already have it, as Brian pointed out:\n>\n> git diff --ws-error-highlight=old,new\n>\n> or by setting diff.wsErrorHighlight accordingly.\n\nNot really. If the context lines end with CRLF and the material is\nnot CRLF, I do not think CR at the end of them should be highlighted\nas whitespace errors (as we tell to detect errors on \"old\" and \"new\"\nlines), but I think we still should make an effort to help them be\nvisible to the users. Otherwise, next Frank will come and complain\nafter seeing\n\n\t-something\n\t some common thing\n\t+something_new^M\n\nWith the suggested change, you can distinguish the following two\n(and use your imagination that there are many \"some common thing\"\nlines), which would help the user guide if the file should be marked\nas CRLF file, or the contents mistakenly has some lines that end\nwith CRLF by mistake. The corrective action between the two cases\nwould vastly be different.\n\n\t-something^M \t-something \n\t some common thing^M \t some common thing\n\t some common thing^M \t some common thing\n\t some common thing^M \t some common thing\n\t+something_new^M \t+something_new^M \n\nWithout, both would look like the RHS of the illustration, and with\nthe \"highlight both\", you'd only get an extra caret-em on the\nremoved line, and need to judge without the help from common context\nlines.\n\nBesides, --ws-error-highlight shows all whitespace errors, and\nshowing CR as caret-em is mere side effect of the interaction\nbetween its coloring and the pager.\n\n"},{"id":"364453","messageId":"7a4ecc75-2dc4-041b-3d12-46cddae0a27f@googlemail.com","threadId":"49886","inReplyTo":"xmqqk1kwr5tp.fsf@gitster-ct.c.googlers.com","subject":"Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF","fromName":"Frank Schäfer","fromEmail":"fschaefer.oss@googlemail.com","sentAt":"2018-12-02T19:31:47Z","receivedAt":"2018-12-02T19:30:51Z","isPatch":false,"sender":{"key":"fschaefer.oss@googlemail.com","avatar":null},"body":"Hi Junio,\n\nAm 29.11.18 um 03:11 schrieb Junio C Hamano:\n[...]\n> This was misspoken a bit. Let's revise it to\n>\n> \tWhen producing a colored output (not limited to whitespace\n> \terror coloring of diff output) for contents that are not\n> \tmarked as eol=crlf (and other historical means), insert\n> \t before a CR that comes immediately before a LF.\nYou mean\n     ...\n     *after* a CR that comes immediately before a LF.\"\n\n\nOK, AFAICS this produces the desired output in all cases if eol=lf.\n\nNow what about the case eol=crlf ?\nKeeping the current behavior of '-' lines is correct.\nBut shouldn't ^M now be suppressed in '+' lines for a consistent behavior ?\n\nWith other words:\n\"If CR comes immediately before a LF, do the following with *all* lines:\n after the CR if eol=lf but do not after the CR if eol=crlf.\"\n\nRegards,\nFrank\n"},{"id":"364458","messageId":"3572b619-0603-d16d-392c-4cc8e0bc4614@kdbg.org","threadId":"49886","inReplyTo":"7a4ecc75-2dc4-041b-3d12-46cddae0a27f@googlemail.com","subject":"Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2018-12-02T21:22:32Z","receivedAt":"2018-12-02T21:22:38Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 02.12.18 um 20:31 schrieb Frank Schäfer:\n> Am 29.11.18 um 03:11 schrieb Junio C Hamano:\n> [...]\n>> This was misspoken a bit. Let's revise it to\n>>\n>> \tWhen producing a colored output (not limited to whitespace\n>> \terror coloring of diff output) for contents that are not\n>> \tmarked as eol=crlf (and other historical means), insert\n>> \t before a CR that comes immediately before a LF.\n> You mean\n>      ...\n>      *after* a CR that comes immediately before a LF.\"\n> \n> \n> OK, AFAICS this produces the desired output in all cases if eol=lf.\n> \n> Now what about the case eol=crlf ?\n> Keeping the current behavior of '-' lines is correct.\n> But shouldn't ^M now be suppressed in '+' lines for a consistent behavior ?\n\nThat can be achieved with whitespace=cr-at-eol.\n\n> With other words:\n> \"If CR comes immediately before a LF, do the following with *all* lines:\n> after the CR if eol=lf but do not after the CR if eol=crlf.\"\n\nWhy? It is the pager's duty to highlight CR, IMO. If it doesn't, but the \nuser wants to see them, then they are using the wrong pager or the wrong \npager settings.\n\nAs far as I am concerned, I do not have any of my files marked as \neol=crlf, but I do *not* want to see ^M in the pager. I.e., having git \ninsert between CR and LF would do the wrong thing for me.\n\n-- Hannes\n"},{"id":"364461","messageId":"xmqqmupnh0lo.fsf@gitster-ct.c.googlers.com","threadId":"49886","inReplyTo":"7a4ecc75-2dc4-041b-3d12-46cddae0a27f@googlemail.com","subject":"Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-12-03T01:15:31Z","receivedAt":"2018-12-03T01:15:40Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Frank Schäfer writes:\n\n> Hi Junio,\n>\n> Am 29.11.18 um 03:11 schrieb Junio C Hamano:\n> [...]\n>> This was misspoken a bit. Let's revise it to\n>>\n>> \tWhen producing a colored output (not limited to whitespace\n>> \terror coloring of diff output) for contents that are not\n>> \tmarked as eol=crlf (and other historical means), insert\n>> \t before a CR that comes immediately before a LF.\n> You mean\n>      ...\n>      *after* a CR that comes immediately before a LF.\"\n>\n> OK, AFAICS this produces the desired output in all cases if eol=lf.\n\nOK, yeah, I think I meant \"after\", i.e. ... CR LF, in order\nto force CR to be separated from LF.\n\n> Now what about the case eol=crlf ?\n\nI have no strong opinions, other than that \"LF in repository, CRLF\nin working tree\" would make the issue go away (when it is solved for\nEOL=LF like the above, that is).\n\n> Keeping the current behavior of '-' lines is correct.\n> But shouldn't ^M now be suppressed in '+' lines for a consistent behavior ?\n\nIf \"LF in repository, CRLF in working tree\" is done, there would not\nbe ^M at the end of the line, not just for removed lines, but also\nfor added lines, no?\n"},{"id":"364635","messageId":"de3bd8e1-cdd7-5fcf-0912-a216cc8cb3e9@googlemail.com","threadId":"49886","inReplyTo":"3572b619-0603-d16d-392c-4cc8e0bc4614@kdbg.org","subject":"Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF","fromName":"Frank Schäfer","fromEmail":"fschaefer.oss@googlemail.com","sentAt":"2018-12-05T19:29:15Z","receivedAt":"2018-12-05T19:28:20Z","isPatch":false,"sender":{"key":"fschaefer.oss@googlemail.com","avatar":null},"body":"\nAm 02.12.18 um 22:22 schrieb Johannes Sixt:\n> Am 02.12.18 um 20:31 schrieb Frank Schäfer:\n>> With other words:\n>> \"If CR comes immediately before a LF, do the following with *all* lines:\n>> after the CR if eol=lf but do not after the CR if\n>> eol=crlf.\"\n>\n> Why? It is the pager's duty to highlight CR, IMO. If it doesn't, but\n> the user wants to see them, then they are using the wrong pager or the\n> wrong pager settings.\nAFAIU Junios explanation it's not the pagers fault.\n\n>\n> As far as I am concerned, I do not have any of my files marked as\n> eol=crlf, but I do *not* want to see ^M in the pager. I.e., having git\n> insert between CR and LF would do the wrong thing for me.\n>\nBut doing the same thing in added lines is doing the right thing for you ?\nOr are you suggesting to fix the behavior of added lines instead ?\nIn any case, inconsistent behavior is not what we want.\n\nRegards,\nFrank\n\n"},{"id":"364636","messageId":"7796f0ac-d3db-68f9-89fa-9262d2187f57@googlemail.com","threadId":"49886","inReplyTo":"xmqqmupnh0lo.fsf@gitster-ct.c.googlers.com","subject":"Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF","fromName":"Frank Schäfer","fromEmail":"fschaefer.oss@googlemail.com","sentAt":"2018-12-05T19:43:03Z","receivedAt":"2018-12-05T19:42:08Z","isPatch":false,"sender":{"key":"fschaefer.oss@googlemail.com","avatar":null},"body":"\nAm 03.12.18 um 02:15 schrieb Junio C Hamano:\n> Frank Schäfer writes:\n>\n>> Hi Junio,\n>>\n>> Am 29.11.18 um 03:11 schrieb Junio C Hamano:\n>> [...]\n>>> This was misspoken a bit. Let's revise it to\n>>>\n>>> \tWhen producing a colored output (not limited to whitespace\n>>> \terror coloring of diff output) for contents that are not\n>>> \tmarked as eol=crlf (and other historical means), insert\n>>> \t before a CR that comes immediately before a LF.\n>> You mean\n>>      ...\n>>      *after* a CR that comes immediately before a LF.\"\n>>\n>> OK, AFAICS this produces the desired output in all cases if eol=lf.\n> OK, yeah, I think I meant \"after\", i.e. ... CR LF, in order\n> to force CR to be separated from LF.\n>\n>> Now what about the case eol=crlf ?\n> I have no strong opinions, other than that \"LF in repository, CRLF\n> in working tree\" would make the issue go away (when it is solved for\n> EOL=LF like the above, that is).\n>\n>> Keeping the current behavior of '-' lines is correct.\n>> But shouldn't ^M now be suppressed in '+' lines for a consistent behavior ?\n> If \"LF in repository, CRLF in working tree\" is done, there would not\n> be ^M at the end of the line, not just for removed lines, but also\n> for added lines, no?\nJust to be sure that I'm not missing anything here:\nWhat's your definition of \"LF in repository, CRLF in working tree\" in\nterms of config parameters ?\n\nRegards,\nFrank\n\n"},{"id":"364646","messageId":"337ed78c-afd5-24ab-709b-ff74ee6b90ac@kdbg.org","threadId":"49886","inReplyTo":"de3bd8e1-cdd7-5fcf-0912-a216cc8cb3e9@googlemail.com","subject":"Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2018-12-05T21:31:21Z","receivedAt":"2018-12-05T21:31:24Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 05.12.18 um 20:29 schrieb Frank Schäfer:\n> \n> Am 02.12.18 um 22:22 schrieb Johannes Sixt:\n>> Am 02.12.18 um 20:31 schrieb Frank Schäfer:\n>>> With other words:\n>>> \"If CR comes immediately before a LF, do the following with *all* lines:\n>>> after the CR if eol=lf but do not after the CR if\n>>> eol=crlf.\"\n>>\n>> Why? It is the pager's duty to highlight CR, IMO. If it doesn't, but\n>> the user wants to see them, then they are using the wrong pager or the\n>> wrong pager settings.\n> AFAIU Junios explanation it's not the pagers fault.\n\nThen Junio and I are in disagreement. IMO, Git does not have to be more \nclever than the pager when it comes to presentation of text.\n\n>> As far as I am concerned, I do not have any of my files marked as\n>> eol=crlf, but I do *not* want to see ^M in the pager. I.e., having git\n>> insert between CR and LF would do the wrong thing for me.\n>>\n> But doing the same thing in added lines is doing the right thing for you ?\n\nYes, I think so. As long as I'm not telling Git that my files are CRLF \nwhen they actual are, then the CR before LF is a whitespace error. \nNevertheless, I do *NOT* want Git to outwit my pager by inserting \n between CR and LF all the time so that it is forced to treat the \nlone CR like a control character that is to be made visible.\n\n> Or are you suggesting to fix the behavior of added lines instead ?\n> In any case, inconsistent behavior is not what we want.\n\nI'm suggesting that users who knowingly store CRLF files in the \ndatabase, but do not want to see ^M in added lines have to use \nwhitespace=cr-at-eol and that's all. I do not see inconsistency.\n\n-- Hannes\n"},{"id":"364664","messageId":"xmqq1s6vbhdr.fsf@gitster-ct.c.googlers.com","threadId":"49886","inReplyTo":"7796f0ac-d3db-68f9-89fa-9262d2187f57@googlemail.com","subject":"Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-12-06T00:58:40Z","receivedAt":"2018-12-06T00:58:45Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Frank Schäfer writes:\n\n> Just to be sure that I'm not missing anything here:\n> What's your definition of \"LF in repository, CRLF in working tree\" in\n> terms of config parameters ?\n\n:::Documentation/config/core.txt:::\n\ncore.autocrlf::\n\tSetting this variable to \"true\" is the same as setting\n\tthe `text` attribute to \"auto\" on all files and core.eol to \"crlf\".\n\tSet to true if you want to have `CRLF` line endings in your\n\tworking directory and the repository has LF line endings.\n\tThis variable can be set to 'input',\n\tin which case no output conversion is performed.\n"},{"id":"364700","messageId":"d04ff31e-c8e9-23fe-6ed1-597e32c1f4ab@googlemail.com","threadId":"49886","inReplyTo":"xmqq1s6vbhdr.fsf@gitster-ct.c.googlers.com","subject":"Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF","fromName":"Frank Schäfer","fromEmail":"fschaefer.oss@googlemail.com","sentAt":"2018-12-06T18:42:19Z","receivedAt":"2018-12-06T18:41:24Z","isPatch":false,"sender":{"key":"fschaefer.oss@googlemail.com","avatar":null},"body":"\nAm 06.12.18 um 01:58 schrieb Junio C Hamano:\n> Frank Schäfer writes:\n>\n>> Just to be sure that I'm not missing anything here:\n>> What's your definition of \"LF in repository, CRLF in working tree\" in\n>> terms of config parameters ?\n> :::Documentation/config/core.txt:::\n>\n> core.autocrlf::\n> \tSetting this variable to \"true\" is the same as setting\n> \tthe `text` attribute to \"auto\" on all files and core.eol to \"crlf\".\n> \tSet to true if you want to have `CRLF` line endings in your\n> \tworking directory and the repository has LF line endings.\n> \tThis variable can be set to 'input',\n> \tin which case no output conversion is performed.\nArgh... crap. I was missing that I have to set the \"text\" attribute\nmanually...\nThats why core.eol=crlf didn't make a difference in my tests. :/\n\nLet me thoroughly re-validate all cases.\nI will likely not find the time for that before Monday, but I think that\nbreak could be helpful. ;)\n\nRegards,\nFrank\n\n"}]}