all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Philip Kaludercic <philipk@posteo.net>
To: john muhl <jm@pub.pink>
Cc: 70167@debbugs.gnu.org
Subject: bug#70167: [PATCH] Mark Flymake regions more accurately in lua-ts-mode
Date: Wed, 10 Apr 2024 08:28:08 +0000	[thread overview]
Message-ID: <87h6g9mxk7.fsf@posteo.net> (raw)
In-Reply-To: <875xwwq5o4.fsf@pub.pink> (john muhl's message of "Thu, 04 Apr 2024 11:45:05 -0500")

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





  reply	other threads:[~2024-04-10  8:28 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-03 17:55 bug#70167: [PATCH] Mark Flymake regions more accurately in lua-ts-mode john muhl
     [not found] ` <handler.70167.B.171216697516444.ack@debbugs.gnu.org>
2024-04-03 17:59   ` john muhl
2024-04-04  6:05     ` Philip Kaludercic
2024-04-04  6:05     ` Philip Kaludercic
2024-04-04 16:45       ` john muhl
2024-04-10  8:28         ` Philip Kaludercic [this message]
2024-04-13  8:12         ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87h6g9mxk7.fsf@posteo.net \
    --to=philipk@posteo.net \
    --cc=70167@debbugs.gnu.org \
    --cc=jm@pub.pink \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.