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 the debugger #143

Merged
merged 2 commits into from
Feb 29, 2020
Merged

Fix the debugger #143

merged 2 commits into from
Feb 29, 2020

Conversation

axelson
Copy link
Member

@axelson axelson commented Feb 28, 2020

The debugger was inadvertently broken in #93 which the change from the deprecated ([2]) Code.load_file/1 to Code.require_file/1. The compatibility docs for 1.10 [1] list both Code.require_file/1 and Code.compile_file/1 as possible replacements for Code.load_file/1. In our case we need Code.compile_file/1 instead of Code.require_file/1 since Code.require_file/1 tracks which files have been required and compiled and does not compile them more than once. Specifically the docs state:

If require_file/2 is called more than once with a given file, that file will be compiled only once

So since Code.load_file/1 would always compile the file we need to use Code.compile_file/1 rather than Code.require_file/1. When Code.require_file/1 was used, the call in
ElixirLS.Debugger.Server.change_env/1 would have no effect because the file was previously required. By changing to Code.compile_file/1 the project will always be reloaded by compiling it's mix.exs file.

[1] https://hexdocs.pm/elixir/1.10.2/compatibility-and-deprecations.html#table-of-deprecations
[2] elixir-lang/elixir#7201

Fixes #131

The debugger was inadvertantly broken in #93 which the change from the
deprecated ([2]) `Code.load_file/1` to `Code.require_file/1`. The compatability
docs for 1.10 [1] list both `Code.require_file/1` and `Code.compile_file/1` as
possible replacements for `Code.load_file/1`. In our case we need
`Code.compile_file/1` instead of `Code.require_file/1` since
`Code.require_file/1` tracks which files have been required and compiled and
does not compile them more than once. Specifically the docs state:

> If require_file/2 is called more than once with a given file, that file will
> be compiled only once

So since `Code.load_file/1` would always compile the file we need to use
`Code.compile_file/1` rather than `Code.require_file/1`. When
`Code.require_file/1` was used, the call in
`ElixirLS.Debugger.Server.change_env/1` would have no effect because the file
was previously required. By changing to `Code.compile_file/1` the project will
always be reloaded by compiling it's `mix.exs` file.

[1] https://hexdocs.pm/elixir/1.10.2/compatibility-and-deprecations.html#table-of-deprecations
[2] elixir-lang/elixir#7201
@lukaszsamson
Copy link
Collaborator

Nice find. It was quite hard to decide between those two as it was not clear what the original intention was.

@axelson axelson merged commit 85d5ceb into elixir-lsp:master Feb 29, 2020
@axelson axelson deleted the fix-debugger branch February 29, 2020 18:22
axelson added a commit that referenced this pull request Feb 29, 2020
@axelson
Copy link
Member Author

axelson commented Feb 29, 2020

Good point, added a couple code comments to explain the reasoning in-line.

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

Successfully merging this pull request may close these issues.

Unable to launch test task from VS Code
2 participants