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

ui: enable semantic tokens by default #2286

Open
hyangah opened this issue Jun 10, 2022 · 4 comments
Open

ui: enable semantic tokens by default #2286

hyangah opened this issue Jun 10, 2022 · 4 comments

Comments

@hyangah
Copy link
Contributor

hyangah commented Jun 10, 2022

We can set this default before gopls switches its default (golang/go#45313)
Semantic tokens fixes many issues TextMate-based syntax highlighting has (incorrect highlighting, lack of generics support, etc)

Potential concerns:

Setting:

Currently, we ask users to use

"gopls": {
   "ui.semanticTokens": true
}

But how about promoting this as a go setting?

"go.ui.semanticTokens": true

@golang/tools-team

@gopherbot gopherbot added this to the Untriaged milestone Jun 10, 2022
@suzmue suzmue modified the milestones: Untriaged, vscode-go/later Jun 13, 2022
@suzmue
Copy link
Contributor

suzmue commented Jul 25, 2022

There are a number of significant changes to the coloring of Go code when semantic tokens are turned on. We will need to make sure to have clear communication as we transition over to semantic tokens.

A possible path forward would be to:

  1. Fix or decide how to handle outstanding blocking bugs
  1. Create clear, easy to find documentation about semantic tokens
  2. Recommend users opt-in to semantic tokens with a pop-up
  • should we have a go.* setting for convenience and translate it to gopls setting?
  1. Enable semantic tokens by default for >=go1.18 (generics are not supported by the textmate coloring)
  2. Enable semantic tokens by default for all users and provide a clear way for users to opt out.

@hyangah
Copy link
Contributor Author

hyangah commented Jan 4, 2023

Closing since "gopls" "ui.semanticTokens" setting will go away.
See #2598

@hyangah hyangah closed this as not planned Won't fix, can't repro, duplicate, stale Jan 4, 2023
@hyangah hyangah reopened this Feb 26, 2023
@hyangah
Copy link
Contributor Author

hyangah commented Feb 26, 2023

We learned the trick described in #2598 doesn't work. Reopening.

@findleyr
Copy link
Member

findleyr commented Feb 26, 2023

I have been thinking about this recently.

I think we should have two modes for semantic tokens.

  • "syntax": syntax only, no type-checking; maybe even just tokenization
  • "types": the current implementation

(very open to better names: fast/rich?)

The problems with the current implementation, that have always made me hesitant to advocate for it by default, are as follows:

  1. type-checking can cause noticeable latency in token calculation, leading to flickering
  2. type-checking can be inaccurate in files with parse errors
  3. type-checking doesn't work if we don't have a package for the file, such as if it is guarded by build tags
  4. as semantic tokens are managed client-side, there is no way to force them to update if a change in a dependent package invalidates tokens
  5. there are too many token types, resulting in (IMO) many distracting colors. This is definitely a matter of taste however, and I don't fault anyone who likes the richer highlighting

Semantic tokens based on go tokenization / parsing doesn't suffer from any of these problems, though it is arguably not as useful. However, I think it could provide ~80% of the benefit, would be much more reliable, and frankly is the mode I would enable.

Therefore, I think we should at least implement a syntax mode (it should be very easy to do), and I would argue that it should be the default. Furthermore, we can use this fast mode for files that don't have packages, even if the "types" highlighting is selected.

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

4 participants