From: Augusto Stoffel <arstoffel@gmail.com>
To: Philip Kaludercic <philipk@posteo.net>
Cc: Eli Zaretskii <eliz@gnu.org>,
57884@debbugs.gnu.org, Stefan Kangas <stefankangas@gmail.com>
Subject: bug#57884: [PATCH] Flymake backend using the shellcheck program
Date: Sun, 18 Sep 2022 14:38:34 +0200 [thread overview]
Message-ID: <87fsgoyi0l.fsf@gmail.com> (raw)
In-Reply-To: <87zgewdhhd.fsf@posteo.net> (Philip Kaludercic's message of "Sun, 18 Sep 2022 11:55:42 +0000")
[-- Attachment #1: Type: text/plain, Size: 193 bytes --]
With the attached patch I believe I've addressed all comments from this
and previous messages. (In case I missed some detail, the committer
should feel free to make any desired copyediting.)
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-New-Flymake-backend-using-the-shellcheck-program.patch --]
[-- Type: text/x-patch, Size: 5186 bytes --]
From 1cadf763114eb733036a1c8182900fe2df17092c Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Sat, 17 Sep 2022 18:30:04 +0200
Subject: [PATCH] New Flymake backend using the shellcheck program
See bug#57884.
* lisp/progmodes/sh-script.el (sh-shellcheck-program,
sh--shellcheck-process, sh-shellcheck-flymake): Variables and function
defining a Flymake backend.
(sh-mode): Add it to 'flymake-diagnostic-functions'.
---
etc/NEWS | 4 ++
lisp/progmodes/sh-script.el | 76 +++++++++++++++++++++++++++++++++++++
2 files changed, 80 insertions(+)
diff --git a/etc/NEWS b/etc/NEWS
index a6a8883593..5b2ed16063 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1320,6 +1320,10 @@ This controls how statements like the following are indented:
foo &&
bar
+*** New Flymake backend using the ShellCheck program
+It is enabled by default, but requires that the external "shellcheck"
+command is installed.
+
** Cperl Mode
---
diff --git a/lisp/progmodes/sh-script.el b/lisp/progmodes/sh-script.el
index 517fbbd8e7..dd36f92766 100644
--- a/lisp/progmodes/sh-script.el
+++ b/lisp/progmodes/sh-script.el
@@ -31,6 +31,9 @@
;; available for filenames, variables known from the script, the shell and
;; the environment as well as commands.
+;; A Flymake backend using the "shellcheck" program is provided. See
+;; https://www.shellcheck.net/ for installation instructions.
+
;;; Known Bugs:
;; - In Bourne the keyword `in' is not anchored to case, for, select ...
@@ -1580,6 +1583,7 @@ sh-mode
((equal (file-name-nondirectory buffer-file-name) ".profile") "sh")
(t sh-shell-file))
nil nil)
+ (add-hook 'flymake-diagnostic-functions #'sh-shellcheck-flymake nil t)
(add-hook 'hack-local-variables-hook
#'sh-after-hack-local-variables nil t))
@@ -3103,6 +3107,78 @@ sh-delete-backslash
(delete-region (1+ (point))
(progn (skip-chars-backward " \t") (point)))))))
+;;; Flymake backend
+
+(defcustom sh-shellcheck-program "shellcheck"
+ "Name of the shellcheck executable."
+ :type 'string
+ :version "29.1")
+
+(defcustom sh-shellcheck-arguments nil
+ "Additional arguments to the shellcheck program."
+ :type '(repeat string)
+ :version "29.1")
+
+(defvar-local sh--shellcheck-process nil)
+
+(defun sh-shellcheck-flymake (report-fn &rest _args)
+ "Flymake backend using the shellcheck program.
+Takes a Flymake callback REPORT-FN as argument, as expected of a
+member of `flymake-diagnostic-functions'."
+ (when (process-live-p sh--shellcheck-process)
+ (kill-process sh--shellcheck-process))
+ (let* ((source (current-buffer))
+ (dialect (named-let dialect ((s sh-shell))
+ (pcase s
+ ((or 'bash 'dash 'sh) (symbol-name s))
+ ('ksh88 "ksh")
+ ((guard s)
+ (dialect (alist-get s sh-ancestor-alist))))))
+ (pattern "^-:\\([0-9]+\\):\\([0-9]+\\): \\([^:]+\\): \\(.*\\)$")
+ (sentinel
+ (lambda (proc _event)
+ (when (memq (process-status proc) '(exit signal))
+ (unwind-protect
+ (if (with-current-buffer source
+ (not (eq proc sh--shellcheck-process)))
+ (flymake-log :warning "Canceling obsolete check %s" proc)
+ (with-current-buffer (process-buffer proc)
+ (goto-char (point-min))
+ (cl-loop
+ while (search-forward-regexp pattern nil t)
+ for msg = (match-string 4)
+ for (beg . end) = (flymake-diag-region
+ source
+ (string-to-number (match-string 1))
+ (string-to-number (match-string 2)))
+ for type = (pcase (match-string 3)
+ ("error" :error)
+ ("warning" :warning)
+ (_ :note))
+ when (and beg end)
+ collect (flymake-make-diagnostic source beg end type msg)
+ into diags
+ finally (funcall report-fn diags))))
+ (kill-buffer (process-buffer proc)))))))
+ (unless dialect
+ (error "`sh-shellcheck-flymake' is not suitable for shell type `%s'"
+ sh-shell))
+ (setq sh--shellcheck-process
+ (make-process
+ :name "shellcheck" :noquery t :connection-type 'pipe
+ :buffer (generate-new-buffer " *flymake-shellcheck*")
+ :command `(,sh-shellcheck-program
+ "--format=gcc"
+ "--color=never"
+ "-s" ,dialect
+ ,@sh-shellcheck-arguments
+ "-")
+ :sentinel sentinel))
+ (save-restriction
+ (widen)
+ (process-send-region sh--shellcheck-process (point-min) (point-max))
+ (process-send-eof sh--shellcheck-process))))
+
(provide 'sh-script)
;;; sh-script.el ends here
--
2.37.3
[-- Attachment #3: Type: text/plain, Size: 1916 bytes --]
On Sun, 18 Sep 2022 at 11:55, Philip Kaludercic wrote:
> Augusto Stoffel <arstoffel@gmail.com> writes:
>
>>
>> I could split this into two defcustoms if you feel strongly about it,
>> but it seems a bit of overengineering to me. Not many people will want
>>to customize the program name, and the switches would have to be
>> provided as a list anyway, since we're not going to call this through a
>> shell.
>
> I think it would be good, because then you could make the flags file
> local in case you need something special, without having to worry about
> some evil file that sets the command to
>
> '("rm" "-rf" "--no-preserve-root" "/")
>
Well, fine then. I've split this into two variables.
> I don't use `named-let' that much, but calling both the result and the
> recursive function `dialect' seems confusing to me.
>
Welcome to Lisp-2...
>> + (pattern "^-:\\([0-9]+\\):\\([0-9]+\\): \\([^:]+\\): \\(.*\\)$")
>
> Do you think that that using `rx' would make this pattern more maintainable?
>
I use rx often, but I think this is still at a level where rx doesn't
really help much.
>> + (sentinel
>> + (lambda (proc _event)
>
> Wouldn't it be cleaner to pull this lambda out into a separate named function?
>
That's not possible, it needs to be in that lexical scope.
>> + (setq sh--shellcheck-process
>> + (make-process
>> + :name "luacheck" :noquery t :connection-type 'pipe
> ^
> Typo?
>
>> + :buffer (generate-new-buffer " *flymake-luacheck*")
> ^
> same here
Ouch, fixed.
> Also what happens if someone doesn't have shellcheck installed?
Then Flymake logs a warning and disables the backend. The error message
will be whatever make-process gives, which I find more than good enough.
next prev parent reply other threads:[~2022-09-18 12:38 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-17 16:48 bug#57884: [PATCH] Flymake backend using the shellcheck program Augusto Stoffel
2022-09-17 17:05 ` Eli Zaretskii
2022-09-17 17:32 ` Augusto Stoffel
2022-09-17 17:52 ` Eli Zaretskii
2022-09-17 18:17 ` Stefan Kangas
2022-09-17 17:55 ` Eli Zaretskii
2022-09-17 18:04 ` Stefan Kangas
2022-09-18 7:52 ` Augusto Stoffel
2022-09-18 11:55 ` Philip Kaludercic
2022-09-18 12:38 ` Augusto Stoffel [this message]
2022-09-18 12:50 ` Philip Kaludercic
2022-09-18 13:30 ` Augusto Stoffel
2022-09-18 13:46 ` Philip Kaludercic
2022-09-18 19:38 ` Augusto Stoffel
2022-09-18 20:22 ` Philip Kaludercic
2022-09-18 21:18 ` Philip Kaludercic
2022-09-19 7:33 ` Augusto Stoffel
2022-09-19 8:03 ` Philip Kaludercic
2022-09-24 7:05 ` Augusto Stoffel
2022-09-24 8:01 ` Philip Kaludercic
2022-11-04 22:56 ` Philip Kaludercic
2022-09-18 11:58 ` Philip Kaludercic
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87fsgoyi0l.fsf@gmail.com \
--to=arstoffel@gmail.com \
--cc=57884@debbugs.gnu.org \
--cc=eliz@gnu.org \
--cc=philipk@posteo.net \
--cc=stefankangas@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.