LSP
LSP copied to clipboard
sublimelsp
Add insert replace support
New:
- setting for determining the
completion_insert_mode(defaults to"insert") Available:"insert"or"replace". -
enter/tabwill commit the completion item with the default insert mode. -
shift+enter/shift+tabwill commit the completion item with the opposite insert mode.
I have tested this only with the LSP-angular server.
It's pretty cool, but at least Shift+Tab conflicts with a default keybinding when the autocomplete popup is visible (described in the ST preferences under "tab_completion"). Maybe it's better to leave the keybinding commented out by default, like most of the other keybindings? Additionally I would add another context {"key": "lsp.session_with_capability", "operand": "completionProvider"} to it.
An alternative could be Ctrl+Tab ("primary+tab"), which indeed has a default binding too (switch to next file tab), but not specific to when the autocompletion popup is visible. But it's not as easy to type as Shift+Tab.
The possible options "insert"/"replace" should probably also be mentioned in the comment which describes the setting.
I was thinking of adding additional text in the st_detail field to distinguish insert/replace variants, instead of a command. The current diff isn’t very discoverable. I wonder how many people will really use this feature.
I was thinking of adding additional text in the st_detail field to distinguish insert/replace variants, instead of a command. The current diff isn’t very discoverable. I wonder how many people will really use this feature.
Even knowing about this feature, I probably wouldn't remember to pick the right insert mode by using a different shortcut. Especially if we don't indicate anywhere that the "replace" is supported by the given completion. Without indication, the user would never know if both the server and the given completion supports it and it would be a guessing game.
I would maybe even remove the setting and always use the "insert" mode with the regular Tab/Enter binding for completions - just like it is now, and because it's the way how completions work in general in ST.
Without indication, the user would never know if both the server and the given completion supports it and it would be a guessing game.
I agree, this should be indicated in some way. Otherwise it would be confusing why it doesn't work most of the time (probably most servers don't support it).
I was thinking of adding additional text in the st_detail field to distinguish insert/replace variants
If I understand it correctly, that would mean to duplicate the entries which use InsertReplaceEdit? I don't like this approach a lot, because multiple entries with the same label is the thing I dislike the most in autocompletions. It requires to read more info (from the annotation hint or details field) to pick the correct one, and it breaks the typing flow if you need to scroll down first in case the order is wrong (or if you want to see the details for the other entries). So I think I would prefer a keybinding for the "replace" mode, provided that it doesn't conflict with an existing default binding.
I tried to mimic VS code behavior with this PR.
https://m.youtube.com/watch?v=QIze0qkugNc
In the PR description I haven't explained in detail why I implemented this the way I did and that is simply that users switching from VS Code to ST could easily transfer their knowledge.
I wanted to create a second draft, that would address all the feedback from this draft and then I would let people decide between the 2 drafts.
Currently I do not think that I will do such a draft in the near time.
In case it makes it easier, I'm gonna state now that I'm fine with how it is now if you just update the setting description. I will look once again through the changes after it's rebased and addressed.
As far as disabling key bindings by default, I'm not sure... That would make the feature very niche. I think that it would be fine to have them enabled by default as long as the behavior fall backs to ST default if completion doesn't use InsertTextReplace. Not sure how easy that would be though...
When it comes to marking completion details with something to indicate that insert/replace is supported, this could be done without duplicating completions. The challenge would be to figure out what to show as it might be hard to find a short text/symbol that would be easily recognizable and meaningful to everyone.
I added the following text next to the More link in the autcomplete popup, to make it more noticeable.
But I am still unsure if I would like to clutter the autocomplete popup with this text...
Note: I had
"completion_insert_mode": "replace"in LSP.sublimse-settings, and thus I saw"⇧ + ↵ to insert"
Should we rename
lsp_commit_completion_with_opposite_insert_mode to something more `short:
a) lsp_complete_with_opposite_insert_mode
b) lsp_insert_replace_complete
Closes #1413
I think this needs to be added to the initial comment to actually have an effect on merging.
Seems fine to me. Someone might still be using it to insert a line break but since shift+enter works it might not be a problem.
I'm still thinking how we could visualize if completion supports insert/replace...
- It shouldn't mention specific key binding as we can't tell which one is defined.
- It shouldn't take too much space.
Some ideas:
- add
[I/R]in the details field -- a bit too cryptic perhaps? - say "(supports replace)" if "insert" is the default mode and "(supports insert)" if "replace" is the default mode -- a bit long?
- ???
How about the following?
Provide a default keybinding "alt+enter", "Replace" or "Insert" will be show at the bottom of the AC popup. So that can give an indication that the server supports insert/replace.
