recenter the cursor when responding to internal window/showDocument downcall

[adonovan]

The LSP showDocument downcall opens the designated file, moves the cursor to the designated position, and raises the frame. One other thing it could do to make it easier to see where the cursor is would be to recenter the buffer.

The required change is this:

(cl-defmethod eglot-handle-request
  (_server (_method (eql window/showDocument)) &key
           uri external takeFocus selection)
  "Handle request window/showDocument."
  (let ((success t)
        (filename))
    (cond
     ((eq external t) (browse-url uri))
     ((file-readable-p (setq filename (eglot-uri-to-path uri)))
      ;; Use run-with-timer to avoid nested client requests like the
      ;; "synchronous imenu" floated in bug#62116 presumably caused by
      ;; which-func-mode.
      (run-with-timer
       0 nil
       (lambda ()
         (with-current-buffer (find-file-noselect filename)
           (cond (takeFocus
                  (pop-to-buffer (current-buffer))
                  (select-frame-set-input-focus (selected-frame)))
                 ((display-buffer (current-buffer))))
           (when selection
             (pcase-let ((`(,beg . ,end) (eglot-range-region selection)))
               ;; FIXME: it is very naughty to use someone else's `--'
               ;; function, but `xref--goto-char' happens to have
               ;; exactly the semantics we want vis-a-vis widening.
               (xref--goto-char beg)
+	       (recenter)
               (pulse-momentary-highlight-region beg end 'highlight)))))))
     (t (setq success :json-false)))
    `(:success ,success)))

I have tested it locally and it works nicely. Can we merge it?

[joaotavora]

Send this as a patch to [email protected]

[nemethf]

The LSP showDocument downcall opens the designated file, moves the cursor to the designated position, and raises the frame. One other thing it could do to make it easier to see where the cursor is would be to recenter the buffer.

I have tested it locally and it works nicely.

What happens when the selection is taller than the half of the window height? Doesn't recenter moves the point in the middle of the window? It would be a bit better to try to display the whole selection.

[joaotavora]

I have some experience in these recenter blues. There' slots of complicated logic in SLY https://github.com/joaotavora/sly https://github.com/joaotavora/sly/blob/ed17d2c2bd7aead0fbb09c3d22861c80a522a097/sly.el#L805.

Perhaps too complicated.

Most recently I dealt with the same problem in an experimental "zapp" extension https://github.com/joaotavora/zapp/blob/a1a640a1496877ca0d91d3aad8c5e76f49cfff10/zapp.el#L772. Maybe that' ssimpler

[joaotavora]

Anyway @adonovan definitely do propose this, and maybe CC Martin Rudalics or some other Emacs windowing expert. This isn't a new problem.

[adonovan]

Good question. A variant of recenter that centers the whole selection if (and only if) it fits might be a useful addition, perhaps to the the core elisp library. In our case, which I suspect is fairly typical, the selection is a single identifier such as a function or type declaration, so the behavior of recenter is ideal, as it allows you to see the doc comments above the declaration and (at least the start of) the function body below it.

[dgutov]

IIUC the proposed "more complex" implementation won't do worse in the one-line case either.

[adonovan]

Thanks; reported at https://debbugs.gnu.org/cgi/bugreport.cgi?bug=70193.