-
Notifications
You must be signed in to change notification settings - Fork 181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use caret for point to look up symbol #2440
Conversation
This commit changes the hover functionality such that the caret, not the beginning of the selection, is used for the symbol lookup.
Note that I have tested with a few language servers, and all of them were smart enough to respond with the correct result even if the point (caret) is at the very end (i.e. actually behind) an identifier name. Based on this, I would assume that the proposed change should be acceptable, and I agree that it would feel more natural to always use the caret position. But I can't rule out whether there are any unintended side effects. |
It make sense to use the caret position. The previous logic was added in June 2020, and
It is not easy to predict if it causes unintended side effects or not, |
I have used this code for the past 6 months or so, haven't noticed any changes. I've tested the commands you mentioned above, and I have not noticed any changed behavior. Is it possible to move forward with this? If not, would you be okay with a PR that makes this behavior optional, configurable with a boolean in the settings? |
I'm more for the change you proposed. I'll try to find the time next week. |
✅ Deploy Preview for sublime-lsp ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
I am starting to test this. @trbjo can you give me more detailed steps on how to reproduce this case(or a short recording of your screen)? I cannot really reproduce the behavior with the description from here:
|
This comment was marked as off-topic.
This comment was marked as off-topic.
Before I would merge this, because the change makes sense.
Like I said, |
I haven't discovered that this PR breaks anything. So if anyone has an example that this PR solves, feel free to provide it. |
This commit changes the hover functionality such that the caret, not the beginning of the selection, is used for the symbol lookup. I use expand_to_... to navigate, and it's annoying that when I land on a symbol, I can't see the popup before the selection is empty again. I think this behavior is more natural and in accordance with what the user expects.