Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

goLanguageServer: set completion follow up command from middleware #3084

Merged
merged 2 commits into from
Mar 20, 2020

Conversation

hyangah
Copy link
Contributor

@hyangah hyangah commented Mar 3, 2020

Middleware checks both editor.parameterHints.enabled and
[go].editor.parameterHints.enabled. The language specific
config takes precedence as long as it is defined.

vscode.workspace.getConfiguration behaves differently when
querying the language-specific configuration section ('[go]')
so I couldn't query [go].editor.parameterHints directly using
getConfiguration.

Tested manually. (sorry)

Fixes #3075

Middleware checks both `editor.parameterHints.enabled` and
`[go].editor.parameterHints.enabled`. The language specific
config takes precedence as long as it is defined.

vscode.workspace.getConfiguration behaves differently when
querying the language-specific configuration section ('[go]')
so I couldn't query [go].editor.parameterHints directly using
getConfiguration.

Tested manually. (sorry)

Fixes microsoft#3075
@hyangah
Copy link
Contributor Author

hyangah commented Mar 3, 2020

@stamblerre @ramya-rao-a

src/goLanguageServer.ts Outdated Show resolved Hide resolved
src/goLanguageServer.ts Outdated Show resolved Hide resolved
src/goLanguageServer.ts Outdated Show resolved Hide resolved
const ret = next(document, position, context, token);

const isThenable = <T>(obj: vscode.ProviderResult<T>): obj is Thenable<T> => obj && (<any>obj)['then'];
if (isThenable<vscode.CompletionItem[] | vscode.CompletionList | null | undefined>(ret)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we expecting ret to be a thenable resolving to null or undefined?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vscode.ProviderResult (the return type of next) allows null or undefined.

@ramya-rao-a
Copy link
Contributor

If I understood the upstream conversations, it looks like gopls already calls editor.action.triggerParameterHints command. Won't we end up calling this command twice with the changes in this PR?

Are we planning to remove the call to the command from gopls?

@stamblerre
Copy link
Contributor

stamblerre commented Mar 19, 2020

Are we planning to remove the call to the command from gopls?

Yes. This will handle the fact that we can't respect user's VS Code configurations in gopls.

@ramya-rao-a
Copy link
Contributor

My concern is for users that have this change in the extension, but uses the gopls that is also running the editor.action.triggerParameterHints command. So, wouldn't this command get triggered twice?

@hyangah
Copy link
Contributor Author

hyangah commented Mar 19, 2020

My concern is for users that have this change in the extension, but uses the gopls that is also running the editor.action.triggerParameterHints command. So, wouldn't this command get triggered twice?

There is only one command in each completion item and this PR is overriding it.
So, basically, it's undoing what gopls did in line 183.

@ramya-rao-a ramya-rao-a merged commit d53b1b3 into microsoft:master Mar 20, 2020
hyangah added a commit to hyangah/vscode-go-old that referenced this pull request Mar 20, 2020
* master:
  goLanguageServer: set completion follow up command from middleware (microsoft#3084)
  Add stacktrace dump and better error messages on EXC_BAD_ACCESS panics (microsoft#2904)
  Address mismatch on path separators in debug config (microsoft#2010) (microsoft#3108)
  Include the link to release note/package overview in the update prompt, and update gopls default version (microsoft#3041)
  bug_report.md: Fix "architecture" typo. (microsoft#3095)
  telemetry.ts: send telemetry only if aiKey is not an empty  string(microsoft#3091)
hyangah added a commit to hyangah/vscode-go-old that referenced this pull request Mar 20, 2020
…sync

microsoft/vscode-go@d53b1b3

* 'master' of https://github.com/microsoft/vscode-go:
  goLanguageServer: set completion follow up command from middleware (microsoft#3084)
  Add stacktrace dump and better error messages on EXC_BAD_ACCESS panics (microsoft#2904)
  Address mismatch on path separators in debug config (microsoft#2010) (microsoft#3108)
gopherbot referenced this pull request in golang/vscode-go Mar 20, 2020
Merge branch 'master' of https://github.com/microsoft/vscode-go@d53b1b3

* 'master' of https://github.com/microsoft/vscode-go:
  goLanguageServer: set completion follow up command from middleware (microsoft#3084)
  Add stacktrace dump and better error messages on EXC_BAD_ACCESS panics (microsoft#2904)
  Address mismatch on path separators in debug config (microsoft#2010) (microsoft#3108)

Created by

`git pull --no-ff --log upstream master`

Change-Id: Id38768f3ec1bd01fa81325978f51f314fc1c08cb
GitHub-Last-Rev: 3a8de3f
GitHub-Pull-Request: #17
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/224240
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to disable parameter hints post completion when using gopls
3 participants