Skip to content
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

Nonempty labels with zero visible width are rendered wrong by graphical handler #224

Closed
Benjamin-L opened this issue Nov 4, 2022 · 3 comments

Comments

@Benjamin-L
Copy link
Contributor

Miette currently renders labels covering an empty span with an arrow character (), while labels covering a nonempty span are rendered with an underline. This is useful, since it would otherwise be impossible to distinguish between an empty label pointing to the left of a character from labels covering that character.

The issue is that, after #202, a span covering a nonempty byte range doesn't necessarily mean that it covers a nonzero number of visible columns in the output. IMO, when a span is pointing at a zero-width character, it should use the pointer rather than . In the example below, the expected output is clear that it's pointing to something inside the parentheses, while the current output looks like it's pointing to the ) character itself.

Steps to reproduce

Source: ZWSP inside this: (\u{200B})\n
Span: (19,3)

Unit test
#[test]
fn single_line_highlight_with_zero_width_space() -> Result<(), MietteError> {
    #[derive(Debug, Diagnostic, Error)]
    #[error("oops!")]
    #[diagnostic(code(oops::my::bad), help("try doing it better next time?"))]
    struct MyBad {
        #[source_code]
        src: NamedSource,
        #[label("this bit here")]
        highlight: SourceSpan,
    }

    let src = "ZWSP inside this: (\u{200B})\n".to_string();
    let err = MyBad {
        src: NamedSource::new("bad_file.rs", src),
        highlight: (19, 3).into(),
    };
    let out = fmt_report(err.into());
    println!("Error: {}", out);
    // There's a literal ZWSP inside the parens for this string, because raw
    // strings don't support escapes.
    //
    // It may or may not show up in your editor :)
    let expected = r#"oops::my::bad

  × oops!
   ╭─[bad_file.rs:1:1]
 1 │ ZWSP inside this: (​)
   ·                    ▲
   ·                    ╰── this bit here
   ╰────
  help: try doing it better next time?
"#
    .trim_start()
    .to_string();
    assert_eq!(expected, out);
    Ok(())
}

Expected output

  × oops!
   ╭─[bad_file.rs:1:1]
 1 │ ZWSP inside this: (​)
   ·                    ▲
   ·                    ╰── this bit here
   ╰────
  help: try doing it better next time?

Current output

  × oops!
   ╭─[bad_file.rs:1:1]
 1 │ ZWSP inside this: (​)
   ·                    ┬
   ·                    ╰── this bit here
   ╰────
  help: try doing it better next time?
@Benjamin-L
Copy link
Contributor Author

In #221, @Boshen mentioned that they expected to be used when pointing at a 1-column span, not just a 0-column span. I would lean towards keeping for 1-column spans, but I think the right answer here isn't entirely clear.

@zkat
Copy link
Owner

zkat commented Nov 24, 2022

I prefer keeping the way it is now: its purpose is to say "actually, this is different, and does not cover any length of span", vs "this is the range that this covers, and it happens to be 1 in length"

@zkat zkat closed this as not planned Won't fix, can't repro, duplicate, stale Nov 24, 2022
@Benjamin-L
Copy link
Contributor Author

Yeah, that makes sense.

Important to note that the current meaning isn't "this is the range that this covers, and it happens to be 1 in length" anymore. Since visible columns don't necessarily correspond to the byte range the span covers, it's "this span covers more than zero bytes, but may cover zero columns of the rendered output". In the ZWSP example earlier, the range that is rendered under doesn't correspond to the range the span actually covers, since the ) character is not included.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants