GNU bug report logs - #79259
[PATCH] Fix arg doc off-by-one bug of eglot signature help

Previous Next

To reply to this bug, email your comments to 79259 AT debbugs.gnu.org.
There is no need to reopen the bug first.

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

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

From: Lin Jian <me <at> linj.tech>
To: bug-gnu-emacs <at> gnu.org
Cc: João Távora <joaotavora <at> gmail.com>
Subject: [PATCH] Fix arg doc off-by-one bug of eglot signature help
Date: Sun, 17 Aug 2025 05:48:14 +0800

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

Tags: patch

`parlabel' is 0-indexed.  Arguments of `substring' are also
0-indexed.  No need to add 1 to `parlabel' before passing it to
`substring'.

[0001-Fix-arg-doc-off-by-one-bug-of-eglot-signature-help.patch (text/patch, attachment)]

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

From: João Távora <joaotavora <at> gmail.com>
To: Lin Jian <me <at> linj.tech>
Cc: bug-gnu-emacs <at> gnu.org
Subject: Re: [PATCH] Fix arg doc off-by-one bug of eglot signature help
Date: Sat, 16 Aug 2025 23:02:22 +0100

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

On Sat, Aug 16, 2025 at 10:48 PM Lin Jian <me <at> linj.tech> wrote:

> Tags: patch
>
> `parlabel' is 0-indexed.  Arguments of `substring' are also
> 0-indexed.  No need to add 1 to `parlabel' before passing it to
> `substring'.
>

Thanks, but I (or whoever) added the 1+ there probably did so for
a reason.

Maybe they were mistaken, but you have to supply a reproduction recipe
that demonstrates there's a problem with that, so we can check the
before and after, else I could be fixing it for you and breaking it for
someone else.

Please see https://joaotavora.github.io/eglot/#Reporting-bugs-1 for how to
report an Emacs -Q recipe (including a LSP server setup).

Thanks,
João

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

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

From: Lin Jian <me <at> linj.tech>
To: João Távora <joaotavora <at> gmail.com>
Cc: bug-gnu-emacs <at> gnu.org
Subject: Re: [PATCH] Fix arg doc off-by-one bug of eglot signature help
Date: Sun, 17 Aug 2025 06:47:38 +0800

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

João Távora <joaotavora <at> gmail.com> writes:

> Thanks, but I (or whoever) added the 1+ there probably did so for 
> a reason.  

It was added by you[1].

> Maybe they were mistaken, but you have to supply a reproduction recipe
> that demonstrates there's a problem with that, so we can check the 
> before and after, else I could be fixing it for you and breaking it for
> someone else.

I've reported some bugs and I'm familiar with (info "(emacs)Bugs").

I did not include a reproduction recipe in this report because I think
it is trivial to reproduce:

1. use a LSP server supporting argument documentation in the signature
help
2. get one signature help with argument documentation from eglot
3. see the highlight of argument documentation is off-by-one

