search

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

Give priority to target module hints when stringifying imports #42

Closed stil4m closed 7 years ago

stil4m commented 7 years ago

Will solve #41.

The bug was introduced by using the first Hint, but it may be the case that the first hint is not a hint from the target module. Solved this by partitioning the list and then concatenating it again. I believe that this will fix it (modules cannot declare a certain type/type alias twice and thus the first Hint will be the correct import reference.

One thing I do not understand is why this code relies on Hints? This makes it prone to these kind of errors. What is the use case for this?

Should I also generate the new indexer.js?

halohalospecial commented 7 years ago

@stil4m, thanks a lot for the PR! I'll try this out tomorrow. This package started out as an experiment to show type hints in Atom (the initial code was from Try Elm). I only wanted to have the "Sidekick" functionality to help me read and understand Elm 0.17 code (I was using 0.16 back then). I realized that I could have other features like Go To Definition by just using the hints, and haven't had the chance to refactor the code since then. The ideal and accurate solution would really be to use ASTs and I'm patiently waiting for the core team to release that :-)

halohalospecial commented 7 years ago

You can generate indexer.js if you like, else I can do it after testing. I had to include that in the repo since it's going to be used by Atom.

stil4m commented 7 years ago

I think it is better for you to generate the javascript. That keeps this PR simple and clean. I had to figure out how the project was set up. Are you planning to add build scripts? I can do this in a new PR if you like.

About the hints: That clears things up :)

halohalospecial commented 7 years ago

Thanks for the offer! You can submit a PR for that. I'll generate the js file after testing. Thanks again!