pulldown-cmark icon indicating copy to clipboard operation
pulldown-cmark copied to clipboard

Use `std::fmt::Write` instead of custom `StrWrite` trait

Open camelid opened this issue 5 years ago • 5 comments

Closes #492.

camelid avatar Oct 09 '20 22:10 camelid

The implementation looks good, but I just now noticed that fmt::Write has its own error. This means that it's no longer possible to pass along IO errors. If a write fails to a Write object, the user will have no way to discover why. This seems like a serious drawback of the change.

Yeah, I agree :/

Maybe not worth doing for now.

camelid avatar Oct 11 '20 18:10 camelid

@marcusklaas Not sure if you're on the Rust Zulip, but I posted this: https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/fmt.3A.3AResult.20doesn't.20allow.20having.20an.20error.20message

camelid avatar Oct 11 '20 18:10 camelid

From that topic:

simulacrum: It is perhaps worth noting that io::Write::write_fmt does preserve the io Error in the raw form (https://doc.rust-lang.org/nightly/src/std/io/mod.rs.html#1498-1513) simulacrum: so if you're using write! or similar with io sources, and not the fmt trait directly, you'll get the error

From my understanding, our use case falls under the former category, so is this still considered a blocker?

edward-shen avatar Dec 02 '20 00:12 edward-shen

I wonder if this PR could be extended to allow a more generic representation of push_html(&mut String,..)? I'm running into a related problem where a template library (Sailfish) has a Buffer type that impls fmt::Write, but not io::Write for similar reasons (i assume) that this library originally wrote StrWrite.

As far as i can tell this requires users of Sailfish to allocate a secondary String to receive html from this lib. Making the push_html generic seems a good way to deal with this, and fmt::Write seems a possible candidate to support that with this PR. Thoughts?

leeola avatar Jul 02 '23 23:07 leeola

This is an old pull request and in draft state, it should be redesigned and thought again. Sorry, but this will not happen in the short term unless someone else begin to work in this.

Martin1887 avatar Jul 04 '23 14:07 Martin1887