Poor trigger characters for the language server

[rchl]

Currently the only registered trigger character for the language server is @:

graphiql/packages/graphql-language-service-server/src/MessageProcessor.ts

Line 168 in 65aa648

triggerCharacters: ['@'],

I 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.

[rchl]

Also:

• $ - in query($arg: String!), for example

[acao]

This 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

[rchl]

I don't quite understand what are you referencing here. :)

Is this is some sort of developer note on how this could be fixed?

[acao]

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

[rchl]

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.

[acao]

additionalTextEdits is the answer I assure you. Please review the completion support in vscode-json-languageservice to see how they use this feature

[acao]

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

[acao]

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.

[rchl]

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.

[acao]

https://github.com/microsoft/vscode-json-languageservice/blob/386122c7f0b6dfab488b3cadaf135188bf367e0f/src/services/jsonCompletion.ts#L881

[acao]

@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?

[acao]

@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

[rchl]

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.

[rchl]

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

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.

[acao]

@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
#2069

[acao]

@rchl what do you think of the current trigger characters now? I've improved them for the server and monaco in #2070

[rchl]

1. I can see that triggers are now: [' ', ':', '$', '\n', ' ', '(', '@']. The space is repeated twice. Not a big deal though.

2. The ( 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.

3. The \n doesn't work either. Could be again related to snippets but with content like:

   query {|}

   after pressing enter:

   query {
       |
   }

   completions don't trigger.

4. 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.

5. The : 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.

[acao]

\n has been removed fyi!

[acao]

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!