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

Remove support for github.com/sourcegraph/go-langserver #3127

Merged
merged 2 commits into from
Mar 28, 2020
Merged

Remove support for github.com/sourcegraph/go-langserver #3127

merged 2 commits into from
Mar 28, 2020

Conversation

stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Mar 25, 2020

PR in Go Nightly: golang/vscode-go#2

I improved the error messages for the go-langserver alternate tools setting, but I figured that it might be too much effort to add buttons to make automatic changes for users. I would imagine that most people have already made the switch, and if not, it's quite a small amount of manual work to justify the extra code for different buttons. Once a user changes the setting and reloads their editor, they will be prompted to install gopls anyway.

@ramya-rao-a: If you disagree, I'd be happy to add the buttons anyway. If I do that, could you point me to an API reference for how the buttons would work? I got as far as adding items to vscode.window.showErrorMessage, but I couldn't figure out how to connect them to functions when they are clicked or how to programmatically modify a user's settings.

Fixes #3028

/cc @hyangah

@ramya-rao-a
Copy link
Contributor

On second thoughts, I am wondering if we should skip the trouble and simplify as below

// Get the path to gopls or any alternative that the user might have set for gopls
	const goplsBinaryPath = getBinPath('gopls');
	if (path.isAbsolute(goplsBinaryPath)) {		
		return goplsBinaryPath;		
	}
	
	// Otherwise, prompt the user to install the language server.
	promptForMissingTool('gopls');

None of the other tools get to show any error when the alternate tool that is defined is not found.
So, am fine removing similar check for the language server

Uers who have been relying on the alternate tool for go-langserver, will now see the prompt for installing gopls which is what we want anyway.
And I believe the number of users falling under this category is going to be a very small

@stamblerre
Copy link
Contributor Author

I agree that it's a small number of users, but I still would prefer to show high-quality error messages for topics relating to the language server. There's been a lot of confusion around it, and people might have old settings configured that they forgot about. I'm happy to delete this code for next release or something like that, but I prefer to err on the side of more error messages.

@ramya-rao-a
Copy link
Contributor

In that case I propose the below:

// Get the path to gopls
const goplsBinaryPath = getBinPath('gopls');
if (path.isAbsolute(goplsBinaryPath)) {
	return goplsBinaryPath;
}

if (goConfig['alternateTools']) {
	// Show error message for alternate tool configured for gopls that cannot be found
	const goplsAlternate = goConfig['alternateTools']['gopls'];
	if (goplsAlternate) {
		vscode.window.showErrorMessage(
			`Cannot find the alternate tool ${goplsAlternate} configured for gopls. 
			Please install it and reload this VS Code window.`
		);
		return;
	}

	// Show error message for alternate tool configured for go-langserver which is no longer respected
	const goLangserverAlternate = goConfig['alternateTools']['go-langserver'];
	if (goLangserverAlternate) {
		vscode.window.showErrorMessage(`Support for "go-langserver" has been deprecated in favor of gopls.
The alternate tool configuration for "go-langserver" can be deleted.`);
	}
}

// Prompt the user to install gopls.
promptForMissingTool('gopls');

@stamblerre stamblerre closed this Mar 27, 2020
Change-Id: Ie2257642b86f1939cc4ead5be69c69fa4ab25ab4
@stamblerre stamblerre reopened this Mar 27, 2020
@stamblerre
Copy link
Contributor Author

Done. Sorry about the confusion with the PR getting closed - managing 3 remotes has proven to be a bit confusing, and I'm not the best at git :)

@ramya-rao-a
Copy link
Contributor

Thanks for your patience @stamblerre :)
I have pushed a commit for a tiny change to replace "this setting" with "alternate tool setting" as the user won't be aware of what "this" refers to

@ramya-rao-a ramya-rao-a merged commit b83ad81 into microsoft:master Mar 28, 2020
@stamblerre stamblerre deleted the go-langserver branch April 28, 2020 05:48
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.

settings: create one setting to specify an alternate language server
2 participants