zesterer / ariadne

A fancy diagnostics & error reporting crate
https://crates.io/crates/ariadne
MIT License
1.61k stars 72 forks source link

Support more newlines #64

Closed TRCYX closed 1 year ago

TRCYX commented 1 year ago

Corresponds to Section 5.8 of the Unicode Standard, as well as chumsky::text::newline.

zesterer commented 1 year ago

Thanks! Any chance of a few tests that include these cases? This seems like code that's easy to accidentally break.

TRCYX commented 1 year ago

Where should I put them? In examples or as cargo tests?

zesterer commented 1 year ago

I think unit tests at the bottom of the same file would be fine, like:

#[cfg(test)]
mod tests {
    #[test]
    fn foo() {
        ...
    }
}
TRCYX commented 1 year ago

Thanks for the idea of tests. The original code was wrong in the len field of a Line ending with CRLF, neglecting the LF. To solve this problem the code becomes considerably uglier :)

TRCYX commented 1 year ago

Tests are added and I force-pushed to keep the history clean.

zesterer commented 1 year ago

Thanks for the idea of tests. The original code was wrong in the len field of a Line ending with CRLF, neglecting the LF. To solve this problem the code becomes considerably uglier :)

Hehe, I suspected as much. Logic like this is often surprisingly difficult to get right. I feel much more comfortable merging this with tests!