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

Add commitCharacters to analysis server's CompletionSuggestion #32240

Closed
DanTup opened this issue Feb 20, 2018 · 5 comments
Closed

Add commitCharacters to analysis server's CompletionSuggestion #32240

DanTup opened this issue Feb 20, 2018 · 5 comments
Labels
analyzer-completion Issues with the analysis server's code completion feature analyzer-server area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@DanTup
Copy link
Collaborator

DanTup commented Feb 20, 2018

I'm currently trying to add support to Code to have better commit characters on completion items to avoid users having to hit tab/enter to accept completion items.

For example:

class Danny {
  Danny(int i) {}
  method() {}
  int field;
  int get getset => 1;
  set getset(int a) {}
}

enum AAA { BBB, CCC };

main() {
  var a = new Danny();
  a.fiel^1
  a.met^2
  print(AA^3
  print(AAA.BB^3
}

Some examples:

  • At ^1 pressing . or space should complete (since they're both reasonable things for the user to type on the end of field).
  • At ^2 pressing ( should complete.
  • At ^3 pressing . should complete.
  • At ^4 pressing , or ) should complete.

while I can code this into my extension (and currently I'm trying - something like the below) it seems like the server would be the best place for this knowledge.

switch (kind) {
	case "CLASS":
	case "CLASS_TYPE_ALIAS":
	case "ENUM":
	case "ENUM_CONSTANT":
	case "GETTER":
	case "PREFIX":
		return ["."];

	case "FIELD":
	case "SETTER":
		return [" ", "."];

	case "CONSTRUCTOR":
	case "FUNCTION":
	case "FUNCTION_TYPE_ALIAS":
	case "METHOD":
		return ["("];

	case "LOCAL_VARIABLE":
	case "TOP_LEVEL_VARIABLE":
		return ["(", "."];

	case "PARAMETER":
		return [","];

	case "TYPE_PARAMETER":
		return [">"];
}
@DanTup DanTup changed the title Add commitCharacter to analysis server's CompletionItem Add commitCharacter to analysis server's CompletionSuggestion Feb 20, 2018
@DanTup DanTup changed the title Add commitCharacter to analysis server's CompletionSuggestion Add commitCharacters to analysis server's CompletionSuggestion Feb 20, 2018
@kevmoo kevmoo added analyzer-server area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. labels Feb 20, 2018
@bwilkerson
Copy link
Member

I'm not sure I'd like that. It means that if there is a matching completion then the user can't type those characters without selecting it, even if that's not what they intended to do. Also, deviating from the IDE's normal behavior can sometimes be disconcerting.

@bwilkerson bwilkerson added P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug labels Feb 20, 2018
@DanTup
Copy link
Collaborator Author

DanTup commented Feb 20, 2018

This is how the IDEs I've used and other languages in Code work. Usually there is a setting to disable it (in VS they call it something like "low impact intellisense mode" and in VS Code you disable editor.acceptSuggestionOnCommitCharacter) but it works really well (it's much more likely the user wants to complete a valid thing than type something invalid).

I've been getting annoyed by this but didn't really twig what it was until recently; imagine this:

dot complete

I always hit . expecting it to complete pathContext and insert a dot, but it doesn't, it just writes a dot after path which is almost never what I want (if there was a path in the completion list, then ofc it would select that). Then I had to bakespace, re-invoke completion, then hit tab/enter/whatever.

In VS Code there's specifically an API to allow for commit characters per completion item, as different characters make sense for different types of items.

I don't mind handling this in Dart Code; though I might ask for you to review the mapping I make it so, to ensure it seems reasonable (you know the syntax and kinds much better than me!).

@bwilkerson
Copy link
Member

Well, if it's a supported feature of VS Code, then it certainly won't bother users. And I agree that it would be better to let server tell you. That way the plugin doesn't need to be updated every time the language changes.

@DanTup
Copy link
Collaborator Author

DanTup commented Aug 24, 2018

@bwilkerson I had a go at implementing this in the plugin for now to see this working, however it triggers way too often and is somewhat unusable right now. For ex.:

void myFunc(Object b, Object c) {}

myFunc(myFunc, myFunc);

When typing a comma in the call (line 3), we want , to be a completion character (so if I'd just typed myFun and hit ,, it should complete myFunc). However in the method declaration (line 1) pressing , after typing b should not try to commit some random identifier from the completion list that starts with b.

So, whether or not to automatically commit is based on the context/location we're typing and not only on the type of the completion item.

In TypeScript, the request for completion includes some information like isNewIdentifierLocation (against the request body, not each completion item). If this is set to true, then automatic committing is disabled:

export interface CompletionInfo {
    readonly isGlobalCompletion: boolean;
    readonly isMemberCompletion: boolean;
    readonly isNewIdentifierLocation: boolean;
    readonly entries: ReadonlyArray<CompletionEntry>;
}
enableCommitCharacters = !body.isNewIdentifierLocation;

Could we do something similar here? Any opinions on what form it should take?

@pq pq added the analyzer-completion Issues with the analysis server's code completion feature label Jan 22, 2020
@DanTup
Copy link
Collaborator Author

DanTup commented Sep 5, 2022

This was for VS Code which is now using LSP. Commit characters are handled as part of LSP, although currently behind a flag (previewCommitCharacters) because VS Code's support is a bit restrictive and causes issues (microsoft/vscode#42544).

@DanTup DanTup closed this as completed Sep 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-completion Issues with the analysis server's code completion feature analyzer-server area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants