bug#70167: [PATCH] Mark Flymake regions more accurately in lua-ts-mode

[john muhl <jm@pub.pink>]

[-- Attachment #1: Type: text/plain, Size: 2053 bytes --]

Philip Kaludercic <philipk@posteo.net> writes:

> john muhl <jm@pub.pink> writes:
>
>> +                           (let (beg end msg type diags)
>> +                             (while
>
> Why do you declare these variables outside of the loop?  Should the
> values persist between iterations?  If not, you could avoid the setq
> soup below, by declaring and binding the variables at once.

Only the list of diagnostics is used outside the loop. I’ve moved
the others inside the while.

>> +                                 (search-forward-regexp
>> +                                  (rx (: bol (0+ alnum) ":"
>                                           ^
>                                           this is not necessary, since
>                                           the rx body has an implicit ":".

Fixed.

>> +                               (setq msg (match-string 4))
>> +                               (setq type (if (string-match "^(W" msg) :warning
>                                                   ^
>                                                   You can avoid a
>                                                   regular expression
>                                                   here using `string-prefix-p'.

Fixed.

>> +                                            :error))
>> +                               (when (and beg end)
>> +                                 (setq diags
>> +                                       (nconc diags
>> +                                              (list (flymake-make-diagnostic
>> + source beg end type msg))))))
>> +                             (funcall report-fn diags)))
>
> If I see this correctly, then you are appending each element to the end
> of the list?  If so, it would be more efficient to just construct the
> list in reverse using `push' and then `nreverse'ing it before passing it
> to REPORT-FN.

Changed to use push. It doesn’t look like the order matters or did
I misunderstand something?

Thanks for the review.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Mark-Flymake-regions-more-accurately-in-lua-ts-mode.patch --]
[-- Type: text/x-patch, Size: 4526 bytes --]

From 910e88cf83415ae1e077fbc36560cb29fd39b666 Mon Sep 17 00:00:00 2001
From: john muhl <jm@pub.pink>
Date: Wed, 13 Mar 2024 08:35:08 -0500
Subject: [PATCH] Mark Flymake regions more accurately in lua-ts-mode

* lisp/progmodes/lua-ts-mode.el (lua-ts-flymake-luacheck): Use
the end position provided by Luacheck rather than relying on
'thing-at-point' to guess where the end should be.  (bug#70167)
---
 lisp/progmodes/lua-ts-mode.el | 53 +++++++++++++++++------------------
 1 file changed, 26 insertions(+), 27 deletions(-)

diff --git a/lisp/progmodes/lua-ts-mode.el b/lisp/progmodes/lua-ts-mode.el
index 407ef230c32..45ea8ec9a81 100644
--- a/lisp/progmodes/lua-ts-mode.el
+++ b/lisp/progmodes/lua-ts-mode.el
@@ -35,7 +35,6 @@
 (require 'treesit)
 
 (eval-when-compile
-  (require 'cl-lib)
   (require 'rx))
 
 (declare-function treesit-induce-sparse-tree "treesit.c")
@@ -544,32 +543,32 @@ lua-ts-flymake-luacheck
                            (eq proc lua-ts--flymake-process))
                          (with-current-buffer (process-buffer proc)
                            (goto-char (point-min))
-                           (cl-loop
-                            while (search-forward-regexp
-                                   (rx (seq bol
-                                            (0+ alnum) ":"
-                                            (group (1+ digit)) ":"
-                                            (group (1+ digit)) "-"
-                                            (group (1+ digit)) ": "
-                                            (group (0+ nonl))
-                                            eol))
-                                   nil t)
-                            for (beg . end) = (flymake-diag-region
-                                               source
-                                               (string-to-number (match-string 1))
-                                               (string-to-number (match-string 2)))
-                            for msg = (match-string 4)
-                            for type = (if (string-match "^(W" msg)
-                                           :warning
-                                         :error)
-                            when (and beg end)
-                            collect (flymake-make-diagnostic source
-                                                             beg
-                                                             end
-                                                             type
-                                                             msg)
-                            into diags
-                            finally (funcall report-fn diags)))
+                           (let (diags)
+                             (while (search-forward-regexp
+                                     (rx bol (0+ alnum) ":"
+                                         (group (1+ digit)) ":"
+                                         (group (1+ digit)) "-"
+                                         (group (1+ digit)) ": "
+                                         (group (0+ nonl)) eol)
+                                     nil t)
+                               (let* ((beg
+                                       (car (flymake-diag-region
+                                             source
+                                             (string-to-number (match-string 1))
+                                             (string-to-number (match-string 2)))))
+                                      (end
+                                       (cdr (flymake-diag-region
+                                             source
+                                             (string-to-number (match-string 1))
+                                             (string-to-number (match-string 3)))))
+                                      (msg (match-string 4))
+                                      (type (if (string-prefix-p "(W" msg)
+                                                :warning
+                                              :error)))
+                                 (push (flymake-make-diagnostic
+                                        source beg end type msg)
+                                       diags)))
+                             (funcall report-fn diags)))
                        (flymake-log :warning "Canceling obsolete check %s" proc))
                    (kill-buffer (process-buffer proc)))))))
       (process-send-region lua-ts--flymake-process (point-min) (point-max))
-- 
2.41.0