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

x/tools/gopls: reconsider and restructure setting parameter names #43101

Closed
hyangah opened this issue Dec 9, 2020 · 2 comments
Closed

x/tools/gopls: reconsider and restructure setting parameter names #43101

hyangah opened this issue Dec 9, 2020 · 2 comments

Comments

@hyangah
Copy link
Contributor

@hyangah hyangah commented Dec 9, 2020

There are many settings parameters that can be used to tune gopls behavior. The defaults were carefully chosen so most users don't need to configure them.
But when they do need customization, it should be easy to find the relevant settings.

Detailed documentation for individual setting helps, but I think it's better if

  • the settings are grouped based on their functions. (e.g. is this setting for workspace symbol search? Or completion?)
  • setting names are self-explanatory and sufficiently descriptive.
  • setting names follow some rules roughly so users can guess what to look for.

Some of currently available setting names are not very descriptive. (e.g. local, matcher).

FYI VSCode uses setting names (configuration key) to namespace and group the settings. These name structure helps the plugin developer to grep/watch a part of configuration more efficiently. VSCode uses the name-based grouping to order and build graphical setting page UI. https://code.visualstudio.com/api/references/contribution-points#Configuration-schema

Screen Shot 2020-12-09 at 12 16 47 PM

I originally attempted to map some of gopls settings to VSCode-friendly setting names (that I chose based on the settings' role). For example, I was thinking patterns like <feature>.<behavior> when possible. (note: some settings are not cleanly mapped to one feature)

hoverKind -> doc.hoverKind
linksInHover -> doc.linksInHover
usePlaceholders -> completion.usePlaceHolders
gofumpt -> format.useGofumpt
matcher -> completion.matcher
symbolMatcher -> workspaceSymbols.matcher

But I think it's more desirable if all gopls-based editors share the similar setting structures.

  • users can map settings from one editor to another editors more easily, so helps more knowledge sharing.
  • less documentation work to explain different editor settings for gopls maintainers.

I was also considering extending the api-json output to include the group info as a field, and utilize the group info to auto-construct VS Code setting names. But for the reasons I mentioned above, I think it's better if the names carry sufficient info.

@gopherbot gopherbot added this to the Unreleased milestone Dec 9, 2020
@stamblerre stamblerre added this to Needs Triage in vscode-go: gopls by default via automation Dec 15, 2020
@stamblerre stamblerre moved this from Needs Triage to Critical in vscode-go: gopls by default Dec 16, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Dec 25, 2020

Change https://golang.org/cl/280192 mentions this issue: internal/lsp: restructure user options (CL 278433 continued)

@stamblerre stamblerre self-assigned this Dec 28, 2020
gopherbot pushed a commit to golang/tools that referenced this issue Dec 30, 2020
This CL copies Heschi's structural changes to the options from CL 278433
and makes the necessary adjustments in the JSON and documentation
generation. Nested settings are grouped together and the "status" of a
given setting is also listed. Currently the only possible statuses are
"experimental" and "debug", but I will add "advanced" in a follow-up (to
indicate that a setting is only for advanced users).

The options "set" function still expects flattened settings to avoid
fundamentally changing people's current configurations, so VS Code Go
will just have to make sure to flatten the settings before sending them
to gopls (which should be easy enough).

No names of any settings are changed (Heschi's earlier CL adjusted the
experimental prefixes). As discussed offline, we've decided to prefix
any setting that we expect to delete with "experimental", and so we'll
leave existing setting names as they are.

Updates golang/go#43101

Change-Id: I55cf7ef09ce7b5b1f8af06fcadb4ba2a44ec9b17
Reviewed-on: https://go-review.googlesource.com/c/tools/+/280192
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
@gopherbot
Copy link

@gopherbot gopherbot commented Dec 30, 2020

Change https://golang.org/cl/280706 mentions this issue: [gopls-release-branch.0.6] internal/lsp: restructure user options (CL 278433 continued)

@stamblerre stamblerre closed this Dec 31, 2020
vscode-go: gopls by default automation moved this from Critical to Done Dec 31, 2020
gopherbot pushed a commit to golang/tools that referenced this issue Jan 11, 2021
This CL copies Heschi's structural changes to the options from CL 278433
and makes the necessary adjustments in the JSON and documentation
generation. Nested settings are grouped together and the "status" of a
given setting is also listed. Currently the only possible statuses are
"experimental" and "debug", but I will add "advanced" in a follow-up (to
indicate that a setting is only for advanced users).

The options "set" function still expects flattened settings to avoid
fundamentally changing people's current configurations, so VS Code Go
will just have to make sure to flatten the settings before sending them
to gopls (which should be easy enough).

No names of any settings are changed (Heschi's earlier CL adjusted the
experimental prefixes). As discussed offline, we've decided to prefix
any setting that we expect to delete with "experimental", and so we'll
leave existing setting names as they are.

Updates golang/go#43101

Change-Id: I55cf7ef09ce7b5b1f8af06fcadb4ba2a44ec9b17
Reviewed-on: https://go-review.googlesource.com/c/tools/+/280192
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants