GNU bug report logs - #72696
Track-changes errors out when file is overwritten using Node.js's fs.writeFile (at least on macOS)

Previous Next

Package: emacs;

Reported by: Dario Gjorgjevski <dario.gjorgjevski <at> gmail.com>

Date: Sun, 18 Aug 2024 11:00:01 UTC

Severity: normal

Done: Stefan Monnier <monnier <at> iro.umontreal.ca>

Bug is archived. No further changes may be made.

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

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#72696; Package emacs. (Sun, 18 Aug 2024 11:00:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Dario Gjorgjevski <dario.gjorgjevski <at> gmail.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Sun, 18 Aug 2024 11:00:02 GMT) Full text and rfc822 format available.

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

From: Dario Gjorgjevski <dario.gjorgjevski <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Track-changes errors out when file is overwritten using Node.js's
 fs.writeFile (at least on macOS)
Date: Sun, 18 Aug 2024 12:58:24 +0200
How to reproduce:

1. In one shell, fire up GNU Emacs, open a file, and start something
that uses track-changes; e.g., Eglot.  In my case it was a Python file
called test.py with contents foo = 0.  Everything is happy so far.
2. In another shell, fire up a Node.js interpreter and execute const
fs = require("node:fs/promises") followed by await
fs.writeFile("test.py", "foo = 1").  This will overwrite the file.
3. Track-changes (and by extension Eglot) will error out.  Restarting
Eglot makes everything go back to normal.

The *Warnings* buffer now contains:

    Warning (emacs): Missing/incorrect calls to
