vscode-go icon indicating copy to clipboard operation
vscode-go copied to clipboard

ui: enable semantic tokens by default

Open hyangah opened this issue 2 years ago • 1 comments

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:

  • https://github.com/golang/vscode-go/issues/1387 @pjweinb can we close this issue now, since gopls is limiting the size of file?
  • files with non-matching build tags won't get any semantic tokens info. We think in that case, other syntax highlighting will handle, so it is still an improvement. We need to test/confirm.

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

hyangah avatar Jun 10 '22 13:06 hyangah

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
  • [ ] golang/go#45753 - string (see @pjweinb's proposal: https://github.com/golang/go/issues/45753#issuecomment-828663792)
  • [ ] golang/go#45792 - number
  • [ ] golang/go#54066 - struct tags as string
  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.

suzmue avatar Jul 25 '22 21:07 suzmue

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

hyangah avatar Jan 04 '23 22:01 hyangah

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

hyangah avatar Feb 26 '23 16:02 hyangah

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.

findleyr avatar Feb 26 '23 17:02 findleyr