erlang / sourcer

An Erlang language server, providing IDE services.
Apache License 2.0
122 stars 19 forks source link

Add config file support, refactorings #12

Closed deadtrickster closed 6 years ago

deadtrickster commented 6 years ago

Hi,

also features:

Today I spent some time integrating erlang_ls -i with my emacs: https://github.com/deadtrickster/erlang-format.

How to ask indent to remove traling whitespace?

deadtrickster commented 6 years ago

Oh, there is no tests. for the new features. Please let me know what's the best way to test config file.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-0.3%) to 63.461% when pulling 9fa758edb6470e2472c1ad531cdb540c06be38bb on deadtrickster:indent/config into f84090f127194cbee464dd105eb5e670e1509ed5 on erlang:master.

vladdu commented 6 years ago

This looks good, thank you Ilya! There are a few minor points:

I don't think there is support for removing the trailing whitespace. Usually, editors have that kind of functionality in a generic way, but I suppose it wouldn't hurt to do it ourselves anyway. Please open a ticket for that.

deadtrickster commented 6 years ago

Did you try any other value to see if the result looks alright?

yep. I use indentW=2 in my own projects, the only thing I noticed (so far) is difference for 'before_arrow'.

But, since config added that can override all paramters I would live with what we have and expect people to patch this naturally. I assume for indentW=4 it behaves as erlang-mode anyway.

vladdu commented 6 years ago

It may be that 'before_arrow' should be 2, instead of IndentW div 2?

deadtrickster commented 6 years ago

dunno, now I think it makes sense as it visually distinguishes between function body and when

vladdu commented 6 years ago

Ok, no problem. Looks good now, if there are problems later we can change those. It's great that you did the emacs integration, we can get feedback from there too.

vladdu commented 6 years ago

Oh, yes, testing the config file... I guess only getPrefs should be tested, because maybe_load_config is just a wrapper around file:consult. We are interested in checking that changing IndentW results in changed indentation options (only checking a couple is enough).

deadtrickster commented 6 years ago

added a test

vladdu commented 6 years ago

Thank you very much!