ur0 / lolcat

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

Lolcat correctly interprets escape sequences - Fixes #14 #18

Closed magnus-ISU closed 2 years ago

magnus-ISU commented 2 years ago

This allows fun things like sl | lolcat and htop | lolcat.

They still look sort of weird, but I think there isn't a great way to get that to work, as I think the weirdness depends on the order of escape sequences given to lolhelp more than anything lolhelp is doing.

As for the change in behavior, previously if lolcat encountered an escape sequence, it (attempted to) ignore it without printing it. It was also a bad implementation as it didn't really know what an escape sequence was. Now, it will echo all escape sequences without changing them (including ones that affect color - in that case it doesn't matter though, because before it prints the next character it change the color again for the lolz). Stdin is no longer buffered by newlines either.

Restructures quite a bit of the code to use iterators rather than str. I tried to make it clean, but the parsing of escape sequences is also a giant mess which probably can't be written cleaner anyway, unless maybe we found a crate to do it.

Imports utf8-chars crate to be able to handle stdin that isn't broken up by newlines.

Does not attempt to gracefully recover from an incorrect escape sequence. However, I've piped a few ncurses programs into it and haven't found anything that causes an error here.

For some reason it doesn't really work with pacman progress bars so more work can probably be done.

magnus-ISU commented 2 years ago

Forgot to mention; this PR also makes it so lolcat -h can accept other flags, such as lolcat -hBD

magnus-ISU commented 2 years ago

Added the -F flag which forces printing color, even when not printing to a terminal. Bumped the program version to 1.4.0 since the handling of escape sequences is a pretty substantial change imo.

magnus-ISU commented 2 years ago

Also, I can take you up on maintaining this going forward. I understand the codebase now, so while I still don't know what more might need to be changed I can definitely get it done if anything comes up.

ur0 commented 2 years ago

@magnus-ISU just a couple of minor notes on the PR, but otherwise this LGTM. Also added you to the collaborators!

magnus-ISU commented 2 years ago

Okay, awesome. I will revert deletion of Cargo.lock as you are correct and I forgot about this. Nice catch.