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

[WIP] Switching to elixir-ls as back-end (ready for beta testing) #341

Open
wants to merge 31 commits into
base: master
from

Conversation

@Trevoke
Copy link
Contributor

Trevoke commented Mar 7, 2018

Beta testing [ instructions are also beta ]

This PR is now in a usable state (though you can see below not all features are really there yet), and could use some beta testers. If you want to beta test, awesome! Thank you!

Things you'll need to do:

  1. Send me a message letting me know you want to beta test - I'm easily findable on the Elixir Slack, or on Gmail, and sometimes on freenode on the Elixir channel.
  2. Clone the source branch ( https://github.com/trevoke/alchemist.el/tree/use-elixir-ls )
  3. Make sure you have installed the following packages: lsp-mode, lsp-ui, company-lsp, company, company-quickhelp
  4. Make sure the alchemist-elixir-ls, alchemist-goto, and the minor mode definition are all evaluated in your emacs, so that the new functionality is loaded.
  • first change your emacs config so it does not require/hook in the pre-existing alchemist
  • restart emacs
  • M-x eshell
  • (add-to-list 'load-path [string for the path where you cloned the project])
  • (require 'alchemist-elixir-ls)
  • (require 'alchemist-goto)
  • (require 'alchemist)
5. Try it out and let me know how things are!

## Description of work

[elixir-ls](https://github.com/elixir-lsp/elixir-ls) is a server that uses Microsoft's [Language Server Protocol](https://github.com/Microsoft/language-server-protocol). It uses [elixir_sense](https://github.com/elixir-lsp/elixir_sense) in the back-end.

Taken from its readme, here are the features that it supports:

* Debugger support (requires Erlang >= OTP 19)
* Automatic, incremental Dialyzer analysis (requires Erlang OTP 20)
* Inline reporting of build warnings and errors (requires Elixir >= 1.6)
* Documentation lookup on hover
* Go-to-definition
* Code completion
* Code formatter (requries Elixir >= 1.6)
* Find references to functions and modules 
* Quick symbol lookup in file 

There is an emacs package called [`lsp-mode`](https://github.com/emacs-lsp/lsp-mode) which generates minor modes for you with almost no effort. This branch leverages this.

## TODO tracker

My fork is keeping track of the undone work -- the checklist underneath is here for historical purposes.

https://github.com/Trevoke/alchemist.el/projects/1

### Things to do before this is ready to be merged (this list is not necessarily exhaustive)

- [x] alchemist.el depends on `lsp-mode`, `lsp-ui`, and `company-lsp`
- [X] allow user to choose between a server compiled with Erlang 19 and Erlang 20 (project-level)
- [X] allow user to choose Windows / UNIX (`.bat` or `.sh`) (emacs-level)
- [x] use alchemist-provided Elixir-LS server
- [x] Alchemist uses Elixir-LS for documentation lookup
- [x] Alchemist uses Elixir-LS for company's completion candidate documentation lookup
- [x] Alchemist's `C-h` / `\` shortcut for completion documentation pop-up still works (depends on https://github.com/tigersoldier/company-lsp/pull/34 - merged in: https://github.com/tigersoldier/company-lsp/pull/34#event-1600693482)
- ~~Alchemist's `C-w` shortcut for completion candidate jump-to-def still works (Need to attempt workaround: send buffer to LSP server as though the word had been completed and ask for a jump-to-definition from there)~~ [ Would like to drop this, much headache ]
- [x] Alchemist uses Elixir-LS for go-to-definition
- [x] Alchemist uses Elixir-LS for code completion
- [x] Alchemist uses Elixir-LS for code formatting
- [x] Alchemist uses Elixir-LS for references to functions and modules ("find usage")
- [x] Alchemist uses Elixir-LS for documentation lookup
- ~~Alchemist uses Elixir-LS for symbol lookup in file~~ ( Works just fine as-is )
  - [x] Use imenu: https://github.com/emacs-lsp/lsp-mode#imenu
- [ ] Alchemist uses Elixir-LS for code inline evaluation
- ~~Alchemist uses Elixir-LS for mix tasks~~ (not necessary unless we want to leverage elixir-ls' compilation time - it's a nice to have)
- [x] Alchemist still has its hook to optionally compile / run tests on save
- [x] Alchemist supports macro expansions through Elixir-LS
- [ ] Alchemist has same buffer-pop-up behavior as before with macro expansions
- [ ] phoenix functions still work, albeit in their current limited way
- [ ] tests pass :D
- [ ] melpa is configured to pull all the right code

This PR supersedes #318 and #339.


Code formatting is provided by `M-x alchemist-format-buffer`.
@cs-victor-nascimento

This comment has been minimized.

Copy link

cs-victor-nascimento commented Mar 25, 2018

Hey @Trevoke! Sorry to use this PR as a way to follow your progress... How is it going though? Your last commit got me curious about the state it is now (since there is nothing after "removing alchemist-server completely to see what needs to be done").

Is it usable? I can test drive-it if needed. Again, sorry for using this PR thread for commenting something other than the source code.

@Trevoke

This comment has been minimized.

Copy link
Contributor Author

Trevoke commented Mar 25, 2018

Hi there! No worries. I started a new job two weeks ago and I've had to put this on hold. I just sat down about twenty minutes ago to catch up to it and see if I could make any progress. You're right, I need to update this with an actual list of tasks.

The current state is as follows:

There are four moving pieces. Alchemist is our plugin, it leverages lsp-mode, which generates a minor mode that can talk to a LSP server. This LSP server is Elixir-LS, which uses elixir_sense under the hood.

I know that alchemist-server is central to many, many tasks in alchemist. I'm ripping it out to see exactly what it needs and what I can directly replace with. The current process for this is fairly raw: I deleted alchemist-server and I'm seeing what errors out when I try to load it. :D

Beyond the standard LSP tasks, LSP supports ways to send custom messages. I just got the lsp-mode maintainer to expose an API to send messages to the server ( emacs-lsp/lsp-mode@afa8d4a ).

The first thing I'm looking at through this process is macro expansion. elixir_sense supports it, so I need to send a custom message to the server - this likely means I'll need to fork elixir-ls to add support for that request (I plan on opening a pull request after the fact, of course).

I would love to have some help! To get started, all you should need to do is make sure that in alchemist-elixir-ls.el there's a reasonable path to the elixir-ls server.

What really sucks is that I haven't really found a friendly way to load alchemist.el in its entirety, so I just do it manually.

You will need to install the following plugins:

  • lsp-mode
  • company
  • company-lsp
  • lsp-ui

All help is welcome, I can think of the following four tasks in order of easy (read documentation) to complex (figure it out):

  1. Provide sane defaults for company / company-lsp / lsp-ui so that it doesn't just throw a ton of documentation and completion help at you. Bonus, provide alchemist-level configuration for these.
  2. Create the alchemist-level configuration for whether we're running on Windows or Unix (whether the server extension is .bat or `.sh).
  3. Create project-level configuration in alchemist so that when launching alchemist-mode, it'll either ask with of the Erlang 20 or Erlang 19 server to start, or find that knowledge in the configuration and start the correct server.
  4. Find something else that depends on alchemist-server and see how easily it can be replaced through LSP.

How much of this makes sense? Is that enough to get started? I'm Trevoke everywhere, including on the Elixir Slack and on the Elixir IRC (though I'm around on the Elixir Slack more often)

@Trevoke

This comment has been minimized.

Copy link
Contributor Author

Trevoke commented Mar 25, 2018

@cs-victor-nascimento Oh - and I just joined the alchemist.el Gitter room as well.

@Trevoke

This comment has been minimized.

Copy link
Contributor Author

Trevoke commented Apr 1, 2018

support for macro expansions is added. Since this is through elixir_sense, it supports all macros, not just core Elixir macros. Basically, if you assume that the [] tell you what is selected:

defmodule MyModule do
  import MacroModule

  [this_is_a_macro()]
end

Call (alchemist-macro-expand) and you get

defmodule MyModule do
  import MacroModule
  # macro code here
  # macro code here
  # as many lines as needed
  [this_is_a_macro()]
end
@Trevoke

This comment has been minimized.

Copy link
Contributor Author

Trevoke commented Apr 22, 2018

Support for Company's C-h to pop-up documentation when quickhelp is not enabled is working .. Pending the PR being merged in company-lsp.

@victorolinasc

This comment has been minimized.

Copy link

victorolinasc commented May 3, 2018

Hey @Trevoke! I was looking into your branch just to see if I would be able to test this. Perhaps, this is not the time for testing yet. From what I could tell, the path to elixir-ls is still hard-coded. I could simply change it and have a fixed elixir-ls version on my system too, but I'll wait a bit more instead.

I guess you'll tackle this issue when you reach the "decide whether nix/ Windows system you are running", right?

Anyway, thanks once again for your work!

@Trevoke

This comment has been minimized.

Copy link
Contributor Author

Trevoke commented May 3, 2018

@Trevoke Trevoke changed the title [WIP] Switching to elixir-ls as back-end [WIP] Switching to elixir-ls as back-end (ready for beta testing) Jun 11, 2018
@Trevoke

This comment has been minimized.

Copy link
Contributor Author

Trevoke commented Jun 11, 2018

@cs-victor-nascimento Hey there! this PR is now ready for beta testing. It's missing some functionality but what I consider to be the fundamentally needed behavior is present.

@dsdshcym

This comment has been minimized.

Copy link

dsdshcym commented Jun 11, 2018

alchemist-mix is not working anymore in this branch. (because it was removed)

the error message says alchemist.elc failed to define function alchemist-mix-test .... etc.

@victorolinasc

This comment has been minimized.

Copy link

victorolinasc commented Jun 11, 2018

Thanks @Trevoke for the awesome work! I will try to setup it sometime this week.

@Trevoke

This comment has been minimized.

Copy link
Contributor Author

Trevoke commented Jun 11, 2018

@dsdshcym what do you use alchemist-mix for? (Just by asking the question, I realize that it's the entry point to, well, any mix task, so it's convenient to have it).

The reason I completely removed it was because the code for it is heavily dependent on the alchemist server and I didn't want to deal with that -- but since elixir_sense, the code analysis server, uses some version of the alchemist server behind the scenes, I suspect this can be adjusted. It'll take some additional work.

@dsdshcym

This comment has been minimized.

Copy link

dsdshcym commented Jun 12, 2018

@Trevoke I use it mostly for running tests, sometimes for running mix phx.server. Since I heavily depend on this feature to do TDD, this beta version is completely unusable to me.

I think for running tests, we can have something like rspec-mode, which does not depend on a language server and just runs test commands in the root directory.

BTW, sometimes I feel alchemist is a bloated package, as it's trying to do so many things, like phoenix project navigations, running mix tasks, etc. What's your thought on split these features out into there own packages (like phoenix-mode, mix-mode, etc.)?

@dsdshcym

This comment has been minimized.

Copy link

dsdshcym commented Jun 13, 2018

also, the company auto completion also doesn't work well, it will also complete the function parameters instead of just complete the function name

For example:

  • expected completion result: insert|
  • current behavior: insert(struct_or_changeset, opts)|
@Trevoke

This comment has been minimized.

Copy link
Contributor Author

Trevoke commented Jul 13, 2018

FYI - I'm starting to track issues and such on my fork, because a single PR is just not enough to handle all the conversations : https://github.com/Trevoke/alchemist.el/projects/1

@axelson

This comment has been minimized.

Copy link

axelson commented Jul 25, 2018

@Trevoke does the description of this PR still have the most up-to-date instructions for trying out your changes? (if so then consider this me messaging you).

@Trevoke

This comment has been minimized.

Copy link
Contributor Author

Trevoke commented Jul 28, 2018

@axelson Hi there! Yes, these instructions are still the way to try out the changes. Thanks for your help :)

Trevoke added 8 commits Jul 30, 2018
set extension for elixir-ls based on OS
@bram209

This comment has been minimized.

Copy link

bram209 commented Oct 26, 2018

@Trevoke any updates on the state of this? Would love to use alchemist together with elixir ls :)

@Trevoke

This comment has been minimized.

Copy link
Contributor Author

Trevoke commented Oct 29, 2018

@kpanic

This comment has been minimized.

Copy link

kpanic commented Dec 7, 2018

Dear @Trevoke I joined the gitter room. Not sure if I can help, (no emacs lisp knowledge) but let's try maybe!

@dgutov

This comment has been minimized.

Copy link

dgutov commented Apr 27, 2019

If elixir_sense is based on alchemist-server, and elixir-lsp is based on it, wasn't it more natural to switch to using elixir_sense instead? Or did they change the protocol a lot?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants
You can’t perform that action at this time.