Skip to content
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

fix: check if cursor is on definition(#1042) #1086

Closed
wants to merge 3 commits into from

Conversation

shinohara-rin
Copy link

This pr addresses #1042 by checking if the cursor is on the same line as the definition of the symbol, if so, it will return references to the symbol instead.

@lukaszsamson
Copy link
Collaborator

Please add a test

@shinohara-rin
Copy link
Author

Please add a test

Hi! I've added a test, please let me know if this is okay.

@@ -1384,6 +1384,45 @@ defmodule ElixirLS.LanguageServer.ServerTest do
wait_until_compiled(server)
end)
end

test "find references if already on definition", %{server: server} do
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
test "find references if already on definition", %{server: server} do
test "definition provider finds references when cursor already on definition", %{server: server} do

@lukaszsamson
Copy link
Collaborator

I wonder if this is the correct approach. microsoft/vscode#73081 and microsoft/vscode@1711f6e suggests that VSCode is doing that logic in the editor, not in language server. If the definition range contains cursor, it emits a second request for references. If that is indeed the case then addressing #1042 would solve the issue without reimplementing editor logic in the server

@lukaszsamson
Copy link
Collaborator

Yes, I confirmed that VSCode emits references request and displays Peek References when I return range from definitions, see this branch for PoC https://github.com/elixir-lsp/elixir-ls/tree/return-ranges

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants