On Tue, Apr 23, 2024 at 8:43 AM Felician Nemeth <felician.nemeth@gmail.com>
wrote:

> 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.
>

window.el seem like a good place.


> > Just for that one user?
>
> I don't think that should matter.
>

What I meant is that if it goes into eglot.el it will be "just for that one
[Eglot]
user" .  If it goes somewhere else, it's for more users.

> 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.
>

I see, more or less.  So it's a further refinement specific to
window-recenter-region specific to prog-mode buffers.  Can it operate
independently  or does it have to come necessarily after a call to
window-recenter-region?  Regardless of the answer, I think it
would be best placed in prog-mode.el or whereabouts.


> > 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.
>

Eh.  Guilty as charged.  So another user, showing Flymake diagnostics
(not necessarily from LSP publishDiagnostics, mind you).


> > * 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.
>

It's a nice one.  I think compat.el is not in Eglot just yet anyway.

João