bug#70193: [PATCH] Re: bug#70193: Acknowledgement (eglot: RFE: recenter buffer upon showDocument request)

[Felician Nemeth <felician.nemeth@gmail.com>]

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

> I'm sure the patch is correct but 50+ lines of window-management code
> in eglot.el? 

I agree it can go elsewhere.

> Just for that one user?

I don't think that should matter.

> There's nothing LSP-specific in the new function
> `eglot--window-recenter-region`,  it's pure window management code.
>
> The derived-mode-p specifically bit also looks very much out
> of place in Eglot.  What is it accomplishing, and why is this
> prog-mode exception not in the preceding function itself
> (maybe as an optional toggle)

reposition-window relies on `beginning-of-defun' and `end-of-defun', but
it is potentially better than window-recenter-region, because if the
selection of showDocument is inside a function, then it tries to show
the preceding comments as well.

> In fact, there's nothing intrinsically LSP-specific about having
> clients request Emacs shows the user a given part of a file.
> I can't believe this obscure LSP interface is its only client.
> Is it really?

Maybe textDocument/publishDiagnostics is somewhat similar.  Flymake
shows the region of the diagnostic messages with as little care as Eglot
currently shows the selection of showDocument.

> So, if this new impeccably documented function designed by Emacs's
> window management expert :-) does what it says, I think we should
> put it somewhere else.

I agree.

> Practical matters: there's the usual problem with Eglot
> compatibility with older Emacsen, but there's:
>
> * compat.el (I think Phil has already made Eglot use compat.el recently)
> * there's the strategy of using (or creating) another GNU ELPA :core package
> (like external-completion.el and many other ones)
> * there' the strategy of 'fboundp' where this nice-to-have "excellent
> recentering" feature would only appear to Eglot users running it on
> a recent Emacs.

I think the third option is good enough in this case.

The current patch can be regarded as a prof-of-concept implementation if
anyone is interested in continuing to work on this.