Nemo157 / maud-pulldown-cmark

An adapter to allow rendering strings as markdown inside a maud macro using pulldown-cmark efficiently.
Apache License 2.0
5 stars 2 forks source link

Handle "invalid" input correctly #9

Open Nemo157 opened 7 years ago

Nemo157 commented 7 years ago

At the moment with maud v0.11.0 if a consumer of this library passes its markdown through a filter generating an unexpected series of events then maud-pulldown-cmark will exit early with an Err response, maud will just ignore the error and continue rendering the next part of the template.

An example:

#![feature(plugin)]
#![plugin(maud_macros)]

extern crate maud;
extern crate maud_pulldown_cmark;
extern crate pulldown_cmark;

use maud_pulldown_cmark::Markdown;
use pulldown_cmark::{Event, Tag};

fn main() {
    let events = vec![
        Event::Start(Tag::Image("http://example.com/image".into(), "title".into())),
        Event::Start(Tag::Code),
        Event::Text("hello".into()),
        Event::End(Tag::Code),
        Event::End(Tag::Image("http://example.com/image".into(), "title".into())),
    ];

    let buffer = html!(
        div {
            (Markdown::from_events(events.into_iter()))
        }
    );

    println!("{}", buffer.into_string());
}

saved to examples/invalid.rs and run with cargo run --example invalid gives:

<div><img src="http://example.com/image" alt="</div>

After the update to maud v0.12.0 in #8 the same example now gives:

<div><img src="http://example.com/image" alt="hello" title="title" /></div>

which is more valid, but still is silently dropping some of the input. Hopefully there is some way to report this sort of issue back to the user.

TheNeikos commented 7 years ago

Is this possible to generate with 'pure' markdown? And not just events?

Nemo157 commented 7 years ago

Not that I'm aware of.

TheNeikos commented 7 years ago

As that seems to be the use case number one I don't think it is a big issue.

However I would definitely note somewhere that invalid input might cause invalid output. I do agree though that it shouldn't silently drop errors.

What about adding a 'check' method?

Nemo157 commented 7 years ago

I was thinking about something like that, but not sure how possible that is with the support for iterating the events just once. I guess pulldown_cmark::Event would need to have impl Clone added then a clone of the event stream could be checked. Other big downside is that no-one would probably even use it since it's not how maud is really intended to be used.

Another alternative may be to have a log feature that hooks into something like https://github.com/rust-lang-nursery/log since a lot of applications would probably be using it.

I'm not planning on doing anything for this right now, I need to take a look at rust's logging libraries for another project anyway so I'll post anything that looks relevant from that.

TheNeikos commented 7 years ago

What about a lazy enum? If it succeeds you can replace itself with the computed value