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

Add interactive command for case splitting #112

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

TOTBWF
Copy link

@TOTBWF TOTBWF commented Feb 27, 2021

Description

This PR adds an interactive function for performing case splits. To use it, simply place your cursor over a typed hole, and call lsp-haskell-case-split. You should be prompted for a list of identifiers, and selecting one from that list will case split on it.

Notes

It would be nice if lsp-mode provided a version of lsp--select-action that offered the ability to have a custom prompt/transformation function. If such a function is written, this code should be updated to use it.

@TOTBWF
Copy link
Author

TOTBWF commented Feb 27, 2021

Build failures are unrelated to this change. Looks like something is up with the spinner package.

@michaelpj
Copy link
Collaborator

Dumb question, since I haven't used any of this yet, but what does this offer over just using the normal methods to trigger the code action? Is it that this is a particular combination that you want to trigger a lot, and so it's nice to have a specific function or keybinding for it?

"Case split on an identifier.
To use this, place make sure the point is over a type hole."
(interactive)
(let* ((case-split-regex "Case split on \\(.*\\)")
Copy link
Collaborator

Choose a reason for hiding this comment

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

This feels a little fragile. I think the right way to do this is to use code action kinds: they're explicitly there to allow filtering by the client (i.e. us). We might need to teach the tactics plugin to add a kind, though.

Copy link
Author

Choose a reason for hiding this comment

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

I can update the tactics plugin with kinds, good call!

(t (lsp--completing-read "Identifier: "
(seq-into actions 'list)
(lambda (action)
(cadr (s-match case-split-regex (lsp:code-action-title action))))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, you do rather rely on the regex here. I guess I don't have a better suggestion, then, since AFAICT there's no proper way to include some extra structured data in a CodeAction...

I guess we could avoid the regex by just using the code action title as is, for something like:

Pick a case-splitting action:
1. Case split on x
2. Case split on y

but I guess that's a worse UX.

Copy link
Author

Choose a reason for hiding this comment

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

If it is too fragile, then we can remove it! Shame that the spec doesn't let us communicate more in the kind.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, I just want to avoid the situation where someone changes the title, rightly reasoning that it's a user-facing string that can be changed freely, and then this function breaks.

@isovector
Copy link

FWIW, this is now implemented directly in HLS, under a code action with kind refactor.wingman.caseSplit

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