-
-
Notifications
You must be signed in to change notification settings - Fork 445
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
WIP: Add customization point to retrieve next-checkers #1836
base: master
Are you sure you want to change the base?
WIP: Add customization point to retrieve next-checkers #1836
Conversation
This is an interesting approach. Can you give a couple of examples of how one would use this to say, chain other checkers to lsp-mode's checker? I imagine python, JS and go devs will run into this issue the most. |
@wyuenho you may wanna check - #1762 (comment) |
Example with (defun my-flycheck-lsp-next-checkers (checker)
(pcase major-mode
('sh-mode
(pcase checker
('lsp '(sh-shellcheck sh-bash))
('sh-shellcheck '(sh-bash))
))
('web-mode '(javascript-eslint))
)
)
(defcustom my-lsp-flycheck-next-checkers-function nil
"Variable to hold the function that returns flycheck next checkers for lsp."
:type '(choice function (const nil)))
(make-variable-buffer-local 'my-lsp-flycheck-next-checkers-function)
;; in my case I only use this function for lsp checker
(defun my-lsp-flycheck-next-checkers (checker)
"Function called by flycheck to find next checkers for CHECKER."
(when (functionp my-lsp-flycheck-next-checkers-function)
(funcall my-lsp-flycheck-next-checkers-function checker)))
(flycheck-set-next-checkers-function 'lsp #'my-lsp-flycheck-next-checkers)
(add-hook 'lsp-after-initialize-hook
(lambda()
(setq my-lsp-flycheck-next-checkers-function #'my-flycheck-lsp-next-checkers))) |
0d250c9
to
67ce045
Compare
With the previous code to have With the new code, you should be able to do : (defun my-flycheck-lsp-next-checkers (checker)
(pcase major-mode
('sh-mode
(pcase checker
('lsp '(sh-shellcheck sh-bash))
('sh-shellcheck '(sh-bash))
))
('web-mode '(javascript-eslint))
)
)
(add-hook 'lsp-after-initialize-hook
(lambda()
(setq flycheck-next-checkers-function #'my-flycheck-lsp-next-checkers))) Anyway, I do not think this PR will land and you can use an advice as @yyoncho mentioned. |
Why won't this land? Buffer local checker chain sounds like a great idea to me. |
Because none of the maintainers have found the time to review the code, unfortunately :/ I'm on the job market atm and finishing a PhD, so not much time for free software. Additionally, what this PR does can already be achieved with advice; yyoncho posted sample code for that in his previous comment. With that and the buffer-local obarray options, there's a family of possible design choices that need to be evaluated. @damienrg , thanks a lot for the PR. Have you had a look at the advice-based solution? Is there a reason to prefer this patch over that? Your example use case would work the same, just with |
I have opened this PR to have feedback about the solution I propose to chain checkers and it is maybe not the right way to do it.
Yes it works the same way. The main difference is that advice-based solution means it is not an official feature of From a user point of view, I would like to be able to do either:
I do not believe that changing properties of a checker is the way to go because the user has to change the properties on all the next checkers or |
This PR introduce a customization point to retrieve next checkers.
In 0d250c9, the customization point is for a given checker based on new
next-checkers-function
property.In 67ce045, the customization point is
flycheck-next-checkers-function
that is buffer local.Related to:
#1660
#1762
emacs-lsp/lsp-mode#2329