bat
bat copied to clipboard
Remove code that tries to handle ANSI escape inputs
Proof of concept to aid discussion in #2185. I have not made up my mind yet if this change in sensible, but I think it helps to see it in person. Make sure to ignore whitespace changes when diffing.
IMO this problem is not a blocker to get the v0.21.0 release out since this is not a change in behaviour compared to v0.20.0.
The main use case for the old code seems to be that colored input across newlines should be handled correctly. If you try this:
echo "\x1B[33mColor\nColor \x1B[m\nPlain\n" | ./target/debug/bat
it looks like this with cat
:
![Skärmavbild 2022-05-10 kl 08 42 04](https://user-images.githubusercontent.com/115040/167564591-4403187a-eb4e-4842-acf8-876a97d80834.png)
and the same with the latest release of bat
, but with the code in this PR it looks like this instead:
![Skärmavbild 2022-05-10 kl 08 43 00](https://user-images.githubusercontent.com/115040/167564731-9d476840-efbc-472e-a6e2-789b7ffdcc10.png)
since we reset colors when we encounter \n
it seems. But IMHO we should just consider ANSI escape inputs as out-of-scope for bat, since syntax highlighting is broken when input contains ANSI escape characters anyway.
I have also confirmed that this change makes bat
show bat-bug-test
in #2185 the same way as cat
.
So what are the things that couldn't be done anymore when removing the special ANSI sequence handling? CC @eth-p
Apart from the use case I described from the start, I know of no additional use of the ANSI escape handling code. And worth emphasising: the current code breaks the use case in #2185. I suppose it is possible to fix it. But is it worth it?
I would also greatly appreciate any input you might have here @eth-p.
Apart from the use case I described from the start, I know of no additional use of the ANSI escape handling code.
Oh, sorry. I misread your post. I understand now.
No objections have been raised, so I think we can go ahead and merge this. We can always change our mind later and bring back the code, if we want to.
I will wait another week or so, and then rebase and merge this unless any objections are raised.
3 months has passed since last release, and there a bunch of nice fixes waiting to be released, so it would be nice to get a release out in the coming weeks, IMHO.
CC @sharkdp @keith-hall @eth-p