tonini / alchemist.el

Elixir Tooling Integration Into Emacs
http://www.alchemist-elixir.org
906 stars 103 forks source link

[Abandoned - use elixir-ls] Switching to elixir-ls as back-end #341

Open Trevoke opened 6 years ago

Trevoke commented 6 years ago

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 is a server that uses Microsoft's Language Server Protocol. It uses elixir_sense in the back-end.

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

There is an emacs package called 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)

This PR supersedes #318 and #339.

Code formatting is provided by M-x alchemist-format-buffer.

cs-victor-nascimento commented 6 years ago

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 commented 6 years ago

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 ( https://github.com/emacs-lsp/lsp-mode/commit/afa8d4a099a243b1c17d9d3b5056aeddaaca5588 ).

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:

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 commented 6 years ago

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

Trevoke commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

Hey!

Thanks for checking in. I think it's testable, and I think it's completely fine if you adjust the path to something that works for you temporarily. The server is provided in the PR so that should be a non-issue.

One thing I need is for someone who has used the alchemist features before to go through these things and make sure it's as close to parity as possible, or to make a list of how the changes may not match the existing behavior.

Finally, I will need someone who is familiar with alchemist's code eval to help determine what is needed in Elixir-ls and elixir_sense. I think that is the last big bastion of undone work.

On Thu, May 3, 2018, 14:28 Victor Oliveira Nascimento < notifications@github.com> wrote:

Hey @Trevoke https://github.com/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!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tonini/alchemist.el/pull/341#issuecomment-386392573, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEJSURHqRAy9ThtUXQ54dFTy7_F9qUpks5tu0xZgaJpZM4ShAI7 .

--

Phoned this one in.

Trevoke commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

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

Trevoke commented 6 years ago

@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 commented 6 years ago

@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 commented 6 years ago

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:

Trevoke commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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

bram209 commented 5 years ago

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

Trevoke commented 5 years ago

Hi there! There's another smaller pull request in progress, and there's a

alchemist-dev channel that has started.

Work is continuing, albeit slowly. I don't have the energy to work on it when I get home lately. Maybe I need to change my diet. :)

--

Aldric. Sent from a mobile device.

On Fri, Oct 26, 2018, 08:45 Bram <notifications@github.com wrote:

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

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tonini/alchemist.el/pull/341#issuecomment-433395136, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEJST22YUtVhH624Pk6Jg01yduRyxs4ks5uowPSgaJpZM4ShAI7 .

kpanic commented 5 years ago

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

dgutov commented 5 years ago

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?

gdrte commented 4 years ago

@Trevoke Can I hope this PR is not abandoned 🙏. Thank you for all the great work.

Trevoke commented 4 years ago

@kpanic the community solidifed in the Elixir Slack, channel #emacs or #language-server :)

@dgutov elixir_sense is the project analysis tool, and elixir-ls uses it to be a proper server based on the LSP protocol

@gdrte we have transitioned to the https://github.com/elixir-lsp project, instead of trying to retrofit alchemist into this project, because there was a complex interplay.

[ keeping this pull request open since it'll be more visible this way ]