Align "Expand Selection" fallback behavior with "Goto Definition" and "Find References" #2325
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The "Expand Selection" command has a
fallbackargument, similar to "Goto Definition" and "Find References", which allows to invoke Sublime's built-in "Expand Selection" if there is no LSP result for the caret position or if the server doesn't have this capability. But for LSP's "Expand Selection" this fallback is enabled by default, while it is turned off for "Goto Definition" and "Find References". I did set thefallbackto be enabled in #2124 to preserve former behavior of that functionality. But now I see no good reason why its fallback behavior should be different from the other commands, and I think it is confusing to see the context menu item for "Expand Selection" under the LSP submenu, even when there is no language server running at all (or if it doesn't have this capability), and also the main menu item not to be greyed out in that case. See also https://forum.sublimetext.com/t/is-there-a-tree-sitter-like-code-navigation-functionality-in-sublime/66442/6I think it was originally enabled by default, because at the time of implementation there wasn't the
lsp.session_with_capability"context" key for key bindings, if I understand the discussion in #1092 (comment) correctly. But now this shouldn't be needed anymore.This PR changes the default value for "Expand Selection" to make the behaviors consistent, and so that the context menu item for this is not visible when there is no LSP running at all.
This could potentially still have an impact on existing key bindings, but only if there is a server which responds with no results for a given caret position while ST's built-in "Expand Selection" would still do something. But I'm not sure if this is really a realistic scenario. For servers without the capability it shouldn't make any difference, assuming the key binding was copied from the template including the "context" value.
Alternatively the
fallbackvalues of "Goto Definition" and "Find References" could be changed to be enabled by default.