pine-vm/pine

When importing, the editor suggestion are quite useless

VladimirMarko opened this issue · 1 comments

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

Viir commented

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