Publicly expose the TerminalWrite trait

[Byron]

It's visible in the push_tty function, and should thus
be exposed as well.

Maybe I am missing something, but that's the first minor bump I hit when trying to create my own implementation.

[swsnr]

Hey thanks, I'll definitely merge. Looks like Travis isn't happy, tho, can you take a look?

PS: I'm very happy that you want to build something cool on top of mdcat, but I should warn you; as far as I'm concerned I don't think that mdcat's internals are ready to be exposed. The implementation is a bit rough in some corners, and I'm not even remotely sure that the current public interface is any good 😊 See #18 for more infos.

So, expect that the API breaks when I add new features or refactor things. I hope you don't mind.

PPS: I'd also appreciate help on a good API, so please feel free to open more pull requests to expose new stuff or refactor things to suite your needs. I never wrote a public Rust crate before, so I don't know much about the whole business here 😊

[Byron]

Sorry for the sloppiness, I really didn't expect that lint could be in my way :D!

No worries about changing things around mercilessly, as long as you adhere to semver everyone using the library should be fine.

As far as I am concerned, it's already doing well enough for me, as I already have something usable even without this PR.

Something I found a bit odd is the performance, as it really seems to consume a lot of time during initial setup (in debug mode) - maybe it's the syntect syntax set which has to be initialized once per invocation of the push_tty function. Technically it should be alright to pass it as reference to allow reusing it.

As references usually add complexity, it might also be suitable to return the consumed resources as part of the Ok(...) when done.

[Byron]

Something I was also thinking about is if mdcat should not pretty-print things if the output is not a tty.
This crate can be used to determine this, and would make my snapshots look much prettier :).

What do you think about that? With the above trait exposed, I could also implement it that way on my end.

[Byron]

Sorry for the spam, but look at this!! Not bad for a first proof of concept :D!

[swsnr]

@Byron Woa, cool, looks amazing 😍 I'm happy that my little thing helped you do this 😊

No worries about changing things around mercilessly, as long as you adhere to semver everyone using the library should be fine.

Thanks 😊

Something I found a bit odd is the performance, as it really seems to consume a lot of time during initial setup (in debug mode) - maybe it's the syntect syntax set which has to be initialized once per invocation of the push_tty function. Technically it should be alright to pass it as reference to allow reusing it.

As references usually add complexity, it might also be suitable to return the consumed resources as part of the Ok(...) when done.

So… I got no idea what you're talking about here 🙈 My Rust is still pretty weak 😊 Can you elaborate a bit? Perhaps even make a pull request to optimize this? I'm really curious on how that might look like, and would love to learn more 😊

Something I was also thinking about is if mdcat should not pretty-print things if the output is not a tty.
This crate can be used to determine this, and would make my snapshots look much prettier :).

In fact, mdcat already behaves this way: When the output is no TTY it uses a "Dumb" terminal to disable formatting, and it even uses atty for this ☺️

The logic for this is in main.rs though, so it's not there for re-use yet. Feel free to move it into the lib tho—again, I'm happy to merge any pull request that makes mdcat a better library ☺️

[Byron]

In fact, mdcat already behaves this way: When the output is no TTY it uses a "Dumb" terminal to disable formatting, and it even uses atty for this ☺️

Thanks for the hint! Actually I found it myself when I was removing all the proof-of-concept code, which was part of the problem.

Something I would need to make it better is access to that now publicized trait.
What's your release plan? Maybe you could create a patch release for me?
Thanks a lot for your consideration! I am quite excited to work on this - it's quite rare we get to do eye-candy :D!

[swsnr]

@Byron I pushed 0.8.0; made it a new minor version, since I also added (rough) SVG support for iTerm2 😃

[Byron]

Thank you! I just realized that I can't actually implement the trait myself as trait specializations have not yet landed in Rust.
However, there is a workaround which works just fine.

For me the current results are already good enough, great work there!