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

erlang_ls should be more explicit when it finds an erlang_ls.config that isn't valid #1401

Closed
ollien opened this issue Nov 7, 2022 · 1 comment · Fixed by #1406
Closed

Comments

@ollien
Copy link

ollien commented Nov 7, 2022

Is your feature request related to a problem? Please describe.

Right now, if erlang_ls.config is not valid, erlang_ls will log it and move on, but it is not always abundantly clear that this has happened. For instance

[2022-11-07T11:22:18.612874-05:00] [debug] Could not read config file: path="/home/nick/Documents/code/my_project/erlang_ls.config" class=throw error={yamerl_exception,[{yamerl_parsing_error,error,"Unexpected \"yamerl_scalar\" token following a \"yamerl_scalar\" token",2,1,unexpected_token,{yamerl_scalar,2,1,{yamerl_tag,2,1,{non_specific,"?"}},flow,plain,"plt_path \"/home/nick/Documents/code/my_project/_build/default/rebar3_21.3.8.4_plt\""},[]}]} [els_config:consult_config/1 L350] <0.148.0>

On my system, where I have no global erlang_ls.config, this manifests itself in VSCode with an alert saying there's no erlang_ls.config file.

Describe the solution you'd like
I think there's a few ways this could be tackled

  1. At minimum, the above log could be bumped up from a DEBUG to a WARN or ERROR.
  2. Editors like VSCode could give a more explicit error that a config file was invalid, much like there is when there is no config file.
  3. erlang_ls should maybe stop execution if this happens. I can think of very few cases where someone might want to continue operation after a configuration is determined to be invalid; falling back to the next available config ironically masks the problem, since it might not be obvious that this occurred.

Describe alternatives you've considered
See previous section

Additional context
This thread in the Erlang slack is where this originally came up

@fabjan
Copy link
Contributor

fabjan commented Nov 8, 2022

Agree on all points.

I think an error level log with details, an error notice (the name of the concept in the lsp eludes me) should be shown, and the language server should exit with an error.

fabjan added a commit to fabjan/erlang_ls that referenced this issue Nov 14, 2022
fabjan added a commit to fabjan/erlang_ls that referenced this issue Feb 11, 2023
robertoaloi pushed a commit that referenced this issue Feb 15, 2023
* Present configuration read errors for the user

Fixes  #1401

* Clarify error_reporting flag in els_config

* Stop trying to merge config maps that are not maps
zengjinc pushed a commit to zengjinc/erlang_ls that referenced this issue Jun 12, 2023
…ng-ls#1406)

* Present configuration read errors for the user

Fixes  erlang-ls#1401

* Clarify error_reporting flag in els_config

* Stop trying to merge config maps that are not maps
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 a pull request may close this issue.

2 participants