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

Suggest funs and modules from metadata #24

Merged
merged 10 commits into from
Aug 8, 2019

Conversation

lukaszsamson
Copy link
Collaborator

I noticed that module, function and macro suggestions only work for compiled modules. This is irritating when working on unfinished code. With the changes from this PR we are able to make suggestions basing on metadata from currently evaluated piece of code as a fallback if the module is not compiled.
I also did some refactoring in Complete module, most noticeably the aliases are no longer passed through application env

@@ -12,6 +12,7 @@ defmodule ElixirSense.Core.Metadata do
lines_to_env: %{},
calls: %{},
vars_info_per_scope_id: %{},
mods_funs: %{},
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is it a good idea to merge this prop with mods_funs_to_positions?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, we could have the position in the ModFunInfo struct. However, I think mods_funs_to_positions always keep an entry for {mod, func, nil} as a fallback for cases where we cannot introspect the arity. Ideally, we should start using the new Metadata.get_call_arity/3 (see example here). It works for both functions and types, so we could improve "Go to definition" accuracy.

@spec find(String.t, [module], [{module, module}], module, [String.t], [String.t], [module], State.scope, String.t) :: [suggestion]
def find(hint, imports, aliases, module, vars, attributes, behaviours, scope, text_before) do
@spec find(String.t, [module], [{module, module}], module, [String.t], [String.t], [module], State.scope, %{}, String.t) :: [suggestion]
def find(hint, imports, aliases, module, vars, attributes, behaviours, scope, mods_and_funs, text_before) do
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This api needs refactoring

defp find_hint_mods_funcs(hint, imports, aliases) do
Application.put_env(:"alchemist.el", :aliases, aliases)

list1 = Complete.run(hint, imports)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm pretty sure this is no longer needed as the call Complete.run(hint, env) covers imports and local module calls

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the Complete module will eventually need to be refactored or even replaced. It's more complex than it should especially because of the hint stuff, which is a nice feature for iex but I'm not sure it's relevant for editors.

@msaraiva
Copy link
Collaborator

msaraiva commented Aug 7, 2019

@lukaszsamson Let me know if you're still working on this one. if you prefer I can merge it and then we can work on the adjustments later.

@msaraiva msaraiva mentioned this pull request Aug 7, 2019
@lukaszsamson
Copy link
Collaborator Author

@msaraiva I haven't started anything serious besides already committed changes. Let's merge it now as the refafactorings are going to be too big for one PR.

@msaraiva msaraiva mentioned this pull request Aug 8, 2019
@msaraiva msaraiva merged commit b01ec70 into elixir-lsp:master Aug 8, 2019
@msaraiva
Copy link
Collaborator

msaraiva commented Aug 8, 2019

@lukaszsamson great stuff. Thanks! 👍

@lukaszsamson lukaszsamson deleted the metadata-suggestions branch August 18, 2019 13:24
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.

None yet

2 participants