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

[Philip Kaludercic]

john muhl <jm@pub.pink> writes:

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

It would make sense that it shouldn't matter (unless there is some
subtle performance penalty in traversing the buffer in some order), so I
think you can drop the nreverse.

> Thanks for the review.
>
>>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))

LGTM, but I am not really a Flymake expert.

-- 
        Philip Kaludercic on peregrine