typescript-language-server is mentioned in the commit[1] adding `1+', so
I guess it can be used for reproduction.

I am developing[2] the signature help feature for
haskell-language-server, which causes me to discover this off-by-one
"bug".

I added two screenshots before and after applying my patch as
attachments.

[without-my-patch.png (image/png, attachment)]

[with-my-patch.png (image/png, attachment)]

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

Here is the transcript between eglot and my developing
haskell-language-server:

[jsonrpc] e[06:33:30.460] --> textDocument/signatureHelp[192] 
(:jsonrpc "2.0" :id 192 :method "textDocument/signatureHelp" :params
          (:textDocument
           (:uri
            "file:///home/linj/code/origin/hydra-scraper/src/TestHls.hs")
           :position (:line 107 :character 19)))
[jsonrpc] e[06:33:30.471] <-- textDocument/signatureHelp[192] 
(:id 192 :jsonrpc "2.0" :result
     (:activeParameter 0 :activeSignature 0 :signatures
                       [(:activeParameter 0 :documentation
                                          (:kind "markdown" :value
                                                 "\n\nFun doc for  `fsimple` .\n\n")
                                          :label
                                          "fsimple :: forall a. a -> Bool -> Char"
                                          :parameters
                                          [(:documentation
                                            (:kind "markdown" :value
                                                   "\n\nThe first arg.\n\n")
                                            :label [21 22])
                                           (:documentation
                                            (:kind "markdown" :value
                                                   "\n\nThe second  `Bool`  arg.\n\n")
                                            :label [26 30])])
                        (:activeParameter 0 :documentation
                                          (:kind "markdown" :value
                                                 "\n\nFun doc for  `fsimple` .\n\n")
                                          :label
                                          "fsimple :: Integer -> Bool -> Char"
                                          :parameters
                                          [(:documentation
                                            (:kind "markdown" :value
                                                   "\n\nThe first arg.\n\n")
                                            :label [11 18])
                                           (:documentation
                                            (:kind "markdown" :value
                                                   "\n\nThe second  `Bool`  arg.\n\n")
                                            :label [22 26])])]))

> Please see https://joaotavora.github.io/eglot/#Reporting-bugs-1 for how to 
> report an Emacs -Q recipe (including a LSP server setup).

Thanks for the link.  I forgot to check that it before sending my
report.

I do not have time to provide more info now.

I think the above information is probably enough for this bug.  In
particular, the above server reply of signature help should be enough
for reproduction.  If it is not the case, please let me know and I will
provide more info later when I have time.

[1]: e33c0a549153fa3894f3b5e9c5e42ce07a1a68c7
[2]: https://github.com/haskell/haskell-language-server/pull/4626

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: joaotavora <at> gmail.com, Lin Jian <me <at> linj.tech>
Cc: 79259 <at> debbugs.gnu.org
Subject: Re: bug#79259: [PATCH] Fix arg doc off-by-one bug of eglot signature
 help
Date: Sat, 30 Aug 2025 11:04:47 +0300

> Cc: 79259 <at> debbugs.gnu.org
> Date: Sun, 17 Aug 2025 06:47:38 +0800
> From:  Lin Jian via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> João Távora <joaotavora <at> gmail.com> writes:
> 
> > Thanks, but I (or whoever) added the 1+ there probably did so for 
> > a reason.  
> 
> It was added by you[1].
> 
> > Maybe they were mistaken, but you have to supply a reproduction recipe
> > that demonstrates there's a problem with that, so we can check the 
> > before and after, else I could be fixing it for you and breaking it for
> > someone else.
> 
> I've reported some bugs and I'm familiar with (info "(emacs)Bugs").
> 
> I did not include a reproduction recipe in this report because I think
> it is trivial to reproduce:
> 
> 1. use a LSP server supporting argument documentation in the signature
> help
> 2. get one signature help with argument documentation from eglot
> 3. see the highlight of argument documentation is off-by-one
> 
> typescript-language-server is mentioned in the commit[1] adding `1+', so
> I guess it can be used for reproduction.
> 
> I am developing[2] the signature help feature for
> haskell-language-server, which causes me to discover this off-by-one
> "bug".
> 
> I added two screenshots before and after applying my patch as
> attachments.
> 
> 
> Here is the transcript between eglot and my developing
> haskell-language-server:
> 
> [jsonrpc] e[06:33:30.460] --> textDocument/signatureHelp[192] 
> (:jsonrpc "2.0" :id 192 :method "textDocument/signatureHelp" :params
>           (:textDocument
>            (:uri
>             "file:///home/linj/code/origin/hydra-scraper/src/TestHls.hs")
>            :position (:line 107 :character 19)))
> [jsonrpc] e[06:33:30.471] <-- textDocument/signatureHelp[192] 
> (:id 192 :jsonrpc "2.0" :result
>      (:activeParameter 0 :activeSignature 0 :signatures
>                        [(:activeParameter 0 :documentation
>                                           (:kind "markdown" :value
>                                                  "\n\nFun doc for  `fsimple` .\n\n")
>                                           :label
>                                           "fsimple :: forall a. a -> Bool -> Char"
>                                           :parameters
>                                           [(:documentation
>                                             (:kind "markdown" :value
>                                                    "\n\nThe first arg.\n\n")
>                                             :label [21 22])
>                                            (:documentation
>                                             (:kind "markdown" :value
>                                                    "\n\nThe second  `Bool`  arg.\n\n")
>                                             :label [26 30])])
>                         (:activeParameter 0 :documentation
>                                           (:kind "markdown" :value
>                                                  "\n\nFun doc for  `fsimple` .\n\n")
>                                           :label
>                                           "fsimple :: Integer -> Bool -> Char"
>                                           :parameters
>                                           [(:documentation
>                                             (:kind "markdown" :value
>                                                    "\n\nThe first arg.\n\n")
>                                             :label [11 18])
>                                            (:documentation
>                                             (:kind "markdown" :value
>                                                    "\n\nThe second  `Bool`  arg.\n\n")
>                                             :label [22 26])])]))
> 
> > Please see https://joaotavora.github.io/eglot/#Reporting-bugs-1 for how to 
> > report an Emacs -Q recipe (including a LSP server setup).
> 
> Thanks for the link.  I forgot to check that it before sending my
> report.
> 
> I do not have time to provide more info now.
> 
> I think the above information is probably enough for this bug.  In
> particular, the above server reply of signature help should be enough
> for reproduction.  If it is not the case, please let me know and I will
> provide more info later when I have time.

