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

[Philip Kaludercic <philipk@posteo.net>]

john muhl <jm@pub.pink> writes:

>>From a0c1f9c84a7072a807141f7b304a3c98c8e92173 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 | 55 ++++++++++++++++++-----------------
>  1 file changed, 28 insertions(+), 27 deletions(-)
>
> diff --git a/lisp/progmodes/lua-ts-mode.el b/lisp/progmodes/lua-ts-mode.el
> index 407ef230c32..a39cfa9e814 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,34 @@ 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 (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.

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

> +                                         (group (1+ digit)) ":"
> +                                         (group (1+ digit)) "-"
> +                                         (group (1+ digit)) ": "
> +                                         (group (0+ nonl)) eol))
> +                                  nil t)
> +                               (setq beg
> +                                     (car (flymake-diag-region
> +                                           source
> +                                           (string-to-number (match-string 1))
> +                                           (string-to-number (match-string 2)))))
> +                               (setq end
> +                                     (cdr (flymake-diag-region
> +                                           source
> +                                           (string-to-number (match-string 1))
> +                                           (string-to-number (match-string 3)))))
> +                               (setq msg (match-string 4))
> +                               (setq type (if (string-match "^(W" msg) :warning
                                                  ^
                                                  You can avoid a
                                                  regular expression
                                                  here using `string-prefix-p'.

> +                                            :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.

>                         (flymake-log :warning "Canceling obsolete check %s" proc))
>                     (kill-buffer (process-buffer proc)))))))
>        (process-send-region lua-ts--flymake-process (point-min) (point-max))

-- 
	Philip Kaludercic on peregrine