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

added: lsp-haskell-execute-code-action-add-signature #129

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

Conversation

ncaq
Copy link
Contributor

@ncaq ncaq commented Jun 25, 2021

I want it to be a stand-alone function because I often do this in Haskell code actions.

I update Package-Requires emacs 24.3 -> 26.1,
because seq-find require Emacs 25,
lsp-mode of base package require 26.1,
Since lsp-haskell will never work without lsp-mode,
there is no point in lsp-haskell supporting old Emacs that lsp-mode does not support.

I want it to be a stand-alone function because I often do this in Haskell code actions.

I update Package-Requires emacs `24.3` -> `26.1`,
because `seq-find` require Emacs 25,
lsp-mode of base package require `26.1`,
Since lsp-haskell will never work without lsp-mode,
there is no point in lsp-haskell supporting old Emacs that lsp-mode does not support.
lsp-haskell.el Outdated
"Execute code action of add signature.
Add the type signature that GHC infers to the function located below the point."
(interactive)
(let ((action (seq-find (lambda (e) (string-prefix-p "add signature" (gethash "title" e))) (lsp-code-actions-at-point))))
Copy link
Member

Choose a reason for hiding this comment

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

you should not use gethash because when lsp-use-plists is t the data structures will be plists. You may refer to lsp-execute-code-action-by-kind in lsp-java.

Copy link
Member

Choose a reason for hiding this comment

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

I meant lsp-java-execute-matching-action

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or we want the stuff in emacs-lsp/lsp-mode#2402

Copy link
Collaborator

Choose a reason for hiding this comment

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

HLS should set an identifiable kind for the action for this reason. If it doesn't please open a PR upstream so that it does. See also #112

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank.
I use lsp:code-action-title.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Code action titles aren't the same as kinds! Titles are user facing strings that can change at any time, kinds should be more stable.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Yes, lsp-java is using the title because the server is refusing to fix the kinds.

@@ -1,7 +1,7 @@
;;; lsp-haskell.el --- Haskell support for lsp-mode

;; Version: 1.0
;; Package-Requires: ((emacs "24.3") (lsp-mode "3.0") (haskell-mode "1.0"))
;; Package-Requires: ((emacs "26.1") (lsp-mode "3.0") (haskell-mode "1.0"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

ncaq added 2 commits June 26, 2021 14:24
I followed the code review.

> you should not use gethash because when lsp-use-plists is t the data structures will be plists. You may refer to lsp-execute-code-action-by-kind in lsp-java.
>
> <emacs-lsp#129 (comment)>
ncaq added a commit to ncaq/.emacs.d that referenced this pull request Jun 26, 2021
[added: lsp-haskell-execute-code-action-add-signature by ncaq · Pull Request #129 · emacs-lsp/lsp-haskell](emacs-lsp/lsp-haskell#129)
@Anton-Latukha
Copy link
Contributor

Seems everything in the thread was addressed. Love providing type signatures for local functions, this would help type programming immensely.

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.

4 participants