bug#61478: 28.2; eglot keeps sending `didSave` notifications despite the active server not supporting `textDocumentSync`

bug#61478: 28.2; eglot keeps sending `didSave` notifications despite the active server not supporting `textDocumentSync`

[~kby via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 725 bytes --]

I've come across this bug whilst using the Dart language server, which 
is included in the Dart SDK.

Steps to reproduce bug:

- Create example Dart project using Flutter: `flutter create hello_world`

- Open `hello_world/lib/main.dart` in Emacs

- Start eglot with `M-x eglot RET`

- Make a rudimentary change in the file; ex. add a line break

- Save with `C-x C-s`

- Following text appears in the minibuffer area: `[eglot] Server reports 
(type=1): Unknown method textDocument/didSave`

Related bug reports (on GitHub): 
https://github.com/dart-lang/sdk/issues/36464 and 
https://github.com/microsoft/language-server-protocol/issues/288

I am using eglot 1.11 (from ELPA) with Emacs 28.2 on Arch.

Thanks in advance.

||

[-- Attachment #2: Type: text/html, Size: 1286 bytes --]

bug#61478: 28.2; eglot keeps sending `didSave` notifications despite the active server not supporting `textDocumentSync`

[Felician Nemeth]

> - Following text appears in the minibuffer area: `[eglot] Server
>   reports (type=1): Unknown method textDocument/didSave`

Without actually looking into the details, this feels similar to an
earlier issue, which I unfortunately closed:
https://github.com/joaotavora/eglot/issues/360

The commit referenced in #360 only fixed the willSave part.  I guess the
solution to this bug report should be probably similar to that fix:
Eglot should check (eglot--server-capable :textDocumentSync :save)
before sending a textDocument/didSave.

bug#61478: 28.2; eglot keeps sending `didSave` notifications despite the active server not supporting `textDocumentSync`

[João Távora]

[-- Attachment #1: Type: text/plain, Size: 772 bytes --]

Yep, Felician, that is exactly it. Should be a straightforward patch. I'll
do it, if noone beats me to it.

João

On Mon, Feb 13, 2023, 18:30 Felician Nemeth <felician.nemeth@gmail.com>
wrote:

> > - Following text appears in the minibuffer area: `[eglot] Server
> >   reports (type=1): Unknown method textDocument/didSave`
>
> Without actually looking into the details, this feels similar to an
> earlier issue, which I unfortunately closed:
> https://github.com/joaotavora/eglot/issues/360
>
> The commit referenced in #360 only fixed the willSave part.  I guess the
> solution to this bug report should be probably similar to that fix:
> Eglot should check (eglot--server-capable :textDocumentSync :save)
> before sending a textDocument/didSave.
>

[-- Attachment #2: Type: text/html, Size: 1216 bytes --]

bug#61478: 28.2; eglot keeps sending `didSave` notifications despite the active server not supporting `textDocumentSync`

[~kby via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1.1: Type: text/plain, Size: 877 bytes --]

This should suffice, no?

On 13.02.23 19:44, João Távora wrote:
> Yep, Felician, that is exactly it. Should be a straightforward patch. 
> I'll do it, if noone beats me to it.
>
> João
>
> On Mon, Feb 13, 2023, 18:30 Felician Nemeth 
> <felician.nemeth@gmail.com> wrote:
>
>     > - Following text appears in the minibuffer area: `[eglot] Server
>     >   reports (type=1): Unknown method textDocument/didSave`
>
>     Without actually looking into the details, this feels similar to an
>     earlier issue, which I unfortunately closed:
>     https://github.com/joaotavora/eglot/issues/360
>
>     The commit referenced in #360 only fixed the willSave part.  I
>     guess the
>     solution to this bug report should be probably similar to that fix:
>     Eglot should check (eglot--server-capable :textDocumentSync :save)
>     before sending a textDocument/didSave.
>

[-- Attachment #1.2: Type: text/html, Size: 2083 bytes --]

[-- Attachment #2: didSave.patch --]
[-- Type: text/x-patch, Size: 1110 bytes --]

diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 8ecdefd490d..267a7a5ea23 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -2496,13 +2496,14 @@ When called interactively, use the currently active server"
 (defun eglot--signal-textDocument/didSave ()
   "Send textDocument/didSave to server."
   (eglot--signal-textDocument/didChange)
-  (jsonrpc-notify
-   (eglot--current-server-or-lose)
-   :textDocument/didSave
-   (list
-    ;; TODO: Handle TextDocumentSaveRegistrationOptions to control this.
-    :text (buffer-substring-no-properties (point-min) (point-max))
-    :textDocument (eglot--TextDocumentIdentifier))))
+  (when (eglot--server-capable :textDocumentSync :save)
+    (jsonrpc-notify
+     (eglot--current-server-or-lose)
+     :textDocument/didSave
+     (list
+      ;; TODO: Handle TextDocumentSaveRegistrationOptions to control this.
+      :text (buffer-substring-no-properties (point-min) (point-max))
+      :textDocument (eglot--TextDocumentIdentifier)))))
 
 (defun eglot-flymake-backend (report-fn &rest _more)
   "A Flymake backend for Eglot.

bug#61478: 28.2; eglot keeps sending `didSave` notifications despite the active server not supporting `textDocumentSync`

[João Távora]

~kby <kby@tilde.team> writes:

> This should suffice, no?

Yes, that should be the correct patch.  

Eli, this is a one-line/trivial change.  Do we need (rather than prefer)
copyright assignment for this one to go in and close the bug?

João

bug#61478: 28.2; eglot keeps sending `didSave` notifications despite the active server not supporting `textDocumentSync`

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Cc: Felician Nemeth <felician.nemeth@gmail.com>,  61478@debbugs.gnu.org,
>   eliz@gnu.org
> Date: Mon, 13 Feb 2023 23:50:25 +0000
> 
> ~kby <kby@tilde.team> writes:
> 
> > This should suffice, no?
> 
> Yes, that should be the correct patch.  
> 
> Eli, this is a one-line/trivial change.  Do we need (rather than prefer)
> copyright assignment for this one to go in and close the bug?

ENOPATCH

Probably no need for copyright assignment, but I need to see the
changes.

bug#61478: 28.2; eglot keeps sending `didSave` notifications despite the active server not supporting `textDocumentSync`

[João Távora]

[-- Attachment #1: Type: text/plain, Size: 806 bytes --]

On Tue, Feb 14, 2023, 12:02 Eli Zaretskii <eliz@gnu.org> wrote:

> > From: João Távora <joaotavora@gmail.com>
> > Cc: Felician Nemeth <felician.nemeth@gmail.com>,  61478@debbugs.gnu.org,
> >   eliz@gnu.org
> > Date: Mon, 13 Feb 2023 23:50:25 +0000
> >
> > ~kby <kby@tilde.team> writes:
> >
> > > This should suffice, no?
> >
> > Yes, that should be the correct patch.
> >
> > Eli, this is a one-line/trivial change.  Do we need (rather than prefer)
> > copyright assignment for this one to go in and close the bug?
>
> ENOPATCH
>
> Probably no need for copyright assignment, but I need to see the
> changes.
>

I don't understand, i see a didSave.patch file attached to to kby's mail to
this bug. That's the (trivial) patch I'm referring to. Can't you see it,
Eli?

João

>

[-- Attachment #2: Type: text/html, Size: 1912 bytes --]

bug#61478: 28.2; eglot keeps sending `didSave` notifications despite the active server not supporting `textDocumentSync`

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Tue, 14 Feb 2023 12:57:35 +0000
> Cc: kby@tilde.team, Felician Nemeth <felician.nemeth@gmail.com>, 61478@debbugs.gnu.org
> 
>  ENOPATCH
> 
>  Probably no need for copyright assignment, but I need to see the
>  changes.
> 
> I don't understand, i see a didSave.patch file attached to to kby's mail to this bug. That's the (trivial) patch I'm
> referring to. Can't you see it, Eli?

You mean, the one here:

  https://debbugs.gnu.org/cgi/bugreport.cgi?bug=61478#17

?  I received this message after receiving and answering yours.

If that is the patch, it is OK, but please remember a proper commit
log message, which mentions the bug number and includes the
Copyright-paperwork-exempt thingy.

Thanks.

bug#61478: 28.2; eglot keeps sending `didSave` notifications despite the active server not supporting `textDocumentSync`

[João Távora]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: João Távora <joaotavora@gmail.com>
>> Date: Tue, 14 Feb 2023 12:57:35 +0000
>> Cc: kby@tilde.team, Felician Nemeth <felician.nemeth@gmail.com>, 61478@debbugs.gnu.org
>> 
>>  ENOPATCH
>> 
>>  Probably no need for copyright assignment, but I need to see the
>>  changes.
>> 
>> I don't understand, i see a didSave.patch file attached to to kby's mail to this bug. That's the (trivial) patch I'm
>> referring to. Can't you see it, Eli?
>
> You mean, the one here:
>
>   https://debbugs.gnu.org/cgi/bugreport.cgi?bug=61478#17
>
> ?  I received this message after receiving and answering yours.

Odd, as the one you got earlier was a reply to the one you got later. An
EOUTOFORDER somewhere (probably due to moderation?)

> If that is the patch, it is OK, but please remember a proper commit
> log message, which mentions the bug number and includes the
> Copyright-paperwork-exempt thingy.

Yep, I'll provide that.  It's 'Copyright-paperwork-exempt: Yes' in its
own line, I think.

Regarding the "kby <kby@tilde.team>" author.  Kby is there any better
name?  Eli, are these obscure names OK as well?

João

bug#61478: 28.2; eglot keeps sending `didSave` notifications despite the active server not supporting `textDocumentSync`

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Cc: kby@tilde.team,  felician.nemeth@gmail.com,  61478@debbugs.gnu.org
> Date: Tue, 14 Feb 2023 14:12:23 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > You mean, the one here:
> >
> >   https://debbugs.gnu.org/cgi/bugreport.cgi?bug=61478#17
> >
> > ?  I received this message after receiving and answering yours.
> 
> Odd, as the one you got earlier was a reply to the one you got later. An
> EOUTOFORDER somewhere (probably due to moderation?)

Order of mail delivery is not guaranteed in general.

> Regarding the "kby <kby@tilde.team>" author.  Kby is there any better
> name?  Eli, are these obscure names OK as well?

If the authors don't want to provide a longer name, we are fine with
how they call themselves.

bug#61478: 28.2; eglot keeps sending `didSave` notifications despite the active server not supporting `textDocumentSync`

[João Távora]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: João Távora <joaotavora@gmail.com>
>> Date: Tue, 14 Feb 2023 12:57:35 +0000
>> Cc: kby@tilde.team, Felician Nemeth <felician.nemeth@gmail.com>, 61478@debbugs.gnu.org
>> 
>>  ENOPATCH
>> 
>>  Probably no need for copyright assignment, but I need to see the
>>  changes.
>> 
>> I don't understand, i see a didSave.patch file attached to to kby's mail to this bug. That's the (trivial) patch I'm
>> referring to. Can't you see it, Eli?
>
> You mean, the one here:
>
>   https://debbugs.gnu.org/cgi/bugreport.cgi?bug=61478#17
>
> ?  I received this message after receiving and answering yours.
>
> If that is the patch, it is OK, but please remember a proper commit
> log message, which mentions the bug number and includes the
> Copyright-paperwork-exempt thingy.

I just committed this patch as agreed, to emacs-29.  Closing.

João