-
Notifications
You must be signed in to change notification settings - Fork 180
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
Add go to definition support for autoloaded constants #1893
Comments
Hello @vinistock , I'm interested in this issue, can I handle it? |
Sure! Feel free to put up a PR. |
Excuse me, I have finished jump-to-file logic but I notice that I don't have the permission to push my own branch so that I can't put up my PR, @vinistock, could you please give me the permission? |
@Super-Xray Usually, devs contribute to an OSS project by forking it, pushed the new branch to the fork, and then open a PR from the fork against the target repo. |
Thank you very much! Since this is my first OSS contribution, I didn't know this before. |
Sorry, I havn't fully understand this completely, "We can probably add the same for the constant too, since we know that it must resolve to Foo::Bar, which makes it easy to search in the index.", do you mean from ':Bar' in this autoload statement jump to the its class definition line? |
That's what I originally meant, but thinking more about it, I think we can simplify this. Since the definition listener will only handle method call events (and not symbol or string events), it'll be hard to differentiate between the arguments. And the most specific jump you can make is to the constant, so the file path doesn't provide much value. We can change the definition listener to do something like this class Definition
def on_call_node_enter(node)
message = node.name
case message
when :require, :require_relative
handle_require_definition(node)
when :autoload
handle_autoload_definition(node)
else
handle_method_definition(node)
end
end
private
def handle_autoload_definition(node)
# Get the name of the constant from the first argument of the call node
# Invoke find_in_index with the name of the constant
end
end |
haha! That's what I think about too! using |
The For example module Foo
autoload :Bar, "some/file"
end In this case, you need to invoke This will already make the LSP jump to the exact location where that constant is defined. You won't need the |
Thank you very much! I found this way didn't work before probably because the 'ruby-lsp' project is an sorbet project so that |
Basically, the same feature as go to definition for require, but in this case for autoloads.
We can probably add the same for the constant too, since we know that it must resolve to
Foo::Bar
, which makes it easy to search in the index.The text was updated successfully, but these errors were encountered: