0.8 -> 0.10 update broke my codebase

[matthiasbeyer]

First of all: The title is a bit misleading. Of course a minor-version update is allowed to introduce breaking change and of course I'm not complaining about that, it is totally fine.

Second: I do not know whether this can be fixed at all, depending on what you want to do and where you want to go with that crate. So this is more of a feedback than of a real issue or a change request. Please do not think I want you to roll back to the old API design of the crate, I'm sure you had your reasons.

---

Now, lets go. I have this infrastructure of a trait Viewer, where a Viewer can view entries to either a terminal output or something else. I have a Viewer implementation which does markdown rendering and prints it with mdcat.

I'm in the process of updating mdcat from 0.8 to 0.10 - but as the API design changed, I cannot do this.

First of all, I would need to fetch a new instance of Terminal in each iteration (in the implementation of the Viewer::view_entry() fn). I can do this: AnsiTerminal::new(sink), but as sink is not + 'static, it won't work. As Terminal is not Clone, I cannot get an instance of a Terminal in MarkdownViewer::new() and simply clone it to mdcat::push_tty()...

So right now I'm a bit lost here.

Maybe You have an idea how to solve this issue, maybe I will have to stick with the 0.8 version of mdcat.

[swsnr]

Thanks for your feedback. I'm sorry that my changes broke your code, but as hard as it sounds, I can't help you. I wrote mdcat for myself, as a simple CLI tool, and that's all I want from it, and just don't want to spent my time on someone else's use cases. I'm sorry but you're on your own if you're using mdcat as a library in your own code.

I can try to help you a bit but please understand that I do not have the time to look at your code in detail.

I don't think I understand your problem tho. As said I didn't look at your code, so excuse me if I'm asking a stupid question, but

• why do you need to clone a new instance of Terminal in every iteration?
• Why do you even need a fresh instance? Why not just create one instance and pass it down, just like mdcat itself does it?
• And why would sink need to be 'static?! Any reference to Write also implements Write, so why can't you create a single sink and pass a mut reference to AnsiTerminal?

[matthiasbeyer]

but as hard as it sounds, I can't help you. [...] I do not have the time to look at your code in detail.

I'm completely fine with that! I expected that reply! 😄

why do you need to clone a new instance of Terminal in every iteration?

Because push_tty takes ownership of the first parameter. The context is that I have a functor which I apply for each output, I do not pass all output to one push_tty call... the architecture of the whole ecosystem does not allow that (on purpose tho).

That's also why I need a new instance for each iteration.

And why would sink need to be 'static?! Any reference to Write also implements Write, so why can't you create a single sink and pass a mut reference to AnsiTerminal?

Because of the Box<_> the first parameter of push_tty takes. I have a &mut W, where W: Write and I can put that in a AnsiTerminal, but as soon as I put the AnsiTerminal in a Box, I have to have where W: Write + 'static.

[swsnr]

I see. Perhaps push_tty doesn’t need to take ownership? I didn’t give much thought about it 💁‍♂️

Feel free to take a look, I’ll accept a pull request that improves the API 😊

[matthiasbeyer]

I had a look. It is hard to rewrite everything so that push_tty gets a &mut... Maybe I'll succeed... don't know.

[swsnr]

@matthiasbeyer I don't know, the change was rather simple 🤷‍♂️ In the next release push_tty won't take ownership of the terminal anymore.

[swsnr]

@matthiasbeyer 0.11 is out. Would appreciate feedback whether the changed API suites your needs now.

[matthiasbeyer]

Well yeah, I was able to update from 0.8 to 0.11. Thanks!

[swsnr]

You’re welcome 🙂