Canop / termimad

A library to display rich (Markdown) snippets and texts in a rust terminal application
MIT License
914 stars 31 forks source link

Update to crossterm 0.20.0 #29

Closed EverlastingBugstopper closed 3 years ago

EverlastingBugstopper commented 3 years ago

When using crossterm 0.20.0 with termimad 0.11.1, I get the following error:

error[E0308]: mismatched types
   --> src/command/output.rs:125:36
    |
125 |                 skin.bold.add_attr(Underlined);
    |                                    ^^^^^^^^^^ expected enum `crossterm::style::types::attribute::Attribute`, found enum `crossterm::style::Attribute`
    |
    = note: perhaps two different versions of crate `crossterm` are being used?

error[E0308]: mismatched types
   --> src/command/output.rs:125:36
    |
125 |                 skin.bold.add_attr(Underlined);
    |                                    ^^^^^^^^^^ expected enum `crossterm::style::types::attribute::Attribute`, found enum `crossterm::style::Attribute`
    |
    = note: perhaps two different versions of crate `crossterm` are being used?

It looks to me as if there were some breaking changes in the crossterm crate that just moved where the types live. termimad should update their API to accept this new type, and possibly consider re-exporting the types that are expected to be used within the skin builder API so consumer crates don't have to worry about matching dependency versions.

Canop commented 3 years ago

I'm not sure about reexporting. When you use termimad, you probably also directly use crossterm.

EverlastingBugstopper commented 3 years ago

Yeah, that part is definitely not necessary to fix this - just a possible path. I only suggest it since the termimad API directly accepts input parameters thats types are not part of this crate. An example of this pattern in the ecosystem is reqwest which re-exports the types from the http crate so you don't have to include it as a dependency.

Canop commented 3 years ago

I also often reexport types in my crates, but mostly when you don't need to use the other types directly. My opinion isn't yet made on this point.

Canop commented 3 years ago

done: versions of Termimad 0.12.x are based on Crossterm 0.20

EverlastingBugstopper commented 3 years ago

Thanks!!