Closed rchl closed 2 years ago
Also:
$
- in query($arg: String!)
, for exampleThis logic is built into getAutocompleteSelections! There is in fact autocomplete for these characters in language interface, which is used by both the language server and graphiql
I don't quite understand what are you referencing here. :)
Is this is some sort of developer note on how this could be fixed?
Yes I will add notes. We need to add additionalTextItems or whatever they are called. I am planning a whole writeup on how to implement this for both language server and monaco and i think i have an issue already. The vscode-json-languageservice provides a great reference implementation
additionalTextEdits
is a property of the completion and can be used to perform additional edits on the document when a completion is committed. I'm not sure if that is strictly related to my request. The triggerCharacters
is what controls when the completions are shown in the first place.
Do note that this needs to be done carefully to avoid triggering completions too eagerly. For example it might not be the best idea to trigger on typing {
to open a block since the user might want to add a line break before triggering completions.
Unfortunately the LSP spec is not very flexible in that area since the triggers are only character-based. So it might not be that easy or even possible to define good triggers for completing fields, for example.
additionalTextEdits is the answer I assure you. Please review the completion support in vscode-json-languageservice to see how they use this feature
I have already gotten it working partially in experiments. Trigger characters setting does not do what you think. Additional text items is how we evaluate this on a per token basis
For example, in monaco graphql this weekend i had an experiment where any time a user selected an object or list stype field from completion, it would automatically expand braces and insert the cursor in between the braces. That’s what you’re talking about in vscode right? see how the vscode-json-languageservice passes on these additional items in it’s completion to monaco-json and vscode differently.
I'm a LSP client developer for ST so I "more or less" know how those things work.
additionalTextEdits
can do various edits to the document when completion is committed but my initial issue is about showing the completions in the first place since they are not shown.
When mentioning triggerCharacters
I meant those in context of the completionProvider.triggerCharacters
server capability and not in context of individual completion.
@rchl I think I understand better now what you mean. I thought you were talking about a different feature.
What do you suggest we use for trigger characters?
@rchl I would be happy to work with this on you!
our language interface getAutocompleteSuggestions
has it's own built-in behavior that may or may not clash with LSP triggerCharacters
. This language server pre-dates LSP, so there are a few things that still clash with LSP behavior
triggerCharacters
are quite limited so those can't solve this 100%.
For example user types:
query {
album {
|
}
}
where the cursor is at |
. Now, ideally the completions would be triggered at that point already but there is no specific character that was triggered to get here (most likely Enter was pressed after adding {}
). So triggerCharacters won't help this case.
I think it would make sense to have at least $
and (
as a trigger characters so it would trigger completions in these cases:
query() {
foo(|
}
query($foo: String) {
foo(foo: $|
}
But it should be tested in practice because it can get in a way if done for wrong characters.
Here are some trigger characters set by other servers for reference: TS: ".", """, "'", "/", "@", "<" pylsp: "." pyright: ".", "[" vetur: ".", ":", "<", """, "'", "/", "@", "*", " "
Of course which trigger characters fit depends on the syntax so I'm not suggesting to just copy any of those.
our language interface
getAutocompleteSuggestions
has it's own built-in behavior that may or may not clash with LSPtriggerCharacters
. This language server pre-dates LSP, so there are a few things that still clash with LSP behavior
I don't know how getAutocompleteSuggestions
works but again, wouldn't that only apply once the completions were already triggered? I'm talking about triggering the completions popup in the first place.
@rchl I was wrong that getAutocompleteSuggestions
would conflict, and also you were very very correct about the lack of triggerCharacters.
another powerful one to add to your list is \n
! this is fantastic, unlocks some requested functionality, thank you! I actually just tried this in monaco-graphql
and it was incredible: [' ', ':', '{', '\n', '$']
as per (
, that's great for field arguments, but that won't do anything for variables declaration, i.e. myQuery($variable: MyInputType, ...
however i think it would just trigger empty completion so it's a non-issue
also, as per the unrelated discussion about insertText
, I have a local branch I'm about to clean up and provide a PR for, that in combination with this triggerCharacters
effort is going to make monaco-graphql
and vscode-graphql
https://github.com/graphql/graphiql/issues/2069
@rchl what do you think of the current trigger characters now? I've improved them for the server and monaco in #2070
[' ', ':', '$', '\n', ' ', '(', '@']
. The space is repeated twice. Not a big deal though.(
trigger doesn't really work (in ST at least) because the triggers don't apply after the editor inserts a snippet and inserting (
runs a snippet that inserts matching )
automatically. Not much you can do about it. I know that some servers trigger a custom command in that case to force client to trigger completions. I can provide more info if you are interested.
EDIT: Actually I see that you are already doing that in some cases when selecting completions.\n
doesn't work either. Could be again related to snippets but with content like:
query {|}
after pressing enter:
query {
|
}
completions don't trigger.
The "space" trigger is in some cases good an in some cases annoying. Good in:
foo(arg: String, |)
because it will trigger completions that help with the next argument.
Bad in:
query {
foo |
}
because space after foo
is likely to preceed an opening brace {
so completions just get in the way here.
:
trigger can also get in a way a bit because I usually want to add space after :
. Though I can still press space when completions are visible so no big deal.\n
has been removed fyi!
I think most users seem happy with the current trigger characters. If you think of any others that aren't represented or are having issues with ones that are, please open a new ticket!
Currently the only registered trigger character for the language server is
@
: https://github.com/graphql/graphiql/blob/65aa64811ccd40c5e8ba99edec825a91115a3c8b/packages/graphql-language-service-server/src/MessageProcessor.ts#L168I think there are many cases when automatically triggering completions would be beneficial. For example when typing
{
to select fields of an object or when typing(
after field name to specify the arguments to pass). None of those currently trigger auto-completion which means that the user has to trigger it manually with a shortcut.