GNU bug report logs - #67152
[PATCH] Fix flymake integration in lua-ts-mode

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 67152 in the body.
You can then email your comments to 67152 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: jm <at> pub.pink
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Fix flymake integration in lua-ts-mode
Date: Mon, 13 Nov 2023 22:31:40 +0000

If a file has an error on the last line flymake gets an invalid
position for the overlay and raises the error:

  error in process sentinel: Wrong type argument: integer-or-marker-p, nil

Message #8 received at 67152 <at> debbugs.gnu.org (full text, mbox):

From: jm <at> pub.pink
To: 67152 <at> debbugs.gnu.org
Subject: Re: bug#67152: [PATCH] Fix flymake integration in lua-ts-mode
Date: Mon, 13 Nov 2023 22:40:36 +0000

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

[0001-Fix-flymake-integration-in-lua-ts-mode-Bug-67152.patch (text/x-diff, attachment)]

Message #11 received at 67152 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: jm <at> pub.pink, João Távora <joaotavora <at> gmail.com>
Cc: 67152 <at> debbugs.gnu.org
Subject: Re: bug#67152: [PATCH] Fix flymake integration in lua-ts-mode
Date: Tue, 14 Nov 2023 16:13:29 +0200

> Date: Mon, 13 Nov 2023 22:40:36 +0000
> TLS-Required: No
> From: jm--- via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>

João, is this OK?

> From ab3ecedb9e4ed4818603249e774dd8a1e6dae28b Mon Sep 17 00:00:00 2001
> From: john muhl <jm <at> pub.pink>
> Date: Mon, 13 Nov 2023 16:06:07 -0600
> Subject: [PATCH] Fix flymake integration in lua-ts-mode (Bug#67152)
> 
> * lisp/progmodes/lua-ts-mode.el (lua-ts-flymake-luacheck): Use
> flymake-diag-region to mark highlighted region.
> ---
>  lisp/progmodes/lua-ts-mode.el | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/lisp/progmodes/lua-ts-mode.el b/lisp/progmodes/lua-ts-mode.el
> index bb6d5cb8c91..ad753210dd4 100644
> --- a/lisp/progmodes/lua-ts-mode.el
> +++ b/lisp/progmodes/lua-ts-mode.el
> @@ -508,16 +508,18 @@ lua-ts-flymake-luacheck
>                                              eol))
>                                     nil t)
>                              for line = (string-to-number (match-string 1))
> -                            for beg = (string-to-number (match-string 2))
> -                            for end = (string-to-number (match-string 3))
> +                            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
> -                                                             (cons line beg)
> -                                                             (cons line (1+ end))
> +                                                             beg
> +                                                             end
>                                                               type
>                                                               msg)
>                              into diags
> -- 
> 2.41.0
> 

Message #14 received at 67152 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 67152 <at> debbugs.gnu.org, jm <at> pub.pink
Subject: Re: bug#67152: [PATCH] Fix flymake integration in lua-ts-mode
Date: Tue, 14 Nov 2023 14:23:29 +0000

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

On Tue, Nov 14, 2023 at 2:13 PM Eli Zaretskii <eliz <at> gnu.org> wrote:

> > Date: Mon, 13 Nov 2023 22:40:36 +0000
> > TLS-Required: No
> > From: jm--- via "Bug reports for GNU Emacs,
> >  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>
> João, is this OK?
>

Oooof hard to say.  But no I don't think so.  You see the type
of the object changing in the call to flymake-make-diagnostic?
That could be seriously problematic for non-buffer-local diagnostics of
some  backends.

And another call to flymake-diag-region?

Can you point me to the first message in this thread via a debbugs
URL, or to one where a clear Emacs -Q recipe is given?

Thanks,
João

[Message part 2 (text/html, inline)]

Message #17 received at 67152 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 67152 <at> debbugs.gnu.org, jm <at> pub.pink
Subject: Re: bug#67152: [PATCH] Fix flymake integration in lua-ts-mode
Date: Tue, 14 Nov 2023 14:26:30 +0000

On Tue, Nov 14, 2023 at 2:23 PM João Távora <joaotavora <at> gmail.com> wrote:
>
> On Tue, Nov 14, 2023 at 2:13 PM Eli Zaretskii <eliz <at> gnu.org> wrote:
>>
>> > Date: Mon, 13 Nov 2023 22:40:36 +0000
>> > TLS-Required: No
>> > From: jm--- via "Bug reports for GNU Emacs,
>> >  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>>
>> João, is this OK?
>
> Oooof hard to say.  But no I don't think so.

Oh no! Silly me, this isn't flymake.el at all!!  I saw a fragment
of a loop and thought it was my code :-), i.e. a framework problem.

Well, then in this case, I think it should be OK, though users should
test. Backends are indeed expected to call flymake-diag-region to
get the region to highlight, as the manual and docstrings explain
 (I think).

João

Message #20 received at 67152 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: João Távora <joaotavora <at> gmail.com>
Cc: 67152 <at> debbugs.gnu.org, jm <at> pub.pink
Subject: Re: bug#67152: [PATCH] Fix flymake integration in lua-ts-mode
Date: Tue, 14 Nov 2023 16:37:43 +0200

> From: João Távora <joaotavora <at> gmail.com>
> Date: Tue, 14 Nov 2023 14:23:29 +0000
> Cc: jm <at> pub.pink, 67152 <at> debbugs.gnu.org
> 
> Can you point me to the first message in this thread via a debbugs
> URL, or to one where a clear Emacs -Q recipe is given?

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=67152#5

Message #23 received at 67152 <at> debbugs.gnu.org (full text, mbox):

From: jm <at> pub.pink
To: "João Távora" <joaotavora <at> gmail.com>, "Eli Zaretskii"
 <eliz <at> gnu.org>
Cc: 67152 <at> debbugs.gnu.org
Subject: Re: bug#67152: [PATCH] Fix flymake integration in lua-ts-mode
Date: Tue, 14 Nov 2023 18:08:28 +0000

November 14, 2023 at 2:26 PM, "João Távora" <joaotavora <at> gmail.com> wrote:

> Oh no! Silly me, this isn't flymake.el at all!! I saw a fragment
> of a loop and thought it was my code :-), i.e. a framework problem.

Right. The problem is the code in lua-ts-mode. Here is the full
recipe just in case (you need the tree-sitter-lua grammar and
luacheck in PATH or set lua-ts-luacheck-program):

  src/emacs test.lua --init-directory=.
  M-x lua-ts-mode

  # test.lua
  print(1)
  print(2

  # init.el
  (add-hook 'lua-ts-mode-hook #'flymake-mode)
  (add-to-list 'treesit-extra-load-path "~/.guix-profile/lib/tree-sitter")
  (setopt lua-ts-luacheck-program "~/.luarocks/bin/luacheck")
  (toggle-debug-on-error)

The error only shows up when the highlighted region is on the
last line.

> Well, then in this case, I think it should be OK,

Thanks for confirming.

> though users should test.

What kind of test do you mean?

> Backends are indeed expected to call flymake-diag-region to
> get the region to highlight, as the manual and docstrings explain
>  (I think).

I agree the docs are clear. I even used the example from the
manual as a template. Now I can’t remember why I originally
deviated from it on the region handling but I’m sure it was
caused by a misunderstanding on my part.

Message #26 received at 67152 <at> debbugs.gnu.org (full text, mbox):

From: jm <at> pub.pink
To: 67152 <at> debbugs.gnu.org
Subject: Re: bug#67152: [PATCH] Fix flymake integration in lua-ts-mode
Date: Wed, 15 Nov 2023 00:09:20 +0000

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

Updated to remove a compiler warning about the unused 'line' variable.

[0001-Fix-flymake-integration-in-lua-ts-mode-Bug-67152.patch (text/x-diff, attachment)]

Message #29 received at 67152 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: jm <at> pub.pink
Cc: 67152 <at> debbugs.gnu.org
Subject: Re: bug#67152: [PATCH] Fix flymake integration in lua-ts-mode
Date: Wed, 15 Nov 2023 12:24:08 +0000

Just a note that in the recent patch you provided, (match-string 3)
isn't being used.  If is is a useful "end column" identifier, you
might want to call flymake-diag-region again to get the the
corresponding buffer position.

flymake-diag-region could see some docstring improvements.
It gets a line and optionally a column, whidh defines a
single point.  It then tries to guess a region, because it's
impossible to form a region from just a point.

The guess is based on thing-at-point. This guess _may_ be worse
than whatever the backend supplied in (match-string 3) so passing
that number to flymake-diag-region again to obtain a second point
may be a good idea to compose the two buffer positions to give
to flymake-make-diagnostic.

João

Message #34 received at 67152-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: jm <at> pub.pink
Cc: 67152-done <at> debbugs.gnu.org
Subject: Re: bug#67152: [PATCH] Fix flymake integration in lua-ts-mode
Date: Wed, 15 Nov 2023 15:02:34 +0200

> Date: Wed, 15 Nov 2023 00:09:20 +0000
> TLS-Required: No
> From: jm--- via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> Updated to remove a compiler warning about the unused 'line' variable.

Thanks, installed on the master branch, and closing the bug.

Previous Next

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