João, are you okay with the patch, given the additional information
posted by Lin Jian?

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

From: João Távora <joaotavora <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Lin Jian <me <at> linj.tech>, 79259-done <at> debbugs.gnu.org
Subject: Re: bug#79259: [PATCH] Fix arg doc off-by-one bug of eglot
 signature help
Date: Sat, 30 Aug 2025 11:46:42 +0100

Eli Zaretskii <eliz <at> gnu.org> writes:

> João, are you okay with the patch, given the additional information
> posted by Lin Jian?

The patch is OK in spirit, but I think it can be made simpler.

Anyway was added much ealier than
e33c0a549153fa3894f3b5e9c5e42ce07a1a68c7, in this commit:

   commit 9cedae50a27f0d0d6adee82976654d63c9f67602
   Date:   Wed Jan 9 21:09:35 2019 +0000
    
       Handle label offsets in parameterinformation
       
       At least ccls uses this.

So, I think it's more likely thatn not that we will be breaking ccls, or
at least the 2019 ccls.  Is this number 0-indexed?  Hard to say, reading
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_signatureHelp

However, if it's not 0-indexed, it is 1-indexed at most.  Therefore the
use of '1+' to _add_ to the server-supplied number definitely looks
wrong.  Though, again, it is probably wrong to accomodate exactly 2019
ccls, one of the few servers at the time that used this exotic new
feature.

Anyway, I pushed a slightly simpler fix.  I'm closing this issue.  If
the problem persists, we can reopen.

João

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

From: Lin Jian <me <at> linj.tech>
To: João Távora <joaotavora <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 79259-done <at> debbugs.gnu.org
Subject: Re: bug#79259: [PATCH] Fix arg doc off-by-one bug of eglot
 signature help
Date: Wed, 03 Sep 2025 14:52:23 +0800

João Távora <joaotavora <at> gmail.com> writes:

> Is this number 0-indexed?  Hard to say, reading
> https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_signatureHelp

I think it is 0-indexed.  I have verified that using vscode and
rust-analyzer.

> Anyway, I pushed a slightly simpler fix.

That slightly simpler "fix" changes an unrelated line of code.  It
introduces a new off-by-1 bug while keeping the off-by-1 bug my patch
indents to fix unchanged.

There are 3 things here:

1. `add-face-text-property' is 1-indexed
2. `substring' is 0-indexed
3. label offsets of ParameterInformation are 0-indexed

My patch fixes the mis-match of 2 and 3, while the slightly simpler
"fix" changes the correct match of 1 and 3.

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

From: João Távora <joaotavora <at> gmail.com>
To: Lin Jian <me <at> linj.tech>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 79259-done <at> debbugs.gnu.org
Subject: Re: bug#79259: [PATCH] Fix arg doc off-by-one bug of eglot
 signature help
Date: Wed, 03 Sep 2025 08:20:56 +0100

Lin Jian <me <at> linj.tech> writes:

>> Anyway, I pushed a slightly simpler fix.
>
> That slightly simpler "fix" changes an unrelated line of code.  It
> introduces a new off-by-1 bug while keeping the off-by-1 bug my patch
> indents to fix unchanged.

Thanks.  You're right, I pushed another fix.

> My patch fixes the mis-match of 2 and 3, while the slightly simpler
> "fix" changes the correct match of 1 and 3.

Should be taken care of now.  BTW, what I meant by "slightly simpler" is
that there's no need for tricks to transform vector to lists, just in
case a misunderstanding of that is why you're quoting "slightly simpler
fix" repeatedly.

João

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

From: Lin Jian <me <at> linj.tech>
To: João Távora <joaotavora <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 79259-done <at> debbugs.gnu.org
Subject: Re: bug#79259: [PATCH] Fix arg doc off-by-one bug of eglot
 signature help
Date: Wed, 03 Sep 2025 15:49:12 +0800

João Távora <joaotavora <at> gmail.com> writes:

> Thanks.  You're right, I pushed another fix.

Thanks.  The new fix looks good.

> BTW, what I meant by "slightly simpler" is
> that there's no need for tricks to transform vector to lists

I got the idea of the simplification.

> just in
> case a misunderstanding of that is why you're quoting "slightly simpler
> fix" repeatedly.

I just use that as a way to refer to the wrong fix.  No other meanings.

Previous Next

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