halohalospecial / atom-elmjutsu

A bag of tricks for developing with Elm. (Atom package)
https://atom.io/packages/elmjutsu
MIT License
192 stars 24 forks source link

'Go to Definition' fails on some nested modules #30

Closed diegonc closed 7 years ago

diegonc commented 7 years ago

Hello

I've got an import that reads like in the snippet below

import Result.Extra as Result

When the cursor is positioned in the Result.Extra part and I trigger the Go to Definition command an error pops up saying it cannot find the source file.

image

Apparently elmjutsu doesn't replace the dot with a directory separator character and as a result it tries to open the incorrect file.

Additionally, when the cursor is in the Result token (after the as token) the same message pops up and two tabs appear; both having the same file opened: the Result.elm from elm-lang/core module.

halohalospecial commented 7 years ago

Hi! I installed elm-community/result-extra v2.0.1 and Go To Definition worked as expected. Maybe you can give me sample project directory that demonstrates the issue?

As for why 2 tabs were opened, elmjutsu could not determine if you're talking about the module Result or the type Result. You'll notice that in one of the tabs, the cursor is at module Result exposing. In the other tab, the cursor is at type Result error value.

diegonc commented 7 years ago

I can reproduce it with this repo: https://github.com/diegonc/elmjutsu-issue-30

Sorry, I forgot to mention that I'm still using elm 0.17; thus my version of result-extra is 2.0.0. And Windows 10.

Two tabs: ACK, I now see the difference 👍

halohalospecial commented 7 years ago

Hi! I don't have Windows 10, but I tried this in Windows 7 and it's also working properly. Maybe your elmjutsu version is outdated? You can try to reinstall elmjutsu, then restart Atom. Thanks!

diegonc commented 7 years ago

I'm working with Atom 1.12.7 and elmjutsu 2.12.1 and still fails here 😢

I've been looking at the code, here, and using Atom's developer console I found that that port subscription gets the following array when I try to go to Result.Extra definition:

[{
    "filePath": "D:\\diego\\PROGRAMACION\\Src\\elm-lang\\elmjutsu-issue-30\\Main.elm",
    "projectDirectory": "D:\\diego\\PROGRAMACION\\Src\\elm-lang\\elmjutsu-issue-30"
}, {
    "fullName": "Result.Extra",
    "sourcePath": "http://package.elm-lang.org/packages/elm-community/result-extra/2.0.0/Result.Extra",
    "caseTipe": null,
    "kind": "module"
}]

The function then processes sourcePath in the second element of the array by removing some prefix and splitting by / ending with the following parts:

const [namespace, packageName, version, moduleName] = parts;
/*                                      "Result.Extra" */
/*                             "2.0.0"                 */
/*                "result-extra"                       */
/*    "elm-community"                                  */

Finally using those parts a file path is constructed:

filePath = path.join(activeFile.projectDirectory, 'elm-stuff', 'packages',
                     namespace, packageName, version, 'src',
                     moduleName.replace('-', path.sep)) + '.elm';

The moduleName gets its dashes replaced with the path separator. I believe dots should be replaced instead. Dashes are not allowed in module names anyway.

BTW, modules in my own project open just fine. The sourcePath comes with the dots already replaced:

{
    "fullName": "A.B",
    "sourcePath": "D:\\diego\\PROGRAMACION\\Src\\elm-lang\\elmjutsu-issue-30\\A\\B.elm",
    "caseTipe": null,
    "kind": "module"
}
halohalospecial commented 7 years ago

That's the weird part. I'm getting http://package.elm-lang.org/packages/elm-community/result-extra/2.0.0/Result-Extra for the sourcePath. Notice the dash in Result-Extra. You can also try to delete the directory set as Cache Directory so that elmjutsu will fetch the 3rd party package docs again. Thanks for debugging this.

diegonc commented 7 years ago

Oh that's weird, ideed. Also, my URL gives a 404 while yours works just fine. I'll try deleting the cache.

diegonc commented 7 years ago

Ok, I found the issue. There's this little bugger package named colors, coming from atom-typescript, that adds a non-pure zalgo property to the String's prototype breaking character equality in Elm.

After disabling atom-typescript and reloading the window this package works correctly again. 🎉

image

halohalospecial commented 7 years ago

Oh! Things like this are my biggest problems with Atom. Since it's so customizable, a package can affect the behavior of other packages. Good thing you were able to resolve this! :tada: