-
-
Notifications
You must be signed in to change notification settings - Fork 144
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Switch to byte offset spans for strings by default #65
Comments
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.
For now, yes. That too is likely to change.
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. |
I'm going to rename this issue given that zero-width EOIs has now been implemented in 60cba22. |
Thanks!
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). |
Here is an example code that converts byte-based It involves creating a 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)))
} |
codespan
andmiette
do that. Isariadne
different?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). Andcodespan
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.The text was updated successfully, but these errors were encountered: