atanunq / viuer

Rust library for displaying images in the terminal.
MIT License
241 stars 43 forks source link

Extensions of print* functions with custom output streams #31

Closed Eliah-Lakhin closed 2 years ago

Eliah-Lakhin commented 2 years ago

Viuer is awesome! But its capabilities could be utilized even further if we let users customize output streams. Current public API helper functions are sealed with stdout, so the user doesn't have alternatives rather than printing images directly to stdout. I added two new helper functions write and write_from_file that work exactly the same way as print* functions do, but they also accept &mut impl Write as a first additional argument.

The primary motivation for me to introduce this extension is making a custom implementation of the Rust Env Logger for my own graphics project, such that the logger will be able to print images to terminal for the logs referring GPU backed image in debug messages. This is practically useful for visual debugging of the rendering process, and is a nice feature to have. The difficulty here is that the Logger Builder callback tied to the Formatter which in turn is a Write implementation, and requires of buffering printing data beforehand. If I use just print/print_from_file functions the images would come to console before the log message.

All in all with these new functions the end result looks good: Screenshot from 2021-09-26 13-56-59

I also added MIT License file to this repo to match the record in Cargo manifest.

Eliah-Lakhin commented 2 years ago

I also added a newline config option that prevents adding a new line to the output stream. It's value is true by default, so this is not a breaking change.

Eliah-Lakhin commented 2 years ago

Just figured out that this solution does not work well with BlockPrinter, because the block printer is tied to BufferedStanardStream backed by stdout... so, the feature seems to be not generic enough. What do you think?

atanunq commented 2 years ago

Woah, this is a very cool use case! I haven't though about making the write interface public but it does make sense.

Previously, I've made an attempt to abstract the print calls within the Printer trait to target &mut impl Write but couldn't figure out how to work around the limitation you pointed out in termcolor. Also, sixel is a whole 'nother (experimental) beast that I haven't yet understood.

I'm a bit hesitant to merge this because the block printer is considered the "default" and new functionality should work with it as a minimum, imo. If we can't come up with something, we could also ask for help upstream.

Eliah-Lakhin commented 2 years ago

@atanunq Thank you for review.

I don't know how to fix it too, because termcolor's WriteColor is sealed. There are several ways on how to workaround it. e.g. we can statically limit introduced write functions to only work with iTerm and Kitty, or expose Printer interface(again for iTerm and Kitty only), or doing the opposite -- limit "write" functions to always accept termcolor's WriteColor interface only. Either way has its drawbacks. Maybe the only right way would be just give up and close this PR :) To be honest I'm trying to use your crate the way it was not designed for from the beginning. But it's up to you, I can try to implement one of these workarounds and we merge it. Also, perhaps another way would be getting rid of termcolor and write BlockPrinter manually without the termcolor capabilities. I think it should be possible to do, but it would be hard in debugging and implemention both, not something I can do atm :)

Eliah-Lakhin commented 2 years ago

@atanunq I'm closing this Pull Request as I found suitable but lesser generic solution to this problem for my project that doesn't require any changes in Termion.