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.

Sign up for GitHub

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

Jump to bottom

refactor selection range plugin #3003

Merged
merged 24 commits into from
Jul 11, 2022
Merged

refactor selection range plugin #3003

merged 24 commits into from
Jul 11, 2022

Conversation

kokobd
Copy link
Collaborator

@kokobd kokobd commented Jun 30, 2022 •
edited
Loading

This PR should close #2996 and #3001

In brief, hls-selection-range-plugin is changed to hls-code-range-plugin, and the code is rewritten so that adding
"folding range" to this plugin becomes relatively easy.

@sloorush please check if the refactor satisfies your needs.

@kokobd kokobd requested a review from pepeiborra as a code owner June 30, 2022 10:25
@kokobd kokobd marked this pull request as draft June 30, 2022 10:28
michaelpj
michaelpj reviewed Jun 30, 2022
View reviewed changes
plugins/hls-selection-range-plugin/src/Ide/Plugin/SelectionRange.hs Show resolved Hide resolved
plugins/hls-selection-range-plugin/src/Ide/Plugin/SelectionRange.hs Outdated Show resolved Hide resolved
plugins/hls-selection-range-plugin/src/Ide/Plugin/SelectionRange.hs Outdated Show resolved Hide resolved
plugins/hls-selection-range-plugin/src/Ide/Plugin/SelectionRange/CodeRange.hs Outdated Show resolved Hide resolved
plugins/hls-selection-range-plugin/src/Ide/Plugin/SelectionRange/CodeRange.hs Outdated Show resolved Hide resolved
plugins/hls-selection-range-plugin/src/Ide/Plugin/SelectionRange/CodeRange.hs Outdated Show resolved Hide resolved
plugins/hls-selection-range-plugin/src/Ide/Plugin/SelectionRange/CodeRange.hs Outdated Show resolved Hide resolved
plugins/hls-selection-range-plugin/src/Ide/Plugin/SelectionRange.hs Outdated Show resolved Hide resolved
plugins/hls-selection-range-plugin/src/Ide/Plugin/SelectionRange.hs Outdated Show resolved Hide resolved
plugins/hls-selection-range-plugin/src/Ide/Plugin/SelectionRange.hs Outdated Show resolved Hide resolved
@michaelpj
Copy link
Collaborator

cc @sloorush

@michaelpj
Copy link
Collaborator

Oops, I started looking at this before I saw it was a draft, apologies if I made lots of comments that you were going to fix anyway 😅

@kokobd
Copy link
Collaborator Author

kokobd commented Jun 30, 2022

Oops, I started looking at this before I saw it was a draft, apologies if I made lots of comments that you were going to fix anyway 😅

Thank you for your valuable suggestions! I'm a bit hurry, to make sure not to block sloorush for too long.

sloorush
sloorush reviewed Jun 30, 2022
View reviewed changes
Copy link
Contributor

@sloorush sloorush left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also rename the plugin to hls-range-plugin (as you suggested) in this PR?

@kokobd
Copy link
Collaborator Author

kokobd commented Jul 1, 2022

@sloorush As we actually have more time than I initially expected, I will include that in this PR, and polish the code

@michaelpj
Copy link
Collaborator

Should we also rename the plugin to hls-range-plugin (as you suggested) in this PR?

Worth pointing out that we do publish the plugin package to Hackage. I think that's fine, though.

@kokobd
Copy link
Collaborator Author

kokobd commented Jul 1, 2022

Seems that this PR is also affected by #2997

@kokobd
Copy link
Collaborator Author

kokobd commented Jul 5, 2022

Maybe this is ready for a review, while I'm adding unit tests to individual functions.

@kokobd kokobd marked this pull request as ready for review July 5, 2022 03:15
@kokobd kokobd requested review from Ailrun and berberman as code owners July 5, 2022 03:15
@kokobd
Copy link
Collaborator Author

kokobd commented Jul 11, 2022

I'm feeling good about this, merging.

@kokobd kokobd added the merge me Label to trigger pull request merge label Jul 11, 2022
@mergify mergify bot merged commit 445192e into master Jul 11, 2022
@kokobd kokobd mentioned this pull request Jul 13, 2022
Closed
@hololeap hololeap mentioned this pull request Sep 2, 2022
Open
hololeap added a commit to hololeap/gentoo-haskell that referenced this pull request Sep 10, 2022
@hololeap
profiles: Mask hls-selection-range-plugin
beb966d 
hls-selection-range-plugin has been renamed to hls-code-range-plugin upstream

Update profiles/updates/3Q-2022

See: haskell/haskell-language-server#3003
Signed-off-by: hololeap <[email protected]>
hololeap added a commit to gentoo-haskell/gentoo-haskell that referenced this pull request Sep 11, 2022
@hololeap
profiles: Mask hls-selection-range-plugin
b2335d6 
hls-selection-range-plugin has been renamed to hls-code-range-plugin upstream

Update profiles/updates/3Q-2022

See: haskell/haskell-language-server#3003
Signed-off-by: hololeap <[email protected]>
@kokobd kokobd deleted the kokobd/refactor-selection-range-2996 branch July 23, 2023 00:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sloorush sloorush sloorush left review comments

@michaelpj michaelpj michaelpj approved these changes

@pepeiborra pepeiborra Awaiting requested review from pepeiborra

@jneira jneira Awaiting requested review from jneira

@Anton-Latukha Anton-Latukha Awaiting requested review from Anton-Latukha

@Ailrun Ailrun Awaiting requested review from Ailrun

@berberman berberman Awaiting requested review from berberman

Assignees
No one assigned
Labels
merge me Label to trigger pull request merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Refactor selection range plugin
3 participants
@kokobd @michaelpj @sloorush