grep --color=always ... | less -R garbled since 580

[vain]

How to reproduce:

$ echo hello world >foo
$ grep --color=always . foo | less -R
(END)

I would expect it to print that one matching line (colored):

$ echo hello world >foo
$ grep --color=always . foo | less -R
hello world
(END)

It’s because grep uses lots of “Erase in Line” escape sequences:

https://git.savannah.gnu.org/cgit/grep.git/tree/src/grep.c?id=54aac0789909618122d7125854c443f26a94f610#n317

I am not sure how “Erase in Line” is supposed to be interpreted. My VTE terminal and XTerm only erase characters that were already on screen:

$ printf 'hello\ra\n'
aello
$ printf 'hello\r\e[Ka\n'
a

Piping the second command through less erases the a as well and I guess that’s not correct?

(I also see some more corruption like blinking output or misplaced cursors, but I have no idea why that happens, so I can’t provide a meaningful but report for that.)

Let me know if there’s anything else I can do to help.

Thanks!

– edit: Forgot to mention it, this is on Arch Linux with less 580, grep 3.6, XTerm 366, and vte3 0.64.0.

[heftig]

cc: @keszybz @tomprince @poettering

Arch upgraded less to support systemd's hyperlinks (#54). Is Fedora now also suffering from this bug?

[heftig]

Bisection led to the commits implementing hyperlink support.

The first bad commit could be any of: 0f810ef 3087a2c

[gwsw]

This is mostly fixed in 7928864. Thanks to @heftig for the fix.
Horizontal scrolling is still not working with colored grep output. This is apparently due to the (entirely unnecessary) line erase sequences that grep is adding to the output for some reason. As documented, less -R is not guaranteed to work correctly in the presence of escape sequences other than SGR coloring sequences. If the line erase sequences are removed it works correctly:

echo hello | grep --color=always . | sed 's/\o33\[K//g'  | less -R

I will investigate whether there is some easy workaround to ignore these line erase sequences, but it's rather annoying that grep is putting this junk into its output.

[heftig]

This is mostly fixed in 7928864. Thanks to @heftig for the fix.

@gwsw I see you didn't take the hunk changing the hindex increment in ansi_step. The current code (below) doesn't make much sense: pansi->hindex == 0 cannot ever be true, as it is incremented first and thus always greater than zero. So the code can check for ESC (part of the hlink_prefix string) but not the alternative CSI start character.

	if (ch == hlink_prefix[pansi->hindex++] ||
		    (pansi->hindex == 0 && IS_CSI_START(ch)))

[gwsw]

@heftig Thanks for pointing out that omission. Fixed in 3ed1315, as well as the scrolling issue when unrecognized escape sequences are in the file.

[vain]

Hi, just wanted to confirm that I can no longer see the bug in v581. Thank you!