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

Fix completion and eldoc for Elm 0.19, broken by reliance on elm-oracle #142

Closed
purcell opened this issue Oct 2, 2018 · 25 comments
Closed
Assignees

Comments

@purcell
Copy link
Collaborator

purcell commented Oct 2, 2018

It seems that elm-oracle is unlikely to get updated satisfactorily for Elm 0.19, and it would make sense to work around this by taking matters into our own hands. The vscode Elm support has done exactly this.

The completion system in elm-mode currently caches the results of elm-oracle queries for currently-imported libraries in memory. Dependency source code is stored in ~/.elm and dependencies are listed in elm.json, so I envisage a system whereby docstrings and function signatures would be obtained directly from the source code via elisp parsing and cached in memory. If done correctly, this approach would also allow us to provide completions and xref "jump-to-definition" navigation for identifiers defined in the user's source code, which was never supported by elm-oracle.

Unfortunately this all needs a good chunk of work. I'm likely starting a big Elm 0.19 project soon, which might prompt me to tackle this for my own sake.

@purcell
Copy link
Collaborator Author

purcell commented Oct 2, 2018

Also see #134 and #136.

@purcell
Copy link
Collaborator Author

purcell commented Oct 6, 2018

Update: I worked on this a little bit and wrote an elm-meta.el module which can extract info from elm.json or elm-package.json and knows how to find the source for a given module name either in the current project or the cached source of any of its dependencies. It's early days, but this will form the basis for scanning modules to find symbols that are available to the current project for completion and navigation purposes.

@purcell purcell self-assigned this Oct 6, 2018
@mewa
Copy link
Contributor

mewa commented Oct 9, 2018

While I feel integrating a parser is the way to go, I have a couple concerns.

Namely, type annotations are not obligatory, hence when parsing un-annotated expressions we'd have no way of telling their type signature, unless we performed some compilation steps.

I feel that the effort required to implement this from scratch in Elisp is so huge that it could easily overwhelm everyone involved in this package. And not just because it's a time-consuming task on its own - but we have to keep in mind that it this parser will have to be then kept in sync with the upstream version of elm compiler. While changes are unlikely to occur there frequently, individual action would have to be taken to port each such change back to Elisp.

In my opinion, it would be a good move to fork elm/compiler and leverage the existing AST parser (and compiler, to some extent), ensuring both correctness of provided completions, as well as an easy way to upgrade between newer versions of Elm (basically cherry-picking changes from elm/compiler). As an added benefit, this could attract attention from contributors outside of the Emacs ecosystem.

@rl-king
Copy link

rl-king commented Oct 9, 2018

@mewa It might be worth noting that elm-format is working on something you're talking about https://github.com/avh4/elm-format/milestone/3

@purcell
Copy link
Collaborator Author

purcell commented Oct 10, 2018

@mewa:

Namely, type annotations are not obligatory, hence when parsing un-annotated expressions we'd have no way of telling their type signature, unless we performed some compilation steps.

We don't need type annotations: if the source code doesn't declare it, then type annotations simply won't appear in completions, but the functions and types themselves would still be detected and available for completion/navigation etc.

I feel that the effort required to implement this from scratch in Elisp is so huge that it could easily overwhelm everyone involved in this package. And not just because it's a time-consuming task on its own - but we have to keep in mind that it this parser will have to be then kept in sync with the upstream version of elm compiler. While changes are unlikely to occur there frequently, individual action would have to be taken to port each such change back to Elisp.

I agree it's non-trivial, but I think it's not as hard as you're imagining. The main fiddly cases are multi-line type declarations with interspersed comments and data type constructors, and I feel that these are all quite manageable.

I'd also note that we've relied on an upstream tool up to now, and suddenly (but perhaps not unexpectedly) have been left high and dry.

In my opinion, it would be a good move to fork elm/compiler

That sounds like massively more work IMO, even if it would be ideal for the community and all us tool-writers.

@rl-king:

It might be worth noting that elm-format is working on something you're talking about https://github.com/avh4/elm-format/milestone/3

I'll be interested to see if this works out, but it feels a bit backwards that elm-format should be repurposed to do this.

@purcell purcell changed the title Fix completion and eldoc to avoid relying on elm-oracle Fix completion and eldoc for Elm 0.19, broken by reliance on elm-oracle Nov 14, 2018
@purcell
Copy link
Collaborator Author

purcell commented Nov 17, 2018

It looks like elmi-to-json might be helpful here...

@purcell
Copy link
Collaborator Author

purcell commented Jan 12, 2019

I've been stalled on this, but just pushed what I had already written to a branch: https://github.com/jcollard/elm-mode/compare/completion-0.19

The branch adds elm-meta.el, which is an elisp module for finding the files in which specific Elm modules are defined. Mapping modules to files is part of the machinery needed for completion: the other part is parsing those files to extract the names of the exported symbols. elm-meta is quite rough and ready: in the worst case where users have lots of versions of libraries installed across multiple projects, it would need to reproduce Elm's dependency solver. In most cases, though, the latest version of any given module (for the current Elm compiler version) should do the trick, and that's what elm-meta supports.

@dragonwasrobot
Copy link

dragonwasrobot commented Jan 14, 2019

