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

feat(lsp): Allow goto definition on include statements #2027

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

spotandjake
Copy link
Member

This pr allows for you to use go to definition on Include statements.

Work for: #1624

Note:
As we did not reference the source module from the include statement before this change required adding that attr, The best way I could find to do it was when we got the module name. An alternative approach would have been to pass the sod.pinc_path.txt on in include_module but this would lead to inconsistencies with the include format.

@spotandjake spotandjake added the lsp Issues related to the language server. label Feb 15, 2024
@spotandjake spotandjake self-assigned this Feb 15, 2024
@ospencer
Copy link
Member

Is there not a valid location for the included module available from the environment?

@spotandjake
Copy link
Member Author

Is there not a valid location for the included module available from the environment?

I looked for one but couldn't find it.

@spotandjake spotandjake force-pushed the spotandjake-goto-include branch from bfcb543 to b9c1878 Compare March 29, 2024 22:43
@@ -104,7 +105,15 @@ let include_module = (env, sod) => {
);
let newenv = Env.include_module(mod_name, sod, env);

let od = {tinc_path: path, tinc_loc: sod.pinc_loc};
let mod_file =
Copy link
Member

Choose a reason for hiding this comment

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

Just one question before I press approve. When would the module file end in .wasm ?

@ospencer
Copy link
Member

I spent some time looking into this and I don't see an easy path forward right now. The method you're using right now to get the source is a bit fragile because it's looking at where the object file is rather than where the source is. We may want to just add a field to the objects that points to the source so we can read it from there.

@spotandjake spotandjake marked this pull request as draft February 16, 2025 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lsp Issues related to the language server.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants