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

When importing, the editor suggestion are quite useless #11

Open
VladimirMarko opened this issue Nov 13, 2021 · 1 comment
Open

When importing, the editor suggestion are quite useless #11

VladimirMarko opened this issue Nov 13, 2021 · 1 comment

Comments

@VladimirMarko
Copy link

VladimirMarko commented Nov 13, 2021

I went to https://elm-editor.com/ and triggered the suggestions while importing.

Here, Browser or Html is not suggested, even though the rest of the code is using them:

elm-editor bad import suggestions

Again, Html.Events is not suggested:

elm-editor bad import suggestions 2

In fact, all of the suggestions are nonsense. Html.Bitwise does not exist.

EDIT: Further testing shows that it's worse than I thought:

elm-editor missing imported suggestions

Apparently, no dependencies are considered when generating suggestions. This makes them quite useless indeed.

(The project had been already compiled with the dependency at the point the screenshot was taken, so the suggestions should definitely had been available.)

As a learner, this means that I cannot rely on the editor to discover how the dependencies (that I have already chosen to work with) work.

EDIT 2: Testing further and more nonsense suggestions:

elm-editor missing imported suggestions 2

@VladimirMarko VladimirMarko changed the title When importing, editor suggestion quite useless When importing, the editor suggestion are quite useless Nov 13, 2021
@Viir
Copy link
Member

Viir commented Nov 15, 2021

I went to https://elm-editor.com/ and triggered the suggestions while importing.

Here, Browser or Html is not suggested, even though the rest of the code is using them:

elm-editor bad import suggestions

With this test process, you were viewing the default project from https://github.com/elm-fullstack/elm-fullstack/tree/f6bccc95add7755ccae31ac310b6cdd99c2ef7d7/implement/example-apps/elm-editor/default-app

In this case, the module named Browser is not part of the project. Instead, it comes via a reference in the elm.json file when using the 'Compile' function.
The language service uses the modules present in the project to derive the completion suggestions. With the current implementation, it does not consider the references in elm.json that could bring more modules.

Seeing this issue, I think we can improve the default project to avoid confusion in this area.

Again, Html.Events is not suggested:

elm-editor bad import suggestions 2

In fact, all of the suggestions are nonsense. Html.Bitwise does not exist.

That screenshot looks like the branch handling suggestions for import statements does not consider the prefix.

As far as I see, the test suite does not yet contain a case like seen in the screenshot:
https://github.com/elm-fullstack/elm-fullstack/blob/f6bccc95add7755ccae31ac310b6cdd99c2ef7d7/implement/example-apps/elm-editor/tests/LanguageServiceTests.elm#L253-L685

So first step could be adding that case to the automated tests in LanguageServiceTests.elm

The change to fix the productive side will probably around here:
https://github.com/elm-fullstack/elm-fullstack/blob/f6bccc95add7755ccae31ac310b6cdd99c2ef7d7/implement/example-apps/elm-editor/src/LanguageService.elm#L458-L462

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants