Lsp: Keep using trigger characters, or start using selectors?

[predragnikolic]

For some reason the final commit_completion command re-triggers the completion panel "forever". I need to esc to hide it.

Settings are

    "auto_complete": true,
    "auto_complete_commit_on_tab": true,
    "auto_complete_with_fields": true,

_Originally posted by @deathaxe in https://github.com/sublimelsp/LSP/pull/866#issuecomment-603466761_

[deathaxe]

More related information about possible reasons can be found at https://github.com/sublimelsp/LSP/pull/866#issuecomment-605629210.

---

See the bug, too, using the LSP-json completions.

With regards to @jskinner's statements the triggers and selectors of the view are set to:

>>> view.settings().get("auto_complete_selector")
'- string - comment - constant | meta.main-key | meta.rule-key | meta.interpolation-key | meta.animation-key | meta.attributes-sequence - comment | meta.class-selector | meta.function-call.var meta.group'
>>> view.settings().get("auto_complete_character")
Unable to open /C/Apps/Sublime Text/Data/Packages/Default/Preferences.sublime-settings
>>> view.settings().get("auto_complete_triggers")
[{'characters': '<', 'selector': 'text.html'}, {'selector': 'punctuation.accessor'}, {'characters': '":', 'selector': 'source.json'}]

The string scope is explicitly excluded from completitions. The trigger ": is defined.

It looks like ST only uses the first character of the trigger, thus retriggering AC after committing a value which ends with ". Same happens after completing the "class" or just removing and retyping the final ". Both the json-key and json-value are string.quoted.double.

Not sure whether the trigger is intended to be of one character only by design or whether it is a bug.

[jskinner]

'characters': '":' means to trigger after any of " or :

If you want AC to trigger directly after a ": in JSON (before the user has even typed a space), then you'd want an auto_complete_trigger entry of {"selector": "source.json punctuation.separator.mapping.key-value.json"},. This is putting aside that most people would enter a space at this point, so you wouldn't typically want to trigger AC here.

FWIW Sublime Text doesn't use any setting called "auto_complete_character"

[jskinner]

In general, the characters portion of auto_complete_trigger should be ignored, and only the selector used where possible, most syntax definitions are fine grained enough to make this do exactly what you want.

That's why we have {"selector": "punctuation.accessor"} rather than {"characters": "."} in the defaults

[deathaxe]

FWIW Sublime Text doesn't use any setting called "auto_complete_character"

Should have filtered it out. Didn't remember the correct setting when trying that out :-)

If you want AC to trigger directly after a ": in JSON

Seems what the json language server expects as well with regards to its ouput (see below).

(before the user has even typed a space)

Find it weired as well.

In general, the characters portion of auto_complete_trigger should be ignored,

I guess the issue with it is language-servers to return the triggerCharacters only instead of selectors. Sticking to selectors (only) would literly make automatic configuring a syntax specific trigger by the server impossible.

It would then need to be specified in the LSP.sublime-settings or provided by the language-specific plugin, such as json-lsp instead of the server.

---

:: <<< lsp-json 1: {'capabilities': {'textDocumentSync': 2, 'hoverProvider': True, 'completionProvider': {'resolveProvider': True, 'triggerCharacters': ['"', ':']}, 'documentRangeFormattingProvider': True, 'foldingRangeProvider': True, 'documentSymbolProvider': True, 'selectionRangeProvider': True, 'colorProvider': {}}}

[jskinner]

Automatic configuration based on nothing but triggerCharacters from the LSP server is already impossible, if taken at face value it's entirely brain dead. Showing AC after every " or : character in JSON would give a terrible UX.

Just ignore it, it's a broken part of the LSP protocol.

If you don't want to ignore it, then use heuristics or settings to translate trigger characters into selectors.

[deathaxe]

Just ignore it, it's a broken part of the LSP protocol.

Basically have the same feeling about it and agree about the scopes to be sufficient in general.

It might need some tweaks in the syntax specific settings of the Default Packages to support AC better in general - independend from LSP.

Somehow it just feels like AC triggers don't play well with insert_snippet. What do I mean?

Considder a json key "class" was entered. By typing double-colon a snippet : |, is entered _(the | is the caret)_. I'd find it cool to see the AC panel right after the snipped was entered, if one of "auto_complete_triggers" matches the final caret position (defined by $0 in the snippet). Even have "auto_complete_with_fields": true, to explicitly enable completions in fields.

At least it should as soon as " is typed and punctuation.definition.begin being defined as trigger. With settings.auto_match_enabled the insert_snipped command is called, which results in : "|",. AC is not triggered - I guess because the trigger is evaluated against the last inserted character of the snippet?

See:

Notes:

1. AC panel is explicitly triggerd by ctrl+space.
2. The screencast was made with showing the "" issue. I am aware of other settings to be required to also support triggering : |, (just to say it clearly).

[rwols]

Removing the trigger chars for e.g. Python makes the completion not run when typing

self.

Because the scope is

self.
    ^ source.python

The dot receives a punctuation.accessor scope only after typing another character, say an i:

self.i
    ^ source.python meta.qualified-name.python punctuation.accessor.dot.python

This prevents AC from showing up.

For Javascript, the situation seems to be OK:

window.
      ^ source.js punctuation.accessor.js

This makes AC show up as intended.

For C and C++, the same is true: the punctuation.accessor scope is greedily scoped. So that's OK.

I generally like this idea, it seems more sublimey. Another big plus for me is that we can blame syntax authors to add greedy punctuation.accessor scopes. In the end the syntax becomes more precise and everyone wins.

