golang / vscode-go

Go extension for Visual Studio Code
https://marketplace.visualstudio.com/items?itemName=golang.Go
Other
3.79k stars 729 forks source link

ui: enable semantic tokens by default #2286

Open hyangah opened 2 years ago

hyangah commented 2 years ago

We can set this default before gopls switches its default (https://github.com/golang/go/issues/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

suzmue commented 1 year ago

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

  2. Create clear, easy to find documentation about semantic tokens

  3. 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?
  4. Enable semantic tokens by default for >=go1.18 (generics are not supported by the textmate coloring)

  5. Enable semantic tokens by default for all users and provide a clear way for users to opt out.

hyangah commented 1 year ago

Closing since "gopls" "ui.semanticTokens" setting will go away. See https://github.com/golang/vscode-go/issues/2598

hyangah commented 1 year ago

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

findleyr commented 1 year ago

I have been thinking about this recently.

I think we should have two modes for semantic tokens.

(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.