bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#57884: [PATCH] Flymake backend using the shellcheck program


From: Augusto Stoffel
Subject: bug#57884: [PATCH] Flymake backend using the shellcheck program
Date: Sun, 18 Sep 2022 14:38:34 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux)

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.)

Attachment: 0001-New-Flymake-backend-using-the-shellcheck-program.patch
Description: Text Data

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.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]