For JSON, the situation is a bit special. The vscode-json-languageserver expects completions to be requested when encountering the start of a string, or when encountering a colon.

{
    "private": true,
//  ^ source.json meta.mapping.key.json string.quoted.double.json punctuation.definition.string.begin.json
    "scripts": {
//           ^ source.json meta.mapping.json punctuation.separator.mapping.key-value.json
        "dev": "npm run development",
        "development": "cross-env NODE_ENV=development node_modules/webpack/bin/webpack.js --progress --hide-modules --config=node_modules/laravel-mix/setup/webpack.config.js",
        "watch": "npm run development -- --watch",
        "watch-poll": "npm run watch -- --watch-poll",
        "hot": "cross-env NODE_ENV=development node_modules/webpack-dev-server/bin/webpack-dev-server.js --inline --hot --config=node_modules/laravel-mix/setup/webpack.config.js",
        "prod": "npm run production",
        "production": "cross-env NODE_ENV=production node_modules/webpack/bin/webpack.js --no-progress --hide-modules --config=node_modules/laravel-mix/setup/webpack.config.js"
    },
    "devDependencies": {
        "axios": "^0.19",
        "cross-env": "^5.1",
        "laravel-mix": "^4.0.7",
        "lodash": "^4.17.13",
        "resolve-url-loader": "^2.3.1",
        "sass": "^1.15.2",
        "sass-loader": "^7.1.0"
    }
}

In this case, it'd be best if a helper package (see #927) would provide the appropriate auto complete triggers.

Looking in the console, the default setting seems to be

>>> view.settings().get("auto_complete_triggers")
[{'characters': '<', 'selector': 'text.html'}, {'selector': 'punctuation.accessor'}]

So all LSP-json would have to do is provide

{'selector': 'source.json & (punctuation.separator.mapping.key-value | punctuation.definition.string.begin)'}

This would match the opening double-quote of a string and the colon:

>>> selector='source.json & (punctuation.separator.mapping.key-value | punctuation.definition.string.begin)'
>>> sublime.score_selector('source.json meta.mapping.key.json string.quoted.double.json punctuation.definition.string.begin.json', selector)
16384
>>> sublime.score_selector('source.json meta.mapping.json punctuation.separator.mapping.key-value.json', selector)
2048

But e.g. NOT the closing double-quote of a string:

>>> sublime.score_selector('source.json meta.mapping.value.json meta.mapping.value.json string.quoted.double.json punctuation.definition.string.end.json', selector)
0

[deathaxe]

I think the overall point is to try to leave the responsibility for proper AC settings (selectors, chars) to syntax packages in general. The idea is to add syntax-specific sublime-settings entries of each (default) syntax package, so AC could handle those situations with or without LSP.

If changes are needed to make AC work better for a syntax, an issue should be filed at the Default Packages repo.

[rwols]

But vscode-json-languageserver is very specific here. It has a certain workflow expectation that is not necessarily needed in the default JSON.sublime-syntax. It would be weird to add support for that in Packages/Javascript/JSON.sublime-syntax. Perhaps I'm not seeing the argument clearly.

[rwols]

Another scope that might need special attention is the $ from PHP and Bash. This has a scope of punctuation.definition.variable.

Perhaps we can maintain a dict, but ideally this would be handled by LSP-foo helper packages.

completion_selector_from_base_scope = {
    "source.json": "punctuation.separator.mapping.key-value | punctuation.definition.string.begin",
    "source.php": "punctuation.definition.variable",
    "source.shell.bash": "punctuation.definition.variable"
}

EDIT: I guess another option could be that we'd ask users to put it into their client config. Then LSP-* helper packages can also auto-fill that in.

[deathaxe]

It was more or less a very general idea. ;-)

[rchl]

Having it in LSP-* package is probably more correct as in theory two servers supporting same scope might need different triggers. But in practice, a general setting like that would probably work too. We could start with a generic setting and move to packages if that turns out not to be enough.

[FichteFoll]

The dot receives a punctuation.accessor scope only after typing another character, say an i:

That is definitely not ideal behavior. Would you mind opening an issue for that on the Packages repo so it isn't forgotten?

[rwols]

I'm not convinced that we should ignore the trigger characters.

For instance, clangd also completes file paths in include headers:

#include <

This will show a list relevant files on the filesystem.

#include "

As will this.

#include <foo/

The / character triggers another completion, showing a list of files in the folder foo. Intelephense has the same feature.

When ignoring trigger characters I don't think we would quickly notice a new "place of completion" of existing language servers in use. Someone would have to check in the logs if new triggers characters appeared, and would then have to experiment and figure out in what context those trigger characters apply.

[deathaxe]

The / would need to be scoped by the syntax to be able to be used as trigger, if the character is ignored.

Maybe the most flexibel solution was to add a auto_complete_triggers key to the client settings with a combination of known trigger chars and selectors, known to create a good user experience.

The characters announced by the server could be matched against that local map. If a new one was found, it receives the default scope and maybe a message is printed to console (always or in debug mode only).

Any new trigger character would work the same way as it does atm, but with the ability to tweak the behavior or limit it to certain situations. E.g.: I doupt you want AC to trigger after each division operator (/) in the code?

[rwols]

Maybe the most flexibel solution was to add a auto_complete_triggers key to the client settings with a combination of known trigger chars and selectors, known to create a good user experience.

I agree, though perhaps only the auto_complete_selector is sufficient.

[deathaxe]

We'd need the triggers to limit the usage of single characters provided by the server, not always though. Maybe provide both.