ur0 / lolcat

lolcat, now with fearless concurrency.
MIT License
99 stars 14 forks source link

`sl | lolcat` renders incorrectly #14

Closed ellsclytn closed 2 years ago

ellsclytn commented 4 years ago

This recording probably sums it up best.

magnus-ISU commented 2 years ago

This issue is very old, but does anyone have any idea why this happens? sl | cat works, so there is something weird going on.

ellsclytn commented 2 years ago

I assume it will be similar to the issue faced with the Ruby version of lolcat, wherein it wasn't properly handling escape sequences for some time.

magnus-ISU commented 2 years ago

Finally got some time to look at it.

It appears that this has that problem also, where it is emitting an \e0m rather than using the proper escape sequence to not reset other colors.

However, this has a more serious problem that literally any escape sequence is just entirely ignored, up and through an m character. (this doesn't seem to be the behaviour from back in 2020, but it is now). Piping sl to a file, I see the way it will be parsed is a few escape sequences, one of which is terminated by an m, then immediately a bunch of escape sequences which are not terminated by an m and thus the entire rest of sl is ignored, though it does print a couple newlines because of a different hack that seems to have been used.

The proper way to handle this is (in my opinion) to read the full escape sequence, try to understand it; if it is a color, add whatever lolcat would do to it (essentially inverting the color, probably, might be cooler than just leaving it as is); if it is not a color, send it to the standard output the way it came in. It needs to be able to differentiate between characters in and out of an escape sequence, so I will have to rewrite large parts of cat.rs I think. However, I believe I understand the problem now (I didn't back in September for some reason, honestly still don't understand exactly how escape sequences work, but I get the gist of the issue) and so will be able to do what is required.

From the comments on the Ruby version, it sounds like ncurses apps should also be able to work with a properly written lolcat. If so, that will be cool to see.

I believe I should be able to avoid using any regex also, since when an escape sequence starts is unambiguous (\x1b (\e) begin every one I think?) and we're not dealing with any buffering of input like the Ruby version does.

magnus-ISU commented 2 years ago

Also, is there any point to resetting the color? Since the next printable character will have its color set anyway, it seems like needless complexity. Though I don't think removing it helps with anything, it doesn't hurt either, or am I wrong.

magnus-ISU commented 2 years ago

I've been working on it for a bit, and I believe there is no reason to reset the color. However, I've run into a bigger issue, which is that the input is buffered by newline, but sl for example does not create newlines every time it should refresh.

See my fork, it is pushed. sl creates a single newline which lets you see this but then it is frozen until sl terminates a while later when it closes itself. image

magnus-ISU commented 2 years ago

Fixed in #18

Akari202 commented 2 years ago

I am still encountering this issue, Neofetch gets mangled and so does sl, I have version 1.3.2 installed but lolcat still seems to handle escape sequences improperly

Screen Shot 2022-08-13 at 09 57 25
magnus-ISU commented 2 years ago

This is probably related to #21. Here's what I have: image

magnus-ISU commented 2 years ago

For now, you can probably do the following:

1st uninstall lolcat. Then

cd /tmp
git clone git@github.com:ur0/lolcat.git
cd lolcat
cargo build --release

For a system-wide install:

sudo cp target/release/lolcat /usr/local/bin

For a user install without requiring root

cp target/release/lolcat ~/.local/bin

EDIT: those instructions would be for linux, I'm not sure exactly where apps go on Mac. ~/.cargo/bin might also be the place to check?

magnus-ISU commented 2 years ago

You should now also be able to just do cargo install lolcat