Switch to byte offset spans for strings by default

[tailhook]

1. Why it counts characters and not byte offsets? It's hard to spot this thing deep in the documentation. And also most error reporting libraries expect byte offsets, codespan and miette do that. Is ariadne different?
2. End of input span is x..x+1 by default. I believe it should be zero-length. miette just skips the label on out of range label (i.e. doesn't display). And codespan even crashed previously (it maybe fixed now, not tested recently).

It's easy to fix both in my code using Stream::from_iter, but defaults are confusing.

[zesterer]

Why it counts characters and not byte offsets? It's hard to spot this thing deep in the documentation.

I'm planning to switch to byte offsets in the future (as the doc comment says). They do have a number of advantages over character indices (O(1) indexing, for example) but also the potential for problems (spans need access to the original string to verify their correctness), so it's not a clear win.

And also most error reporting libraries expect byte offsets, codespan and miette do that. Is ariadne different?

For now, yes. That too is likely to change.

End of input span is x..x+1 by default. I believe it should be zero-length.

I think that's reasonable. The reason for this initial decision is related to a change that occurred some time ago during the library's development but is no longer relevant. A zero-width range is fine. I can make that change soon.

[zesterer]

I'm going to rename this issue given that zero-width EOIs has now been implemented in 60cba22.

[tailhook]

Thanks!

Why it counts characters and not byte offsets? It's hard to spot this thing deep in the documentation.

I'm planning to switch to byte offsets in the future (as the doc comment says). They do have a number of advantages over character indices (O(1) indexing, for example) but also the potential for problems (spans need access to the original string to verify their correctness), so it's not a clear win.

Is there any computations on spans that chumsky is or might be doing in future other than just merging them (latter doesn't require validation)? There is so small number of computations that can be done on character (unicode codepoint) level that it usually doesn't make any difference. I.e. you can't rely next character being next column in text (because char != grapheme != width 1 column).

[m13253]

Here is an example code that converts byte-based chumsky::span::SimpleSpan into rune-based ariadne::Label::span.

It involves creating a Vec to store byte-to-rune mapping. I don’t think there are any ways to eliminate this step. You might want to cache it if your parser involves multiple input files.

use std::fmt::Display;
use std::io;
use std::rc::Rc;

use ariadne::{ColorGenerator, Label, Report, ReportKind, Source};
use chumsky::prelude::*;

pub fn print_error<T: Display>(
    source: &[u8],
    error: &Rich<T, SimpleSpan<usize, Rc<str>>>,
) -> io::Result<()> {
    let source_str = String::from_utf8_lossy(source);
    let source_idx = source_str
        .char_indices()
        .map(|(idx, _)| idx)
        .collect::<Vec<_>>();
    let byte_to_rune = |span: &SimpleSpan<usize, Rc<str>>| {
        source_idx.partition_point(|&idx| idx < span.start())
            ..source_idx.partition_point(|&idx| idx < span.end())
    };

    let mut colors = ColorGenerator::new();

    let msg = format!("{error}");
    let span = error.span();
    let filename = &span.context();
    Report::build(ReportKind::Error, filename, 4)
        .with_message(&msg)
        .with_label(
            Label::new((filename, byte_to_rune(span)))
                .with_message(&msg)
                .with_color(colors.next()),
        )
        .finish()
        .eprint((filename, Source::from(source_str)))
}