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.