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

Use the default erlang format for erlang_ls.config #1083

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

sirikid
Copy link
Contributor

@sirikid sirikid commented Sep 13, 2021

Subj.

@robertoaloi
Copy link
Member

@maxno-kivra Any concerns with this?

@robertoaloi
Copy link
Member

Hi @sirikid and thanks for your contribution!

I am happy to support Erlang terms as a possible format (that would also make it easier to produce diagnostics for the configuration itself), but I believe we should keep this change backward compatible, at least for now. What do you think about trying to parse the config in both ways (Erlang, YAML) before giving up? Something like

  • erlang_ls.config -> Try both Erlang and YAML for now, only Erlang in the future
  • erlang_ls.yaml -> Read it as YAML (we may want to deprecate this later)

An option is to call it something like erlang_ls.eterm. What do you think?

@sirikid
Copy link
Contributor Author

sirikid commented Sep 18, 2021

I am happy to support Erlang terms as a possible format (that would also make it easier to produce diagnostics for the configuration itself), but I believe we should keep this change backward compatible, at least for now. What do you think about trying to parse the config in both ways (Erlang, YAML) before giving up? Something like

* erlang_ls.config -> Try both Erlang and YAML for now, only Erlang in the future

* erlang_ls.yaml -> Read it as YAML (we may want to deprecate this later)

Sure, we can also emit a warning if erlang_ls.config is written in YAML. I will update the PR.

An option is to call it something like erlang_ls.eterm. What do you think?

I would like to keep it .config because Erlang itself and widely adopted tools like Rebar3 use .config.

@maxno-kivra
Copy link
Contributor

maxno-kivra commented Sep 20, 2021

I was thinking about breaking existing configs. As long as the users doesn't notice this change, why not. But if it means I'm forced to update all my config files I wouldn't be very happy.

I also would like to keep support for YAML. Partly the same reason as above, don't force the user to make an arbitrary change. But also because many other tools use YAML and I like the consistency with those. (Although YAML as a format is not great, I give you that.)

@sirikid
Copy link
Contributor Author

sirikid commented Mar 12, 2022

@robertoaloi ping

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.

3 participants