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

Do not highlight whitespace #118

Closed
petervdonovan opened this issue May 24, 2023 · 2 comments · Fixed by #8
Closed

Do not highlight whitespace #118

petervdonovan opened this issue May 24, 2023 · 2 comments · Fixed by #8

Comments

@petervdonovan
Copy link
Contributor

petervdonovan commented May 24, 2023

Recently I wrote:

I think that most of the changes in the "known good" files come from tokenizing whitespace, which seems entirely harmless.

This is untrue. We should not tokenize whitespace, as was done in #111.

This is because although whitespace is not colored in VS code, if you use a theme such as Abyss or Monokai which uses underlines, the whitespace will be underlined, which is not good.

We can either match a fixed amount of whitespace (which is brittle; we should probably avoid that), use programmatic highlighting instead of declarative highlighting (see highlight.ts), or do some trick with conditionally nested groups of patterns. This third option is what we do in order to apply the correct target language highlighter according to the target declaration, and I think I prefer it.

@lhstrh
Copy link
Member

lhstrh commented May 31, 2023

We should probably correct this before doing a new release. Can I help with this? I'm not sure that I understand this option 3 that you mention.

@petervdonovan
Copy link
Contributor Author

petervdonovan commented May 31, 2023

Probably the most time-efficient thing would be for me to just try option 3 and see if it works at all -- I'm not sure I understand option 3, either (which makes it hard for me to explain it). I can aim to do it by Saturday.

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.

2 participants