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

Previous Next

Full log

View this message in rfc822 format

From: john muhl <jm <at> pub.pink>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 70167 <at> debbugs.gnu.org
Subject: bug#70167: [PATCH] Mark Flymake regions more accurately in lua-ts-mode
Date: Thu, 04 Apr 2024 11:45:05 -0500

[Message part 1 (text/plain, inline)]

Philip Kaludercic <philipk <at> posteo.net> writes:

> john muhl <jm <at> 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.

[0001-Mark-Flymake-regions-more-accurately-in-lua-ts-mode.patch (text/x-patch, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.