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.

Sign up for GitHub

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

Jump to bottom

There should be no source color highlighting if the label points to the whole line, or if the label is multiline #261

Open
RDambrosio016 opened this issue Jul 10, 2020 · 8 comments · May be fixed by #263
Open

There should be no source color highlighting if the label points to the whole line, or if the label is multiline #261

RDambrosio016 opened this issue Jul 10, 2020 · 8 comments · May be fixed by #263

Comments

@RDambrosio016
Copy link
Contributor

Codespan currently highlights in red whatever the primary label is pointing to in the source, while this looks great for errors inside of source code lines, it looks really ugly if the label points to the whole line. It ruins the point of highlighting, you can clearly see what the label is pointing at, a line of all red just looks like it does not belong there. In the case of multiline labels, usually they are used to point to blocks of source code, but this means if you highlight everything inside of them you end up with a giant blob of RED which just looks like it does not belong and it ruins the cleanliness of the diagnostic.

Labels should highlight their source code only if the label points to a subsection of the source code line, and should be fully disabled for multiline labels.
@brendanzab
Copy link
Owner

Yeah, this was definitely one of my original concerns about adding source highlighting - see #206 (comment) - maybe special casing full lines and multiline might be ok?

cc. @jonathandturner @dtolnay

@Johann150
Copy link
Contributor

@RDambrosio016 What would you consider to be the "whole line"? All text (optionally without the leading whitespace). What about trailing whitespace?

@RDambrosio016
Copy link
Contributor Author

@Johann150 the whole line is the line of source code will All leading and trailing whitespace removed, as this isnt really viewable so there is no point in counting it in

@RDambrosio016
Copy link
Contributor Author

Note that this also means that no highlighting should be done if the label goes over the actual source and into trailing or leading whitespace

@Johann150 Johann150 linked a pull request Jul 12, 2020 that will close this issue
Open
@brendanzab
Copy link
Owner

brendanzab commented May 20, 2021 •
edited
Loading

One thing I'm pondering is whether it would be better to do this as a simpler configuration option. Like, personally I'm not the biggest fan of the source highlighting, but perhaps we could add in a boolean flag to enable it? I guess I'm just worried about adding complicated heuristics around this, and how good it will actually look in practice, or whether it will end up confusing people.

@Johann150
Copy link
Contributor

Do you mean that this option should turn off all highlighting and just rely on the underlines?

@brendanzab
Copy link
Owner

brendanzab commented May 21, 2021 •
edited
Loading

Yeah - thinking of how rustc normally renders errors. Maybe an option like highlight_source or something 🤔

@Johann150
Copy link
Contributor

I think that is not related to this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

no excessive highlighting Johann150/codespan
3 participants
@brendanzab @Johann150 @RDambrosio016