-
Notifications
You must be signed in to change notification settings - Fork 647
Conversation
This looks great, thank you for doing this! The only thing I noticed is that with gopls, you will want to disable vet on save as well as build on save, since gopls also returns vet errors. |
Thanks @stamblerre. Disabled vet on save for gopls as well @stamblerre, @ianthehat, I pushed a commit for prompting users to use the language server when using modules, as completion is much better there compared to gocode like @stamblerre mentioned before. My one concern here is the When using |
this looks awesome. I just tried and everything seems to work smooth. However auto completing imports (like |
Happy to see this coming along! Two issues I've noticed so far after a bit of usage:
|
I also experience the broken state. After a while, errors stay here while they have been fixed, or formatting overlapps basically screwing the entire file by combining two or more outputs. When that happens I have to reload vscode to be back in business |
Completing package names and documentation on hover (without comments, for now) should definitely still work. If you encounter this again, do you mind filing an issue with a repro case on the Go issue tracker? The stuck diagnostics seem to be an instance of https://golang.org/issue/30786, so we can move the discussion there. |
I'd like to clarify that I was referring to comments, specifically. I do see function/struct signatures although they seem to use a condensed syntax (one line with Thanks for linking to the issue on the go issue tracker. |
src/util.ts
Outdated
export function getAlternateLanguageServer(goConfig: vscode.WorkspaceConfiguration): string { | ||
if (goConfig['useLanguageServer'] === true | ||
&& goConfig['alternateTools'] | ||
&& goConfig['alternateTools']['go-langserver'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to define this in the package.json file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we do, then we need to define all the tools used in the extension as well.
src/goInstallTools.ts
Outdated
@@ -496,9 +513,10 @@ export function checkLanguageServer(): boolean { | |||
return false; | |||
} | |||
|
|||
let langServerAvailable = getBinPath('go-langserver') !== 'go-langserver'; | |||
const languageServerOfChoice = getAlternateLanguageServer(latestGoConfig) || 'go-langserver'; | |||
const langServerAvailable = path.isAbsolute(getBinPath('go-langserver')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't we check here for languageServerOfChoice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getBinPath
will internally look up alternateTools and do the right thing
@primalmotion, @mafredri, All, @vladbarosan, |
@ramya-rao-a updated to latest pr code but I still have the issue where packages are not autocompleted. |
I also still have the errors that I fixed stuck forever until I reload vscode |
@stamblerre By default, I will be disabling the diagnostics feature of the language server. Users can opt into it by using @primalmotion With the latest changes in the PR, you wont get any errors from the language server and the completion of unimported packages should also work as long as you have set |
Cool. I'll give a go tomorrow. So to get the build error (cause it's cool) should I reactivate the build on save? |
Yes |
const latestGoConfig = vscode.workspace.getConfiguration('go'); | ||
if (!latestGoConfig['useLanguageServer']) return false; | ||
if (!latestGoConfig['useLanguageServer']) return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if there is a way to get a typed response here, is very annoying that we need to do type checks at runtime :( . ill open an issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly looks good, left some suggestions
Thanks @vladbarosan, I have addressed most of the comments |
\o/ |
Thank you @ramya-rao-a, @vladbarosan, and everyone else who contributed to this! We really appreciate your work on getting gopls in VSCode 😄 Looking forward to our continued collaboration and improvements to VSCode-Go! |
well.. now goals crashes every few seconds..
|
@primalmotion: Are you using gopls at the latest commit? I'm not able to reproduce, but if you are at tip, please file an issue here. |
I was on the tip. Then I upgraded go tools with vscode and it started to fail. |
Now that the language server from Google is ready for use, there are some changes needed in the Go extension to ensure good user experience
Go: Install/Update Tools
Edit as per latest commits:
gopls
gopls
or stop disable thego.useLanguageServer
settinggo.useLanguageServer
for the first time,gopls
will be installed and useddiagnostics
is added togo.languageServerExperimentalFeatures
setting so that user can control if they want the language server to return build/vet errors. Set totrue
by default. When enabled, the build and vet on save features of the extension will not be run to avoid duplicate errorscc @stamblerre, @ianthehat