‘before/after-change-functions’!!
    Details logged to `track-changes--error-log'

And `track-changes--error-log' contains:

    (("test.py" #1=(unexpected-after 1 8 0)
      ((t track-changes--recover-from-error (#1#) nil)
       (t track-changes--after (1 8 0) nil)
       (t revert-buffer-insert-file-contents--default-function
          ("/path/to/test.py"
           nil)
          nil)
       (t revert-buffer--default (ignore-auto dont-ask) nil)
       (t revert-buffer (ignore-auto dont-ask preserve-modes) nil)
       (t auto-revert-handler nil nil)
       (t auto-revert-buffer (#<buffer test.py>) nil)
       (t #<subr auto-revert-buffers> nil nil)
       (t auto-revert-buffers <at> buffer-list-filter
          (#<subr auto-revert-buffers>) nil)
       (t apply
          (auto-revert-buffers <at> buffer-list-filter #<subr
                              auto-revert-buffers> nil)
          nil)
       (t auto-revert-buffers nil nil)
       (t apply (auto-revert-buffers nil) nil)
       (t timer-event-handler
          ([nil 26305 53479 773611 5 auto-revert-buffers nil nil 0 nil])
          nil))
      [(nil . minibuffer-complete-and-exit) 5 (nil . move-end-of-line) 127
       (nil . python-indent-dedent-line-backspace) 48
       (nil . self-insert-command) 24 24 19 (nil . save-buffer) 27 91 79
       27 91 73 27 91 79]))

Despite `python-indent-dedent-line-backspace' being present in the
backtrace, the error is not limited to `python-mode'.

My GNU Emacs is built from:

    Repository revision: 40eecd594ac60f38b6729fd9cf3474a8b9d133b9
    Repository branch: master
    System Description: macOS 14.5
    Configured using:
     'configure --without-x --without-ns --with-mailutils
     --with-kerberos5 --with-json --with-tree-sitter
     --with-native-compilation

Best regards,
Dario




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#72696; Package emacs. (Sun, 18 Aug 2024 11:06:01 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dario Gjorgjevski <dario.gjorgjevski <at> gmail.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, João Távora
 <joaotavora <at> gmail.com>
Cc: 72696 <at> debbugs.gnu.org
Subject: Re: bug#72696: Track-changes errors out when file is overwritten using
 Node.js's fs.writeFile (at least on macOS)
Date: Sun, 18 Aug 2024 14:02:11 +0300
> From: Dario Gjorgjevski <dario.gjorgjevski <at> gmail.com>
> Date: Sun, 18 Aug 2024 12:58:24 +0200
> 
> How to reproduce:
> 
> 1. In one shell, fire up GNU Emacs, open a file, and start something
> that uses track-changes; e.g., Eglot.  In my case it was a Python file
> called test.py with contents foo = 0.  Everything is happy so far.
> 2. In another shell, fire up a Node.js interpreter and execute const
> fs = require("node:fs/promises") followed by await
> fs.writeFile("test.py", "foo = 1").  This will overwrite the file.
> 3. Track-changes (and by extension Eglot) will error out.  Restarting
> Eglot makes everything go back to normal.
> 
> The *Warnings* buffer now contains:
> 
>     Warning (emacs): Missing/incorrect calls to
> ‘before/after-change-functions’!!
>     Details logged to `track-changes--error-log'
> 
> And `track-changes--error-log' contains:
> 
>     (("test.py" #1=(unexpected-after 1 8 0)
>       ((t track-changes--recover-from-error (#1#) nil)
>        (t track-changes--after (1 8 0) nil)
>        (t revert-buffer-insert-file-contents--default-function
>           ("/path/to/test.py"
>            nil)
>           nil)
>        (t revert-buffer--default (ignore-auto dont-ask) nil)
>        (t revert-buffer (ignore-auto dont-ask preserve-modes) nil)
>        (t auto-revert-handler nil nil)
>        (t auto-revert-buffer (#<buffer test.py>) nil)
>        (t #<subr auto-revert-buffers> nil nil)
>        (t auto-revert-buffers <at> buffer-list-filter
>           (#<subr auto-revert-buffers>) nil)
>        (t apply
>           (auto-revert-buffers <at> buffer-list-filter #<subr
>                               auto-revert-buffers> nil)
>           nil)
>        (t auto-revert-buffers nil nil)
>        (t apply (auto-revert-buffers nil) nil)
>        (t timer-event-handler
>           ([nil 26305 53479 773611 5 auto-revert-buffers nil nil 0 nil])
>           nil))
>       [(nil . minibuffer-complete-and-exit) 5 (nil . move-end-of-line) 127
>        (nil . python-indent-dedent-line-backspace) 48
>        (nil . self-insert-command) 24 24 19 (nil . save-buffer) 27 91 79
>        27 91 73 27 91 79]))
> 
> Despite `python-indent-dedent-line-backspace' being present in the
> backtrace, the error is not limited to `python-mode'.
> 
> My GNU Emacs is built from:
> 
>     Repository revision: 40eecd594ac60f38b6729fd9cf3474a8b9d133b9
>     Repository branch: master
>     System Description: macOS 14.5
>     Configured using:
>      'configure --without-x --without-ns --with-mailutils
>      --with-kerberos5 --with-json --with-tree-sitter
>      --with-native-compilation

Adding Stefan and João.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#72696; Package emacs. (Sun, 18 Aug 2024 16:12:02 GMT) Full text and rfc822 format available.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Dario Gjorgjevski <dario.gjorgjevski <at> gmail.com>
Cc: 72696 <at> debbugs.gnu.org
Subject: Re: bug#72696: Track-changes errors out when file is overwritten
 using Node.js's fs.writeFile (at least on macOS)
Date: Sun, 18 Aug 2024 12:10:37 -0400
> 1. In one shell, fire up GNU Emacs, open a file, and start something
> that uses track-changes; e.g., Eglot.  In my case it was a Python file
> called test.py with contents foo = 0.  Everything is happy so far.
> 2. In another shell, fire up a Node.js interpreter and execute const
> fs = require("node:fs/promises") followed by await
> fs.writeFile("test.py", "foo = 1").  This will overwrite the file.
> 3. Track-changes (and by extension Eglot) will error out.  Restarting
> Eglot makes everything go back to normal.
>
> The *Warnings* buffer now contains:
>
>     Warning (emacs): Missing/incorrect calls to
> ‘before/after-change-functions’!!
>     Details logged to `track-changes--error-log'

The above is only a "warning": it just logged the occurrence of something
undesired, but track-changes supposedly recovered from that.
IOW you describe two problems:

- Somehow/somewhere there is a "Missing/incorrect calls to
  ‘before/after-change-functions".
  [ But these problems are sufficiently common that track-changes has
    code devoted to detecting and handling those cases.  ]
  We want to try and fix the origin of this problem, but it's
  been with us "for ever", so it's not super extra urgent because there
  will still be other cases where it can show up.
  Eglot has had to deal with this problem since its inception, it just
  used to keep silent about it, contrary to track-changes (because
  I want to get rid of those problems).

- Eglot errors out.  This should definitely not happen and seems like
  a regression.  Restarting Eglot should definitely not be needed.

Could you clarify what you mean by Eglot erroring out and why
restarting it is necessary?

> And `track-changes--error-log' contains:
>
>     (("test.py" #1=(unexpected-after 1 8 0)
>       ((t track-changes--recover-from-error (#1#) nil)
>        (t track-changes--after (1 8 0) nil)
>        (t revert-buffer-insert-file-contents--default-function
>           ("/path/to/test.py"
>            nil)
>           nil)
>        (t revert-buffer--default (ignore-auto dont-ask) nil)
>        (t revert-buffer (ignore-auto dont-ask preserve-modes) nil)

Hmm... assuming the file really started with content "foo = 0" (and no
terminating newline), "track-changes--after (1 8 0)" looks OK: the
revert has changed the file size by 0 chars and the new content spans
chars from 1 to 8.  So my guess is that revert-buffer failed to call the
`before-changes-functions` or that it called it with a smaller region
(e.g. only the region that's actually modified (7..8), where "0" is
turned into "1").

Will have to dig deeper.

BTW, your recipe does not mention enabling auto-revert.  Have you
confirmed on your side that the problem shows up also in
a vanilla config?


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#72696; Package emacs. (Sun, 18 Aug 2024 20:39:02 GMT) Full text and rfc822 format available.

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

From: Dario Gjorgjevski <dario.gjorgjevski <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 72696 <at> debbugs.gnu.org
Subject: Re: bug#72696: Track-changes errors out when file is overwritten
 using Node.js's fs.writeFile (at least on macOS)
Date: Sun, 18 Aug 2024 22:36:01 +0200
Hi, Stefan,

On Sun, Aug 18, 2024 at 6:10 PM Stefan Monnier <monnier <at> iro.umontreal.ca> wrote:
>
> - Eglot errors out.  This should definitely not happen and seems like
>   a regression.  Restarting Eglot should definitely not be needed.
>
> Could you clarify what you mean by Eglot erroring out and why
> restarting it is necessary?

Eglot gets in a weird state and begins reporting erroneous data to the
LSP server.

>
> Hmm... assuming the file really started with content "foo = 0" (and no
> terminating newline), "track-changes--after (1 8 0)" looks OK: the
> revert has changed the file size by 0 chars and the new content spans
> chars from 1 to 8.  So my guess is that revert-buffer failed to call the
> `before-changes-functions` or that it called it with a smaller region
> (e.g. only the region that's actually modified (7..8), where "0" is
> turned into "1").
>
> Will have to dig deeper.

One thing I see is that the buffer becomes empty for a split second,
and is then overwritten with the new contents.  I don't know how
exactly Node.js does the overwriting, but I guess it's worth looking
into.

>
> BTW, your recipe does not mention enabling auto-revert.  Have you
> confirmed on your side that the problem shows up also in
> a vanilla config?

You're right, the problem depends on auto-revert-mode being active.
Sorry about that.

Best regards,
Dario




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#72696; Package emacs. (Tue, 20 Aug 2024 12:17:02 GMT) Full text and rfc822 format available.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Dario Gjorgjevski <dario.gjorgjevski <at> gmail.com>
Cc: 72696 <at> debbugs.gnu.org
Subject: Re: bug#72696: Track-changes errors out when file is overwritten
 using Node.js's fs.writeFile (at least on macOS)
Date: Tue, 20 Aug 2024 08:15:49 -0400
>> - Eglot errors out.  This should definitely not happen and seems like
>>   a regression.  Restarting Eglot should definitely not be needed.
>> Could you clarify what you mean by Eglot erroring out and why
>> restarting it is necessary?
> Eglot gets in a weird state and begins reporting erroneous data to the
> LSP server.

Can you give any further details?


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#72696; Package emacs. (Thu, 22 Aug 2024 11:23:01 GMT) Full text and rfc822 format available.

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

From: Dario Gjorgjevski <dario.gjorgjevski <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 72696 <at> debbugs.gnu.org
Subject: Re: bug#72696: Track-changes errors out when file is overwritten
 using Node.js's fs.writeFile (at least on macOS)
Date: Thu, 22 Aug 2024 13:19:39 +0200
Hi, Stefan,

On Tue, Aug 20, 2024 at 2:15 PM Stefan Monnier <monnier <at> iro.umontreal.ca> wrote:
>
> >> - Eglot errors out.  This should definitely not happen and seems like
> >>   a regression.  Restarting Eglot should definitely not be needed.
> >> Could you clarify what you mean by Eglot erroring out and why
> >> restarting it is necessary?
> > Eglot gets in a weird state and begins reporting erroneous data to the
> > LSP server.
>
> Can you give any further details?

Consider a Python file with the following contents (including a
trailing newline):

    from urllib import urlretrieve
    local_filename, headers = urlretrieve('http://python.org/')

Using the Pyright LSP server, Eglot correctly reports that:

    Pyright [reportAttributeAccessIssue]: "urlretrieve" is unknown import symbol

Now, rewrite the file using Node.js as instructed above so that its
contents become (again, including a trailing newline):

    from urllib.request import urlretrieve
    local_filename, headers = urlretrieve('http://python.org/')

Eglot now weirdly reports that:

    Pyright [reportMissingImports]: Import "urllib.request.request"
could not be resolved

Somehow it thinks that the ".request" part appears twice in a row.
Here is the relevant communication between Eglot and Pyright:

    [jsonrpc] e[13:00:21.956] --> textDocument/didOpen
    {
      "jsonrpc": "2.0",
      "method": "textDocument/didOpen",
      "params": {
        "textDocument": {
          "uri": "file:///Volumes/workplace/playground/test.py",
          "version": 0,
          "languageId": "python",
          "text": "from urllib import urlretrieve\nlocal_filename,
headers = urlretrieve('http://python.org/')\n"
        }
      }
    }

    [jsonrpc] e[13:00:22.617] <-- textDocument/publishDiagnostics
    {
      "jsonrpc": "2.0",
      "method": "textDocument/publishDiagnostics",
      "params": {
        "uri": "file:///Volumes/workplace/playground/test.py",
        "version": 0,
        "diagnostics": [
          {
            "range": {
              "start": {
                "line": 0,
                "character": 19
              },
              "end": {
                "line": 0,
                "character": 30
              }
            },
            "message": "\"urlretrieve\" is unknown import symbol",
            "severity": 1,
            "code": "reportAttributeAccessIssue",
            "source": "Pyright",
            "codeDescription": {
              "href":
"https://github.com/microsoft/pyright/blob/main/docs/configuration.md#reportAttributeAccessIssue"
            }
          }
        ]
      }
    }

    [jsonrpc] e[13:00:26.362] --> textDocument/didClose
    {
      "jsonrpc": "2.0",
      "method": "textDocument/didClose",
      "params": {
        "textDocument": {
          "uri": "file:///Volumes/workplace/playground/test.py"
        }
      }
    }

    [jsonrpc] e[13:00:26.364] --> textDocument/didOpen
    {
      "jsonrpc": "2.0",
      "method": "textDocument/didOpen",
      "params": {
        "textDocument": {
          "uri": "file:///Volumes/workplace/playground/test.py",
          "version": 0,
          "languageId": "python",
          "text": "from urllib.request import
urlretrieve\nlocal_filename, headers =
urlretrieve('http://python.org/')\n"
        }
      }
    }

    [jsonrpc] e[13:00:26.366] --> textDocument/didChange
    {
      "jsonrpc": "2.0",
      "method": "textDocument/didChange",
      "params": {
        "textDocument": {
          "uri": "file:///Volumes/workplace/playground/test.py",
          "version": 1
        },
        "contentChanges": [
          {
            "range": {
              "start": {
                "line": 0,
                "character": 11
              },
              "end": {
                "line": 0,
                "character": 11
              }
            },
            "rangeLength": 0,
            "text": ".request"
          }
        ]
      }
    }

    [jsonrpc] e[13:00:26.629] <-- textDocument/publishDiagnostics
    {
      "jsonrpc": "2.0",
      "method": "textDocument/publishDiagnostics",
      "params": {
        "uri": "file:///Volumes/workplace/playground/test.py",
        "version": 1,
        "diagnostics": [
          {
            "range": {
              "start": {
                "line": 0,
                "character": 5
              },
              "end": {
                "line": 0,
                "character": 27
              }
            },
            "message": "Import \"urllib.request.request\" could not be
resolved",
            "severity": 1,
            "code": "reportMissingImports",
            "source": "Pyright",
            "codeDescription": {
              "href":
"https://github.com/microsoft/pyright/blob/main/docs/configuration.md#reportMissingImports"
            }
          }
        ]
      }
    }

As far as I can see, the testDocument/didChange request should not be
there.  The rewrite seems to have triggered a textDocument/didClose
followed by a textDocument/didOpen, so the erroneous
textDocument/didChange ended up making the LSP server think the
".request" part appears twice in a row.

Best regards,
Dario




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#72696; Package emacs. (Thu, 22 Aug 2024 11:30:02 GMT) Full text and rfc822 format available.

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

From: Dario Gjorgjevski <dario.gjorgjevski <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 72696 <at> debbugs.gnu.org
Subject: Re: bug#72696: Track-changes errors out when file is overwritten
 using Node.js's fs.writeFile (at least on macOS)
Date: Thu, 22 Aug 2024 13:27:06 +0200
BTW, I am using Python for illustrative purposes only.  This issue is
particularly annoying when writing JavaScript/TypeScript with a LSP
server and ESLint.  Running eslint --fix for in-place linting makes
ESLint rewrite the files in exactly this way (see
https://github.com/eslint/eslint/blob/5dbdd63dc83428447e25f1fc1d05d8a69e3b006a/lib/eslint/eslint.js#L752),
which then makes the LSP server exhibit weird diagnostics and
necessitates a restart of Eglot.

Best regards,
Dario




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#72696; Package emacs. (Sat, 07 Sep 2024 07:17:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: monnier <at> iro.umontreal.ca, Dario Gjorgjevski <dario.gjorgjevski <at> gmail.com>
Cc: 72696 <at> debbugs.gnu.org
Subject: Re: bug#72696: Track-changes errors out when file is overwritten using
 Node.js's fs.writeFile (at least on macOS)
Date: Sat, 07 Sep 2024 10:15:20 +0300
> Cc: 72696 <at> debbugs.gnu.org
> From: Dario Gjorgjevski <dario.gjorgjevski <at> gmail.com>
> Date: Thu, 22 Aug 2024 13:27:06 +0200
> 
> BTW, I am using Python for illustrative purposes only.  This issue is
> particularly annoying when writing JavaScript/TypeScript with a LSP
> server and ESLint.  Running eslint --fix for in-place linting makes
> ESLint rewrite the files in exactly this way (see
> https://github.com/eslint/eslint/blob/5dbdd63dc83428447e25f1fc1d05d8a69e3b006a/lib/eslint/eslint.js#L752),
> which then makes the LSP server exhibit weird diagnostics and
> necessitates a restart of Eglot.

Stefan, any comments?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#72696; Package emacs. (Sun, 08 Sep 2024 23:08:01 GMT) Full text and rfc822 format available.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Dario Gjorgjevski <dario.gjorgjevski <at> gmail.com>
Cc: 72696 <at> debbugs.gnu.org
Subject: Re: bug#72696: Track-changes errors out when file is overwritten
 using Node.js's fs.writeFile (at least on macOS)
Date: Sun, 08 Sep 2024 19:06:55 -0400
[Message part 1 (text/plain, inline)]
> As far as I can see, the testDocument/didChange request should not be
> there.

Either the `didClose/didOpen` should not be there, or the `didChange` should
not be there.  The problem is that we have both.

Can you try the patch below, which I suspect should fix your problem?
It should get rid of the `didChange` in your recipe.

We could also change Eglot so it doesn't do `didClose/didOpen` upon
`revert-buffer` (in which case it should/will do the `didChange`), but
that should be a separate concern.


        Stefan
[eglot.patch (text/x-diff, inline)]
diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 82e99a2c920..a2c9f73fc73 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -2813,6 +2813,8 @@ eglot--signal-textDocument/didChange
 
 (defun eglot--signal-textDocument/didOpen ()
   "Send textDocument/didOpen to server."
+  ;; Flush any potential pending change.
+  (eglot--track-changes-fetch eglot--track-changes)
   (setq eglot--recent-changes nil
         eglot--versioned-identifier 0
         eglot--TextDocumentIdentifier-cache nil)

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#72696; Package emacs. (Wed, 18 Sep 2024 22:05:01 GMT) Full text and rfc822 format available.

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

From: Dario Gjorgjevski <dario.gjorgjevski <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 72696 <at> debbugs.gnu.org
Subject: Re: bug#72696: Track-changes errors out when file is overwritten
 using Node.js's fs.writeFile (at least on macOS)
Date: Thu, 19 Sep 2024 00:02:34 +0200
Hi, Stefan,

On Mon, Sep 9, 2024 at 1:06 AM Stefan Monnier <monnier <at> iro.umontreal.ca> wrote:
>
> > As far as I can see, the testDocument/didChange request should not be
> > there.
>
> Either the `didClose/didOpen` should not be there, or the `didChange` should
> not be there.  The problem is that we have both.
>
> Can you try the patch below, which I suspect should fix your problem?
> It should get rid of the `didChange` in your recipe.

I gave your patch a try and it indeed fixes the issue.  Thank you very
much for looking into it!

Best regards,
Dario




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#72696; Package emacs. (Thu, 19 Sep 2024 05:35:01 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dario Gjorgjevski <dario.gjorgjevski <at> gmail.com>,
 João Távora <joaotavora <at> gmail.com>
Cc: 72696 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#72696: Track-changes errors out when file is overwritten using
 Node.js's fs.writeFile (at least on macOS)
Date: Thu, 19 Sep 2024 08:32:21 +0300
> Cc: 72696 <at> debbugs.gnu.org
> From: Dario Gjorgjevski <dario.gjorgjevski <at> gmail.com>
> Date: Thu, 19 Sep 2024 00:02:34 +0200
> 
> Hi, Stefan,
> 
> On Mon, Sep 9, 2024 at 1:06 AM Stefan Monnier <monnier <at> iro.umontreal.ca> wrote:
> >
> > > As far as I can see, the testDocument/didChange request should not be
> > > there.
> >
> > Either the `didClose/didOpen` should not be there, or the `didChange` should
> > not be there.  The problem is that we have both.
> >
> > Can you try the patch below, which I suspect should fix your problem?
> > It should get rid of the `didChange` in your recipe.
> 
> I gave your patch a try and it indeed fixes the issue.  Thank you very
> much for looking into it!

João, is Stefan's patch okay with you?  AFAIU, the same problem exists
on the emacs-30 release branch, so I'd like to install the patch
there, if you agree.

Here's the patch in case you didn't see it (I CC'ed you on the
original report, but subsequent discussion omitted you for some
reason, sigh):

diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 82e99a2c920..a2c9f73fc73 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -2813,6 +2813,8 @@ eglot--signal-textDocument/didChange
 
 (defun eglot--signal-textDocument/didOpen ()
   "Send textDocument/didOpen to server."
+  ;; Flush any potential pending change.
+  (eglot--track-changes-fetch eglot--track-changes)
   (setq eglot--recent-changes nil
         eglot--versioned-identifier 0
         eglot--TextDocumentIdentifier-cache nil)




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#72696; Package emacs. (Thu, 19 Sep 2024 12:59:01 GMT) Full text and rfc822 format available.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 72696 <at> debbugs.gnu.org, Dario Gjorgjevski <dario.gjorgjevski <at> gmail.com>,
 João Távora <joaotavora <at> gmail.com>
Subject: Re: bug#72696: Track-changes errors out when file is overwritten
 using Node.js's fs.writeFile (at least on macOS)
Date: Thu, 19 Sep 2024 08:58:22 -0400
> João, is Stefan's patch okay with you?  AFAIU, the same problem exists
> on the emacs-30 release branch, so I'd like to install the patch
> there, if you agree.

Indeed, it belongs there.
It should have been part of the original commit d7a83e23d47c.


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#72696; Package emacs. (Thu, 19 Sep 2024 21:49:02 GMT) Full text and rfc822 format available.

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

From: João Távora <joaotavora <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 72696 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Dario Gjorgjevski <dario.gjorgjevski <at> gmail.com>
Subject: Re: bug#72696: Track-changes errors out when file is overwritten
 using Node.js's fs.writeFile (at least on macOS)
Date: Thu, 19 Sep 2024 22:47:14 +0100
On Thu, Sep 19, 2024 at 1:58 PM Stefan Monnier <monnier <at> iro.umontreal.ca> wrote:
>
> > João, is Stefan's patch okay with you?  AFAIU, the same problem exists
> > on the emacs-30 release branch, so I'd like to install the patch
> > there, if you agree.
>
> Indeed, it belongs there.
> It should have been part of the original commit d7a83e23d47c.

You can go ahead and merge it.  I dont' know what it does
but i trust Stefan.

Although I have to say that on the topic of synchronization failures
between Eglot and servers, I'm not sure that things have improved
much since this new layer of abstraction was added.  By "I'm not
sure" I really mean that.  Maybe pre-existing failures are just more
visible right now (because the new code warns and logs them), but
I have a nagging feeling that there are more of them.  No hard
proof though.

I also half-expected things to get fixed in Emacs proper, using
Eglot as testing bed, but I'm not sure that has happened either.
Anyway this is just a comment.

João




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#72696; Package emacs. (Fri, 20 Sep 2024 06:23:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: João Távora <joaotavora <at> gmail.com>
Cc: 72696 <at> debbugs.gnu.org, dario.gjorgjevski <at> gmail.com,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#72696: Track-changes errors out when file is overwritten
 using Node.js's fs.writeFile (at least on macOS)
Date: Fri, 20 Sep 2024 09:21:57 +0300
> From: João Távora <joaotavora <at> gmail.com>
> Date: Thu, 19 Sep 2024 22:47:14 +0100
> Cc: Eli Zaretskii <eliz <at> gnu.org>, Dario Gjorgjevski <dario.gjorgjevski <at> gmail.com>, 72696 <at> debbugs.gnu.org
> 
> On Thu, Sep 19, 2024 at 1:58 PM Stefan Monnier <monnier <at> iro.umontreal.ca> wrote:
> >
> > > João, is Stefan's patch okay with you?  AFAIU, the same problem exists
> > > on the emacs-30 release branch, so I'd like to install the patch
> > > there, if you agree.
> >
> > Indeed, it belongs there.
> > It should have been part of the original commit d7a83e23d47c.
> 
> You can go ahead and merge it.  I dont' know what it does
> but i trust Stefan.

Thanks.  Stefan, please install on the emacs-30 branch.

> Although I have to say that on the topic of synchronization failures
> between Eglot and servers, I'm not sure that things have improved
> much since this new layer of abstraction was added.  By "I'm not
> sure" I really mean that.  Maybe pre-existing failures are just more
> visible right now (because the new code warns and logs them), but
> I have a nagging feeling that there are more of them.  No hard
> proof though.
> 
> I also half-expected things to get fixed in Emacs proper, using
> Eglot as testing bed, but I'm not sure that has happened either.
> Anyway this is just a comment.

I agree with your general feeling, and I think this case might belong
to those pre-existing failures which track-changes uncovers.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#72696; Package emacs. (Fri, 20 Sep 2024 18:48:02 GMT) Full text and rfc822 format available.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 72696 <at> debbugs.gnu.org, dario.gjorgjevski <at> gmail.com,
 João Távora <joaotavora <at> gmail.com>
Subject: Re: bug#72696: Track-changes errors out when file is overwritten
 using Node.js's fs.writeFile (at least on macOS)
Date: Fri, 20 Sep 2024 14:46:43 -0400
>> You can go ahead and merge it.  I dont' know what it does
>> but i trust Stefan.
> Thanks.  Stefan, please install on the emacs-30 branch.

Thanks, done.

> I agree with your general feeling, and I think this case might belong
> to those pre-existing failures which track-changes uncovers.

FWIW, I think this bug was one that I introduced when changing Eglot to
use track-changes (basically, because track-changes does its own
"buffering" in addition to the one Eglot does in
`eglot--recent-changes`, and I forgot to flush that buffer when
`eglot--recent-changes` is flushed).


        Stefan





Reply sent to Stefan Monnier <monnier <at> iro.umontreal.ca>:
You have taken responsibility. (Tue, 24 Sep 2024 17:53:02 GMT) Full text and rfc822 format available.

Notification sent to Dario Gjorgjevski <dario.gjorgjevski <at> gmail.com>:
bug acknowledged by developer. (Tue, 24 Sep 2024 17:53:02 GMT) Full text and rfc822 format available.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: 72696-done <at> debbugs.gnu.org
Cc: , dario.gjorgjevski <at> gmail.com,
 João Távora <joaotavora <at> gmail.com>
Subject: Re: bug#72696: Track-changes errors out when file is overwritten
 using Node.js's fs.writeFile (at least on macOS)
Date: Tue, 24 Sep 2024 13:51:57 -0400
>>> You can go ahead and merge it.  I dont' know what it does
>>> but i trust Stefan.
>> Thanks.  Stefan, please install on the emacs-30 branch.
> Thanks, done.

Closing,


        Stefan





bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Wed, 23 Oct 2024 11:24:11 GMT) Full text and rfc822 format available.

This bug report was last modified 298 days ago.

Previous Next


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