bug#70929: 30.0.50; eglot--apply-text-edits prevents point adjustment

[João Távora]

On Tue, May 14, 2024 at 7:23 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Troy Brown <brownts@troybrown.dev>
> > Date: Mon, 13 May 2024 22:15:07 -0400
> >
> > Language Servers may use onTypeFormatting to provide indentation for a
> > buffer.  When this happens, the language server will indicate a
> > newline trigger character (in the DocumentOnTypeFormattingOptions).
> > In the Emacs buffer, after hitting RET, point is moved to the next
> > line and a textDocument/onTypeFormatting request is sent from Eglot to
> > the server.  The server responds back with the corresponding spacing
> > prefix for the line in newText of the TextEdit response.  However,
> > when Eglot applies the text edit to insert this spacing, via
> > eglot--apply-text-edits, it uses save-excursion, and this prevents the
> > point from being pushed to the end of the inserted spacing.  It would
> > seem that save-excursion should be avoided when applying text edits.
> > This issue has been observed with the Ada Language Server.

I've reproduced this on the clangd server and c++-ts-mode, but only after
turning _off_ electric indent-mode, which hides this effect.

> eglot--apply-text-edits, it uses save-excursion, and this prevents the
> point from being pushed to the end of the inserted spacing.  It would
> seem that save-excursion should be avoided when applying text edits.

Doing that naively would lead to chaos.  Edits can be supplied to arbitrary
places in the buffer.  Edits can happen in many situations, even when inserting
completions, for example.  If you circumscribe yourself to OnTypeFormatting,
even Clangd for example performs edits before the full expression that precedes
the newline.  There not even anything forcing the server to provide
whitespace-only changes.

The solution would have to be something like checking these specific
edits in this specific situation supplied by the server one by one and upon
finding one that  is whitespace only and overlaps the points make that
change a "push" change.

This is simply too complex of a change (you can take a stab at it and
provide ample testing, if you want).

The workaround of enabling electric-indent-mode or just turning off
OnTypeFormatting
via eglot-ignored-server-capabilities is much better.