nwolverson / purescript-language-server

MIT License
184 stars 41 forks source link

Qualified auto import fix #143

Closed wclr closed 2 years ago

wclr commented 3 years ago

For example, if a user types Number.fromString (actually Number.anything) and there is no Number module namespace imported, the editor could propose a fix to add import Data.Number as Number (and other modules XXX.Number).

Would this be possible to implement?

nwolverson commented 3 years ago

Something like this is already implemented, in that auto-completion will bring in the qualified import for Number when completing Number.fromString. But on rereading I guess you are talking about writing the qualified identifier without completion and then fixing the resulting error?

(There is a bug around the qualified completion, raising a separate issue for that one - #144).

wclr commented 3 years ago

But on rereading I guess you are talking about writing the qualified identifier without completion and then fixing the resulting error?

If no Number is imported than Number.fromString can not be done with autocompletion obviously -) But I still may want to do this, if I know that I'll be able to get import Data.Number as Number as the fix for the error (that Number namespace is not found) I may want event just type Number. for this to get Number import proposal.

nwolverson commented 3 years ago

If no Number is imported than Number.fromString can not be done with autocompletion obviously -)

Why is this obvious? As I just pointed out that this feature exists. For Number.fromString (or relevant prefix) the LS will suggest Data.Number.fromString and import qualified Data.Number as Number on accepting the suggestion.

In fact the completions list will prioritise Data.Number highly over other matches for fromString because the module contains a matching component.

wclr commented 3 years ago

As I just pointed out that this feature exists.

Ah yes, I just checked, and it actually works with auto-completion, it suggests appropriate imports!

I'm not sure why I didn't notice that it already works. Probably because I wanted it without autocompletion when you type just Number.x (and say you don't remember the function name) and it proposes to import that can be appropriate (based on the name) and then use autocompletion from the already imported module. Though I think the already implemented workflow is quite a working solution.

So this issue is not a bug. And i'm not sure that get what is #144 about.

wclr commented 3 years ago

Though may this kind of related case for example after I inserted the code Number.toString and it shows the error. Then I should manually add this import, or make an emulation of autocompletion (remove toString and start typing it to get autocompletion and auto import).

nwolverson commented 3 years ago

Right, I agree this is a valid feature request, and of particular use when pasting code in, just may be somewhat lower priority given the other way exists. On "emulation of autocompletion", you should be able to trigger this manually somehow, in vscode by default it's ctrl+space, no need to alter text.

144 may be a vscode-specific issue, not sure, will eventually investigate and fix or update that issue.

wclr commented 3 years ago

On "emulation of autocompletion", you should be able to trigger this manually somehow, in vscode by default it's ctrl+space, no need to alter text.

You are right, I can trigger completion with ctrl+space for this case. The only thing is that cursor should be at the end of the function text, if you have a cursor say before fromString after auto-completion you get Number.fromStringfromString, which is funny and probably a vscode's auto-completion feature buggy behaviour.

Eventually, such recepies for working with vscode auto-completion features should be added to the quide, as seem to be easy to miss out. https://github.com/nwolverson/vscode-ide-purescript#autocomplete