dunno if this is in anyway helpful, so please forgive my ignorance, but it looks like there is currently work on an elm language server, https://github.com/elm-tooling/elm-language-server, wondering if that could be helpful in getting closer to better Elm support in combination with https://github.com/emacs-lsp/lsp-mode?

@purcell
Copy link
Collaborator Author

purcell commented Jan 14, 2019

Yeah, if progress is made on that, then I imagine a good way forward would be to completely drop completion / navigation support from elm-mode and recommend that it be used in conjunction with an lsp-mode plugin for Elm, which would probably be distributed separately. From what I understand, though, lsp-mode is still a bit rough.

@dragonwasrobot
Copy link

dragonwasrobot commented Jan 14, 2019

I see, and thanks for the quick response (and a nice emacs package 👍)

@flooose
Copy link

flooose commented Jun 3, 2019

Hi! Is the commit above 1.) something we can start using and 2.) something you'd like us to use to start getting feedback on your work?

@Daniel-V1
Copy link

Just came back around to this issue, and wanted to say that the elm language server is now working pretty great, and there is a client included in lsp-mode now! Combined with this mode, elm dev in emacs is looking pretty good!

@purcell
Copy link
Collaborator Author

purcell commented Jun 3, 2019

the elm language server is now working pretty great

Good to hear. Are you using it with lsp-mode or eglot?

@Daniel-V1
Copy link

Lsp-mode

@purcell
Copy link
Collaborator Author

purcell commented Jun 3, 2019

Thanks!

@purcell
Copy link
Collaborator Author

purcell commented Jun 3, 2019

there is a client included in lsp-mode now

Such great attention to detail from me there, LOL.

But still worth asking, I guess. I test drove both eglot and lsp-mode for the first time a few days ago, and found both a little frustrating tbh, but it's hard to ignore the weight of developer support that LSP has, so it definitely seems the way to go in future.

@Daniel-V1
Copy link

lol no worries. Thanks for this package!

Yeah, personally I never tried eglot because lsp-mode seemed to have a lot more community behind it, and its what works with dap-mode for languages that support it. Anything in particular you found frustrating in lsp-mode?

@purcell
Copy link
Collaborator Author

purcell commented Jun 4, 2019

Anything in particular you found frustrating in lsp-mode?

There's a lot of weird opinionated set-up that gets run by default via magical non-standard methods, e.g. flycheck messages suddenly get shown on the right side of the window, and that stuff's really hard to override. I'd prefer if the basic package just integrated in a standard way with company, flycheck etc. But I guess most people just appreciate that it does useful stuff out of the box without fiddling.

@flooose
Copy link

flooose commented Jun 4, 2019

@Daniel-V1 is there anything you had to do other than what's on the lsp and elm language server sites to get lsp-mode and elm-language-server working? I tried going that route but had some problems https://emacs.stackexchange.com/questions/50832/configuring-lsp-mode-to-run-elm-language-server

@Daniel-V1
Copy link

@flooose No, it works for me just like that. I took a look at the elm-ls::stderr buffer on mine though and it definitely is throwing some weird exceptions. The functionality still seems to work though, I've got completions/docs, etc.
I think it takes a while sometimes for the language server to do all its parsing of the files it needs to provide the stuff though. This issue, elm-tooling/elm-language-server#50, may be related.
Are you getting the features after a while/with other elm files after the first?

@flooose
Copy link

flooose commented Jun 5, 2019

In the *lsp-log* buffer it says "Done parsing all files.", so I guess that's not the problem. Opening other files didn't change anything either. I get the same error, where on the first line it says "Error parsing file".

I also get eldoc errors and don't know who's causing them, or activating eldoc in my elm buffers, so I guess I have to debug a little more :)

Are you using nvm or something like it?

@Daniel-V1
Copy link

Daniel-V1 commented Jun 5, 2019

Oh, so you're getting an elm-analyse error about error parsing file on the first line of your elm file? This happened to me too, for me it was because elm-analyse freaks out about not having Module declaration at the top of the file (even though there generally aren't in the examples), and then everything else seems to die. Once I added
module ModuleName exposing (..)
to the top of my file, the error changes to elm-analyse: Exposing all and then I can get the rest of the functionality working.
Yeah, the eldoc activation at least is a part of lsp-mode I think. Like @purcell said, it does a few magic things behind the scenes when you turn lsp-mode on, though I think you can turn them off if you'd like.
No nvm for me, just node, which is 11.15.0 currently

@flooose
Copy link

flooose commented Jun 5, 2019

That was it! It's not totally streamlined yet because some of the documentation popups cover up all of the source code sometimes, but I've got support for the stuff you mentioned above and I can work with it now. Thanks!

I'm not sure if you're into collecting points on emacs.stackexchange, but if you want to post the answer, I'll give you the ✔️ for the solution. Otherwise I'll just answer it, in case other people have this problem.

@purcell
Copy link
Collaborator Author

purcell commented Jan 26, 2020

I've updated the README to recommend use of elm-language-server with Elm 0.19+.

At a certain point we should probably just drop Elm 0.18 support (at least w.r.t. completion and eldoc) and delete all of the code relating to elm-oracle.

@purcell
Copy link
Collaborator Author

purcell commented Feb 22, 2021

Closing this because we've just ripped out elm-oracle support, and we don't have any plans to add a non-LSP completion mechanism back into lsp-mode at this stage.

@purcell purcell closed this as completed Feb 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants