[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#57884: [PATCH] Flymake backend using the shellcheck program
From: |
Philip Kaludercic |
Subject: |
bug#57884: [PATCH] Flymake backend using the shellcheck program |
Date: |
Sun, 18 Sep 2022 13:46:34 +0000 |
Augusto Stoffel <arstoffel@gmail.com> writes:
> On Sun, 18 Sep 2022 at 12:50, Philip Kaludercic wrote:
>
>>>> 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...
>>
>> It is not so much that they share the same name, but that I don't
>> understand why you close to give the function the name "dialect". From
>> what I have seen, named-let is usually given a self-referential call
>> like "next", "recur", "loop", etc.
>
> Yeah, I have no strong opinion about the name.
>
>>>> 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.
>>
>> Not if you use `process-put' and `process-get'. I recently reworked
>> flymake-proselint and did the same thing. IMO it doesn't look that bad:
>>
>> https://git.sr.ht/~manuel-uberti/flymake-proselint/commit/30c4baa08db32e73d956c978c81a9f79062c2e1d
>
> Honestly, I much prefer to have some state localized in a closure than
> in what is effective a global variable...
Not quite, you can only access the value if you have access to the
process object. I guess you can argue that the process object is stored
as a file local variable which sort of makes it global...
> I like in your backend that you read a JSON output, which presumably
> provides the start _and end_ of the diagnostic region. How did you
> convert from line/column to buffer position?
I didn't do that, Flymake takes care of that if you give it the buffer
position (unless I misunderstood something, but it appears to be working
for me).
> I think it would be nice to extend flymake-diag-region to have signature
>
> (flymake-diag-region BUFFER LINE &optional COL LINE-END COL-END)
>
> If you provide LINE-END and COL-END, then those are converted to buffer
> positions and there's no guessing as to what they should be.
>
>>>> 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.
>>
>> But isn't that rather late to detect the error. Couldn't you also check
>> if "shellcheck" (or whatever the new option name will be) can be found
>> in the path before adding the hook in `shell-mode'?
>
> I think this is worse than not doing anything. If you don't check and
> let the backend fail, it gets displayed as a disabled backend, and the
> user has the chance to look at the log buffer and try to figure out why
> it's not working. If we did what we suggest, we'd have to reinvent the
> wheel to convey that information to the user, I think.
On the other hand there might be people who don't care about shellcheck
(say they just open a shell script once a year) and any error message
would just be an annoyance.
The most important part is having the ability to use shellcheck
prominently documented, the rest is probably just a matter of taste.
- bug#57884: [PATCH] Flymake backend using the shellcheck program, (continued)
- bug#57884: [PATCH] Flymake backend using the shellcheck program, Philip Kaludercic, 2022/09/18
- bug#57884: [PATCH] Flymake backend using the shellcheck program, Augusto Stoffel, 2022/09/18
- bug#57884: [PATCH] Flymake backend using the shellcheck program, Philip Kaludercic, 2022/09/18
- bug#57884: [PATCH] Flymake backend using the shellcheck program, Augusto Stoffel, 2022/09/18
- bug#57884: [PATCH] Flymake backend using the shellcheck program,
Philip Kaludercic <=
- bug#57884: [PATCH] Flymake backend using the shellcheck program, Augusto Stoffel, 2022/09/18
- bug#57884: [PATCH] Flymake backend using the shellcheck program, Philip Kaludercic, 2022/09/18
- bug#57884: [PATCH] Flymake backend using the shellcheck program, Philip Kaludercic, 2022/09/18
- bug#57884: [PATCH] Flymake backend using the shellcheck program, Augusto Stoffel, 2022/09/19
- bug#57884: [PATCH] Flymake backend using the shellcheck program, Philip Kaludercic, 2022/09/19
- bug#57884: [PATCH] Flymake backend using the shellcheck program, Augusto Stoffel, 2022/09/24
- bug#57884: [PATCH] Flymake backend using the shellcheck program, Philip Kaludercic, 2022/09/24
- bug#57884: [PATCH] Flymake backend using the shellcheck program, Philip Kaludercic, 2022/09/18