PoiScript / orgize

A Rust library for parsing org-mode files.
https://poiscript.github.io/orgize/
MIT License
278 stars 34 forks source link

Debug Panic on Parsing Table #4

Closed dpbriggs closed 4 years ago

dpbriggs commented 4 years ago

Hi! I wanted to thank you for making this great crate. I'm currently using orgize to add org rendering to crates.io and the background workers are panicking when they encounter tables:

use orgize::Org;

const org_contents: &'static str = r#"
|-----+-----+-----|
| foo | bar | baz |
|-----+-----+-----|
|   0 |   1 |   2 |
|   4 |   5 |   6 |
|   7 |   8 |   9 |
|-----+-----+-----|
"#;

fn main() {
    let mut buf = Vec::new();
    Org::parse(org_contents).write_html(&mut buf);
    println!("Hello, world!");
}

This isn't a problem, per se, but would it be possible to avoid panicking? Ideally we'll have a

pub fn try_parse_custom(text: &'a str, config: &ParseConfig) -> Result<Org<'a>, Vec<ValidationError>>

so we could recover from malformed org-mode files.

I could open a PR if you'd like :P

PoiScript commented 4 years ago

Thanks for bring this up! This's definitely a bug. The parser takes the trailing newline as a table row by mistake:

Screenshot_20191105_145613

It shouldn't be hard to fix and I'm working on it right now.

but would it be possible to avoid panicking?

Unfortunately, no. The parser will not paincs in any cases and there's no need to avoid.

so we could recover from malformed org-mode files.

In fact, Org::validate is used for validating the Org struct itself, not the input string. Actually, any string is a validate org-mode string and Org::parse is guaranteed to return a valid Org struct.

To sum up, if you're just parsing and rendering org-mode files, you don't need to validate anything. Calling the Org::validate function only when you have modified the inner arena via Org::arena_mut.

PoiScript commented 4 years ago

Also, I'm very happy to know that you're bring the org-mode feature to crates.io! Feel free to contact me at any time if you have problem about orgize.

PoiScript commented 4 years ago

Ok, the bug is fixed now. There's still something I want to change also, so I will release it a bit late.

Also, I had updated the panic message. Like I explained above, Org::parse is expected to return a valid Org struct.

https://github.com/PoiScript/orgize/blob/5d5fc5802742ed5ce6cda1a50a12bbbcf4ee3c04/src/validate.rs#L187-L195

dpbriggs commented 4 years ago

Hey thank you so much! You're the best