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

Unable to get quick fixes in problems view to work #54803

Closed
Gama11 opened this issue Jul 21, 2018 · 9 comments · Fixed by #55090
Closed

Unable to get quick fixes in problems view to work #54803

Gama11 opened this issue Jul 21, 2018 · 9 comments · Fixed by #55090
Assignees
Labels
api feature-request Request for new features or functionality verified Verification succeeded
Milestone

Comments

@Gama11
Copy link
Contributor

Gama11 commented Jul 21, 2018

Issue Type: Bug

VS Code version: Code - Insiders 1.26.0-insider (26e5a55, 2018-07-20T05:18:39.569Z)
OS version: Windows_NT x64 10.0.17134

After trying to support #52627 in the Haxe language server didn't work as expected, I tried with a simple VSCode extension, but I didn't have much luck here either. I'm setting the diagnostics property of the CodeAction as required according to #52627 (comment) - is there anything else I'm missing?

import * as vscode from "vscode";

export function activate(context: vscode.ExtensionContext) {
	var diagnostics = vscode.languages.createDiagnosticCollection("foo");
	vscode.window.onDidChangeActiveTextEditor(editor => {
		if (editor != null) {
			var diagnostic = new vscode.Diagnostic(new vscode.Range(0, 0, 1, 0), "Message");
			diagnostics.set(editor.document.uri, [diagnostic]);
		}
	});
	vscode.languages.registerCodeActionsProvider('plaintext', {
		provideCodeActions(document) {
			var action = new vscode.CodeAction("Fix");
			action.diagnostics = diagnostics.get(document.uri);
			console.log(action);
			return [action];
		}
	});
}

@jrieken
Copy link
Member

jrieken commented Jul 24, 2018

This is fishy diagnostics.get(document.uri) because Uri-objects aren't comparable by object id because new instances are created all the time. Use document.uri.toString() for something that works as key.

@jrieken jrieken added the *dev-question VS Code Extension Development Question label Jul 24, 2018
@vscodebot
Copy link

vscodebot bot commented Jul 24, 2018

We have a great developer community over on slack where extension authors help each other. This is a great place for you to ask questions and find support.

Happy Coding!

@vscodebot vscodebot bot closed this as completed Jul 24, 2018
@Gama11
Copy link
Contributor Author

Gama11 commented Jul 24, 2018

Hm.. get() expects a Uri instance, so a toString() there would result in a compiler error. Why is toString() important, because of the normalization it does?

Anyway, it's actually not the diagnostics.get() call that is failing. If you dig down into the console.log() in the next line, you can see that the returned CodeAction does have the correct diagnostic attached to it:


I also thought that the example may be too isolated, and that an edit may be required for it to work. However, even with:

action.edit = new WorkspaceEdit();
action.edit.replace(document.uri, new vscode.Range(0, 0, 1, 0), "Foo");

...the quick fix still can't be triggered from the problems view:

@jrieken jrieken removed the *dev-question VS Code Extension Development Question label Jul 24, 2018
@jrieken jrieken reopened this Jul 24, 2018
@jrieken
Copy link
Member

jrieken commented Jul 24, 2018

Sorry, I missed that this is our DiagnosticCollection and not a map you have created (internally we call toString then).

@jrieken
Copy link
Member

jrieken commented Jul 24, 2018

Ok, I know now. The context menu filters for code actions of the quickFix-kind. So adding action.kind = vscode.CodeActionKind.QuickFix; to your sample will make them show.

@jrieken
Copy link
Member

jrieken commented Jul 24, 2018

@mjbvz I think we should print a message when we drop code actions on the client because of filtering/request constraints

@Gama11
Copy link
Contributor Author

Gama11 commented Jul 24, 2018

Oh, interesting. I had assumed that CodeActionKind.QuickFix would be the default, so I didn't bother setting it explicitly. Especially considering Empty doesn't exist in the LSP types and QuickFix is the first value in CodeActionKind there:

https://github.com/Microsoft/vscode-languageserver-node/blob/5f9c993ff38f5c369949aeb359b3e9b178172dbc/types/src/main.ts#L1738-L1746

So I guess the default kind is actually Empty...? Perhaps the CodeAction.kind docs could clarify that.

Gama11 added a commit to vshaxe/haxe-language-server that referenced this issue Jul 24, 2018
@jrieken jrieken assigned mjbvz and unassigned jrieken Jul 24, 2018
@mjbvz
Copy link
Collaborator

mjbvz commented Jul 24, 2018

@Gama11 Yes the default for quick fixes is empty. This is because the editor lightbulb isn't tied to quick fixes specifically but also will show up if there are refactorings and other action types available. For quick fixes in the problems view however, we do request that extensions only provide quick fixes and will filter out anything that isn't marked as such

@jrieken I can add some debug logging here but it will be noisy since many code action providers return commands or return CodeAction but fail to set the codeActionKind. Worth pushing extension developers to update their code? Maybe don't log when someone is returning commands?

@jrieken
Copy link
Member

jrieken commented Jul 25, 2018

Worth pushing extension developers to update their code? Maybe don't log when someone is returning commands?

Yeah, only when a code action is returned. We can also use the log service for this

@mjbvz mjbvz added this to the July 2018 milestone Jul 25, 2018
@mjbvz mjbvz added feature-request Request for new features or functionality api labels Jul 25, 2018
mjbvz added a commit to mjbvz/vscode that referenced this issue Jul 25, 2018
Fixes microsoft#54803

Adds a loggin warning when a code action provider returns code actions that will be dropped. Warn in the the following cases:

- A provider returns code actions (not commands)
- And a specific code action type is requested.
- And the returned code actions either don't set kind or are of the wrong kind
mjbvz added a commit that referenced this issue Jul 26, 2018
* Add extension logging when returned code action will be dropped

Fixes #54803

Adds a loggin warning when a code action provider returns code actions that will be dropped. Warn in the the following cases:

- A provider returns code actions (not commands)
- And a specific code action type is requested.
- And the returned code actions either don't set kind or are of the wrong kind

* Use log service

* Include extension id in warning
@mjbvz mjbvz added the verification-needed Verification of issue is requested label Jul 27, 2018
@JacksonKearl JacksonKearl added verified Verification succeeded and removed verification-needed Verification of issue is requested labels Jul 31, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Sep 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api feature-request Request for new features or functionality verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants