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

add support for disabling automatic builds though a config #440

Merged
merged 1 commit into from
Oct 6, 2022

Conversation

Hanspagh
Copy link
Contributor

closes #341

Adds a config check before starting a build in trigger_build.
I was hesitant to add it to build_enabled since it also seems to be used by dialyzer_enabled

I tried to follow what was outlined in this comment

First PR on a language server, so any feedback is very welcome.

@axelson
Copy link
Member

axelson commented Dec 22, 2020

Hmm, I don't think this approach will work very well since the code would never get rebuilt which means that your auto completions and go to implementation, etc. will be out of date. And if you wanted to build you'd have to go in your settings to enable autoBuild, trigger a build, and then go back to settings to disable it.

Here's some potential approaches:

  1. Add a custom command to build (will have to add support to all clients)
  2. When this setting is enabled then use the projects _build instead of .elixir_ls/build
  • Additionally we'll have to add support to load updated code from mix compile, possibly with a file watcher, or possibly do a manually check when we would have triggered a build (although there would likely be race conditions)
  1. Add some other method to build only when the user wants to build

@lukaszsamson any other ideas or thoughts on implementing this feature?

@Hanspagh
Copy link
Contributor Author

My understanding was that the config would start (My understanding was that people wanted to disable builds entirely). You bring up some good points with the above, I will see if I can tackle them, I am still new to the code base, so feel free to add any pointers.

@lukaszsamson
Copy link
Collaborator

@axelson I think you covered all of the aspects in your comment.

Add a custom command to build (will have to add support to all clients)

I like that idea

When this setting is enabled then use the projects _build instead of .elixir_ls/build

It would be risky IMO. Every elixir release changes something in the build system. IIRC There were issues with race conditions/locks between mix and elixirLS in the past.

Add some other method to build only when the user wants to build

File watcher on mix.exs?. Besides that I can't think of anything that doesn't involve client code changes or using external tools.

@Hanspagh Hanspagh changed the title add support for disabling automatic builds trough a config add support for disabling automatic builds though a config Feb 3, 2021
@paulstatezny
Copy link

paulstatezny commented Nov 22, 2021

@Hanspagh Thank you for this PR. Even though it wasn't merged, I'm using your branch to make my editor setup take way less CPU. ❤️

Disables a couple of features but super worth it in my opinion.

@lukaszsamson
Copy link
Collaborator

I think we can merge it and add rebuild command later. @Hanspagh can you resolve merge conflict?

@DaliborHorinek
Copy link
Contributor

@Hanspagh Hello, thank you for your PR, do you think it would be possible to update the branch, so it can be merged?

@abdul-hamid-achik
Copy link

should we merge?

@Hanspagh
Copy link
Contributor Author

Hanspagh commented May 2, 2022

The merge conflicts is solved now atleast, then you can decide what wanna do with this :)

@lukaszsamson lukaszsamson merged commit 9160db5 into elixir-lsp:master Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants