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

Refactor folding_ranges onto POIs, Add support for records. #1268

Merged
merged 1 commit into from
Apr 21, 2022

Conversation

tks2103
Copy link
Contributor

@tks2103 tks2103 commented Apr 14, 2022

Moving folding_range from functions to a general field on each poi. Added support for records. Refactored support for functions. Added foldingrange_provider test

@robertoaloi
Copy link
Member

robertoaloi commented Apr 19, 2022

Thanks a lot for this @tks2103 ! Great generalization!

CI is failing, but I believe it's because many tests depend on the content of the code_navigation test module. We could either adjust the expectations or have a specific test file for folding ranges and point to that in the test suite.

Other than that, looks great to me.

@tks2103
Copy link
Contributor Author

tks2103 commented Apr 19, 2022

@robertoaloi is this passing CI now? Can't figure out if it ran another set of tests...

I don't have commit access, feel free to merge if it's good.

@robertoaloi
Copy link
Member

@robertoaloi is this passing CI now? Can't figure out if it ran another set of tests...

Since this is your first contribution to the project, GitHub requires me to manually approve the CI run. Triggered it now, so let's see. You can also run this manually on your machine, following the steps at:

https://github.com/erlang-ls/erlang_ls/blob/main/.github/workflows/build.yml

So, essentially, you can:

rebar3 do compile, ct, lint, proper, dialyzer, xref, edoc

@robertoaloi robertoaloi merged commit 0f6962e into erlang-ls:main Apr 21, 2022
@robertoaloi
Copy link
Member

Thanks!

@robertoaloi robertoaloi mentioned this pull request Apr 21, 2022
@robertoaloi
Copy link
Member

And credit should also go to @AminArria who provided the original implementation in #1266.

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.

2 participants