bug#60338: [PATCH] Add option to present server changes as diffs

[Philip Kaludercic]

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

> On Thu, Dec 29, 2022 at 2:39 PM Philip Kaludercic <philipk@posteo.net>
> wrote:
>
>> João Távora <joaotavora@gmail.com> writes:
>>
>> > The idea is good, but how does this play along with the existing
>> > eglot-confirm-server-initiated-edits?
>>
>> It takes precedence, since the diff is regarded as a kind of prompt.  If
>> you want to, I can also update the patch to use that option instead of a
>> custom one (e.g. present a diff if
>> `eglot-confirm-server-initiated-edits' is set to `diff').
>
> This is an improvement, but it's not enough, unfortunately.
>
> The current semantics of eglot-confirm-server-initiated-edits must first be
> investigated, then potentially expanded/consolidated, even before the
> augmentation
> with the new 'diff value.   Only then should 'diff be added.  If we do this
> some other
> way, we only increase inconsistency and confusion.
>
> Here is some information to get started with the investigation.
>
> The function responsible for applying edits, eglot--apply-workspace-edit is
> called currently from 3 places:
>
> 1. eglot-rename (this ignores eglot-c-s-initiated-edits, but confirms with
> a prefix
> argument).  The reasoning is that this is such a common action that by
> default
> we shouldn't bother the user with confirmation.

This is also one of the situations where I believe that a diff is not
necessary, since the change is predictable.

> 2. When the user chooses a code action from a list of code actions presented
> as a menu and that code action happens to contain an edit.  This also
> ignores
> eglot-c-s-initiated-edits.  The locus of the call is
> eglot--read-execute-code-action.
> The reasoning here is, I believe, that the user is already being presented
> with
> an interactive prompt, and presenting a second follow-up seemed too much.
>
> 3. When applying a code action that makes the server initiate a code
> action.
> The locus is eglot--handle-request (for workspace/applyEdit).  Here
> eglot-confirm-server-initiated-edits is read honoured.  The reasoning here
> is
> that the user might not be aware of the breadth of the code action that the
> server is about to propose.
>
> Note that the differences between 2 and 3 are subtle, and perhaps
> conceptually
> non-existent, but technically they do exist.  In 2, the edit is immediately
> available whereas in 3, a further server trip is required to get the edit
> to apply.

I am afraid I can't conceptualise the difference between the two.  The
2. is probably what I am familiar with, but when does the server
initiate a code action.  Are we talking about an unprompted change?  Or
something like code formatting?

> Moreover, the current method of  "confirmation" is just a prompt that lists
> the
> files about to be changed.  This is what should change, perhaps even by
> default,  to a presentation of a diff in a separate buffer.

Would you say that this means we should always generate a diff and only
apply it when the user confirms the change, or is the cost of doubling
the communication acceptable?

> Currently, the confirmation happens also (regardless of the value of
> eglot-confirm-server-initiated-edits) if any of the files about to be
> changed
> is not yet visited in a buffer.
>
> So to summarize, I would like to hear proposals on how to make
> user confirmation of edits more consistent and predictable across the
> various use cases.
>
> Then, as I've already said, the "diff" method of confirmation sounds in
> principle very robust  and in-line with Emacs' principles. It could
> eventually
> even be made the default.

So in the sense that a diff is presented as a kind of confirmation,
along with the option to apply all the changes immediately.   All in
all, this makes me less certain that merging the change into
`eglot-confirm-server-initiated-edits' is the right approach.

> João