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

Quickfix for importing modules #161

Open
imerkle opened this issue Mar 25, 2020 · 13 comments
Open

Quickfix for importing modules #161

imerkle opened this issue Mar 25, 2020 · 13 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@imerkle
Copy link

imerkle commented Mar 25, 2020

It would be great to have quickfix suggestions for importing modules.

@axelson
Copy link
Member

axelson commented Mar 26, 2020

Can you give some example scenarios when a quickfix suggestion for importing a module would be shown? Although, it might make more sense to provide a quickfix suggestion for aliasing a module.

@lukaszsamson
Copy link
Collaborator

Quickfix import, alas and require would definitely be useful. Unfortunately I don't think it's doable without compilation traces (which means elixir >= 1.10).

@hworld
Copy link
Contributor

hworld commented Mar 27, 2020

Was just gonna make an issue for this one. I'd also love to type out the last part of a module, ie the part after the last dot, and have it suggest all modules that might match that. Completion would add the alias.

PHP has a similar system to Elixir with the new-ish namespaces feature. It gets really tedious typing out the full name, so you usually "use" it at the top like an alias. Here's an example of what it looks like in VSCode.
Screenshot from 2020-03-26 22-10-31

If we had the data available (seems like that compiler tracing PR would need to get merged first?), I wouldn't mind taking a stab at implementing. I think it would save a ton of time for me while writing Elixir code.

@axelson
Copy link
Member

axelson commented Mar 27, 2020

@lukaszsamson couldn't we support a quickfix for alias using a similar module list as the one that we used for the workspace symbols provider? Although compiler tracing might make it easier, but I think it will be a while before we want to require that users run their code with Elixir 1.10. Also I think adding a require quickfix would involve detecting the you must require Logger before invoking the macro type warning and generating a require Logger statement near the top of the file. I think that actually might be the easiest quickfix to start with, although the alias example that @hworld gives would be the most useful. I'm not personally sold on a quickfix for import because there's a huge list of functions that you could import and I also think that it's easy to overly rely on import which can then cause recompilation times to increase so I don't think it's something that the tooling should be encouraging (although I'm open to changing my mind on this).

@axelson
Copy link
Member

axelson commented Mar 27, 2020

Also @hworld a PR would be very welcome that provided similar functionality to the php "use" that you show, although that be implemented by providing a TextEdit to additionalTextEdits of a CompletionItem (Completion Request docs), and a PR (and associated issue) would be very welcome 👍

@axelson axelson added the help wanted Extra attention is needed label Apr 28, 2020
@lukaszsamson
Copy link
Collaborator

@lukaszsamson couldn't we support a quickfix for alias using a similar module list as the one that we used for the workspace symbols provider?

@axelson Yep, it might be doable. We'd need to parse compiler warnings/errors, extract the module thet misses requre/alias, do the search and find place to insert code.

I wouldn't mind taking a stab at implementing. I think it would save a ton of time for me while writing Elixir code.

@hworld If you would like to take it up then open a PR to SuggestionsProvider in https://github.com/elixir-lsp/elixir_sense

@ajayvigneshk
Copy link
Contributor

ajayvigneshk commented May 9, 2022

I've opened a draft elixir-sense PR regarding this.

I'm also working on an PR for elixir-ls. One thing that I would like to get some suggestions on is, how to find the line where the alias should be added. I'm guessing this has to be preferrably at a module scope. The closest idea that I've is using the {:alias,...} compiler trace event to get the first line at a module scope and insert before that(as multi-aliases that spawn multiple lines are listed by the event as belonging to the first line). But what about modules that don't have any alias es at all? Would inserting at a line before the current cursor be okay for it?

@dylan-chong
Copy link

You could place it before any module attributes, but after any @moduledoc, use or import calls? That's how I've always styled it personally. (Although if you put the alias before use/import, you can then alias the module getting imported/used, if that's something you want)

@ajayvigneshk
Copy link
Contributor

Yeah, what am not sure is how to programmatically get the line that's safe to insert for the alias. Because the module attributes could be multi-line, I don't think it would be safe to insert say on the next line of a module attribute always

@dylan-chong
Copy link

dylan-chong commented May 9, 2022 via email

@ajayvigneshk
Copy link
Contributor

That would be nice. I just don't know how to figure out the line that comes after a module attribute (considering multiline strings and such).

@dylan-chong
Copy link

dylan-chong commented May 9, 2022 via email

@dylan-chong
Copy link

dylan-chong commented May 9, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

6 participants