Skip to content

Off-by-one error in multiline error highlights #41

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

Closed
digama0 opened this issue Feb 11, 2021 · 2 comments · Fixed by #42
Closed

Off-by-one error in multiline error highlights #41

digama0 opened this issue Feb 11, 2021 · 2 comments · Fixed by #42

Comments

@digama0
Copy link
Contributor

digama0 commented Feb 11, 2021

In both cases, the highlight begins at letter e and ends at letter l. In the one-line case, it is correct:

error: oops
 --> <current file>:1:6
  |
1 | abcd efgh ijkl mnop
  |      ^^^^^^^^^ oops
  |

But in multiple lines it is off by one - the end pointer points to one past the end rather than the last character of the highlight:

error: oops
 --> <current file>:1:6
  |
1 |   abcd efgh
  |  ______^
2 | | ijkl mnop
  | |_____^ oops
  |

I could propose code but I'm not sure I'll get all the details of continuations right since I haven't used those and am not certain of the desired behavior. Here's the issue presented as a test case.

use annotate_snippets::{display_list::DisplayList, snippet};
use snippet::Snippet;

#[test]
fn with_space_works() {
    let snippets = Snippet {
        title: Some(snippet::Annotation {
            id: None,
            label: Some("oops"),
            annotation_type: snippet::AnnotationType::Error,
        }),
        footer: vec![],
        slices: vec![snippet::Slice {
            source: "abcd efgh ijkl mnop",
            line_start: 1,
            origin: Some("<current file>"),
            annotations: vec![snippet::SourceAnnotation {
                range: (5, 14),
                label: "oops",
                annotation_type: snippet::AnnotationType::Error,
            }],
            fold: true,
        }],
        opt: Default::default(),
    };
    let expected = r#"error: oops
 --> <current file>:1:6
  |
1 | abcd efgh ijkl mnop
  |      ^^^^^^^^^ oops
  |"#;
    assert_eq!(DisplayList::from(snippets).to_string(), expected);
}

#[test]
fn with_newline_fails() {
    let snippets = Snippet {
        title: Some(snippet::Annotation {
            id: None,
            label: Some("oops"),
            annotation_type: snippet::AnnotationType::Error,
        }),
        footer: vec![],
        slices: vec![snippet::Slice {
            source: "abcd efgh\nijkl mnop",
            line_start: 1,
            origin: Some("<current file>"),
            annotations: vec![snippet::SourceAnnotation {
                range: (5, 14),
                label: "oops",
                annotation_type: snippet::AnnotationType::Error,
            }],
            fold: true,
        }],
        opt: Default::default(),
    };
    let expected = r#"error: oops
 --> <current file>:1:6
  |
1 |   abcd efgh
  |  ______^
2 | | ijkl mnop
  | |____^ oops
  |"#;

    assert_eq!(DisplayList::from(snippets).to_string(), expected);
}
@digama0
Copy link
Contributor Author

digama0 commented Feb 11, 2021

A possible fix is to change

let range = (
(end - line_start) - margin_left,
(end - line_start + 1) - margin_left,
);

to

                        let end_mark = (end - line_start).saturating_sub(1);
                        let range = (
                            end_mark - margin_left,
                            (end_mark + 1) - margin_left,
                        );

The question here is what to do about annotations that end on a newline, like "efgh\n", since there is no final character to point to. The suggestion here highlights it as

error: oops
 --> <current file>:1:6
  |
1 |   abcd efgh
  |  ______^
2 | | ijkl mnop
  | |_^ oops
  |

which is the same as the current behavior, but if the annotation is extended to "efgh\ni" then you get the exact same result, and thereafter it will point at the last character in the highlight, so that the original example with `"efgh\nijkl" is highlighted as

error: oops
 --> <current file>:1:6
  |
1 |   abcd efgh
  |  ______^
2 | | ijkl mnop
  | |____^ oops
  |

I don't know how to test what rustc does if the error highlight ends on a newline; it generally tries to avoid this and it also doesn't come up in my application so it doesn't seem very important.

@digama0
Copy link
Contributor Author

digama0 commented Feb 11, 2021

Ah, I found the relevant code in rustc.

            start_col: self.end_col.saturating_sub(1),
            end_col: self.end_col,

Using end_col.saturating_sub(1) for the start and end_col for the end leads to the somewhat odd situation that the marker disappears completely if the annotation ends on a newline, like this:

error: oops
 --> <current file>:1:6
  |
1 |   abcd efgh
  |  ______^
2 | | ijkl mnop
  | |_ oops
  |

But this is also an option if that seems reasonable to folks.

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

Successfully merging a pull request may close this issue.

1 participant