From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: =?UTF-8?B?Sm/Do28gVMOhdm9yYQ==?= Newsgroups: gmane.emacs.devel Subject: Re: Eglot "inlay hints" landed Date: Thu, 23 Feb 2023 23:59:42 +0000 Message-ID: References: <83edqqaf8c.fsf@gnu.org> <2B284D77-97DF-4B3E-89FB-13F0CA93D240@gmail.com> <87356xv65z.fsf_-_@gmail.com> <83fsawriye.fsf@gnu.org> <835ybsr6aa.fsf@gnu.org> <83356wr224.fsf@gnu.org> <87bklktu89.fsf@gmail.com> <83y1oophd0.fsf@gnu.org> <83k008pah3.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="19570"; mail-complaints-to="usenet@ciao.gmane.io" Cc: Eli Zaretskii , dalal.chinmay.0101@gmail.com, emacs-devel@gnu.org, dimitri@belopopsky.com, luangruo@yahoo.com To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Fri Feb 24 01:00:48 2023 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1pVLW4-0004vj-GF for ged-emacs-devel@m.gmane-mx.org; Fri, 24 Feb 2023 01:00:48 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pVLVL-0003P2-Pw; Thu, 23 Feb 2023 19:00:03 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pVLVG-0003O0-Mj for emacs-devel@gnu.org; Thu, 23 Feb 2023 18:59:58 -0500 Original-Received: from mail-ot1-x32b.google.com ([2607:f8b0:4864:20::32b]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pVLVD-0005Qg-Jh; Thu, 23 Feb 2023 18:59:58 -0500 Original-Received: by mail-ot1-x32b.google.com with SMTP id a14-20020a056830100e00b00690ed91749aso3097199otp.7; Thu, 23 Feb 2023 15:59:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1677196794; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=FVBGLTb4htdwsEBI4Z6MYVdGfKZwUqEvqrx3dZhbNqI=; b=jrthuYvgP/j7BMDC2582YX+ROmx98ttKKOXhyiXaG3lcxo50ExBpJioHHlh3Lcgfn2 GtUG+LwvIoHvXv+hRUi7Rxm3xpiLNSxXlsewPkJq1BUz9m9z6z6/lgYXErfXy6eZ9Keo P6kLw5adD4wgrcSvSW54fUP4W2o7r8zyvgFlXlCHBqIPXPthUIjY6zcvL8rMXq3WwaZd yuGG1LVgV/BS7RW7ZmFEPt45Q4iYqc5xWiupNx5NETMyyiN3YBzhRsBRXIODqAKZ716H cbG0wG7vIv5kFFerMUYZwnRYPhD7+zDkvquzyMmfIB0Je7S43zVyUe2joNtuSJfixx58 U2GQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1677196794; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=FVBGLTb4htdwsEBI4Z6MYVdGfKZwUqEvqrx3dZhbNqI=; b=Fk7taYNZQHLaB1cIEyRwGpK0yt8BB4lXWX/Dh9Zw6hucy/cOQyWwkHhfOqeZ7z7+gJ vS7rVIn+FeOSHAvYJxeEdwpkbmDx5VYpefvRrHsmEYd71nRPCiaVCuYFOLBtMoYV9qYi i7aBwPLpg4rR5ZxOsvljvrfkU9ufoROPY2mZq5GKWxe7mdfQ6Hu7wXjYAv+RJmvw9XbI XaPOJISh6OfMm45SpJtLHLo/Ei01SNcINRkX2yBEGABRdZFkjfCirzFjeri6LMnm87aJ QmNm5Merr7FSCfsXBht3aGLwlkkBxyDqoZ9ckSR3wqWMTOhyodcWZUmGeYBEKKu7MJxz tWHw== X-Gm-Message-State: AO0yUKW1L+oD7XA52uO3b2KB7hTUHLPKol3V7D/fCDegfGtBGdf1ZSYp VirJ+t5EjKULBuM8HstIkd0KdqmIqgC9RFTXJGs= X-Google-Smtp-Source: AK7set+lGcanqY0Ss4spm+Gn9IJYoOkIUXv5rKhP3WH+jKtzLtRqQ96yiMxaEDMaL6BNIoeYxejqDOQF6mEr0mDrwyY= X-Received: by 2002:a05:6830:2468:b0:690:ef0d:d946 with SMTP id x40-20020a056830246800b00690ef0dd946mr972144otr.3.1677196793869; Thu, 23 Feb 2023 15:59:53 -0800 (PST) In-Reply-To: Received-SPF: pass client-ip=2607:f8b0:4864:20::32b; envelope-from=joaotavora@gmail.com; helo=mail-ot1-x32b.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:303726 Archived-At: On Thu, Feb 23, 2023 at 10:19 PM Stefan Monnier wrote: > > > Mostly the fact that it's operating on a separate timer, but one > > that is directly correlated to jit-lock-context-time. > > But that's because you're willing to wait for the context refresh to do > your own. > > > So the bookkeeping and the coalescing of the small+large jit chunks > > should be provided by the jit infrastructure instead. > > So far, you're the first to need such a thing. In my experience the > needs for "jit display refresh" can be fairly subtly different, so it's > not clear how generally useful your approach would be. Maybe we could > provide some shared infrastructure to maintain a "coalescing set of > buffer regions", but if so, I suspect that it wouldn't need to be tied > to `jit-lock.el`. > > Also, I'm not sure it gives exactly the info you need/want: > I suspect that in some languages you can have: > > foo (x) > ... > function foo (bar : Int) Yeah, in that case we're frobbed. But isn't that a problem already those "some languages" for regular "contextual" fontification? > > And then no extra timer or logic would be needed. > > You mean you'd integrate it into `jit-lock-context`? > Maybe that could be done. > > Can we make jit-lock.el a :core ELPA package? > I have no opinion on that. That's the only reasonable way that Eglot can ever be changed to make use of new jit-lock infrastructure. Anyway, have a look at this patch: This implementation is much simpler than the one based on windows-scroll-functions. It's also conceptually safer, since jit-lock is guaranteed to refontify the regions that may need redisplaying. It's not trivially simple though, as simply adding 'eglot--update-hints-1' to jit-lock-functions, while possible, is going to request inlay hints from the LSP server for every small fraction changed by the user. So we do some bookkeeping on regions and coalesce them at a suitable time for a larger request. To do this, we use a jit-lock implementation detail, jit-lock-context-unfontify-pos, which tells us that the contextual fontification has just finished. Not sure how brittle it is, but it seems to work reasonably. * doc/misc/eglot.texi (Eglot Variables): Remove mention to deleted eglot-lazy-inlay-hints. * lisp/progmodes/eglot.el (eglot-lazy-inlay-hints) (eglot--inlay-hints-after-scroll) (eglot--inlay-hints-fully) (eglot--inlay-hints-lazily): Remove. (eglot--update-hints): Add function. (eglot-inlay-hints-mode): Simplify. --- diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el index eea8be6d1aa..9955b04574f 100644 --- a/lisp/progmodes/eglot.el +++ b/lisp/progmodes/eglot.el @@ -3489,32 +3489,38 @@ eglot-type-hint-face (defface eglot-parameter-hint-face '((t (:inherit eglot-inlay-hint-face))) "Face used for parameter inlay hint overlays.") -(defcustom eglot-lazy-inlay-hints 0.3 - "If non-nil, restrict LSP inlay hints to visible portion of the buffer. - -Value is a number specifying how many seconds to wait after a -window has been (re)scrolled before requesting new inlay hints -for the now-visible portion of the buffer shown in the window. - -If nil, then inlay hints are requested for the entire buffer. -This could be slow. - -This value is only meaningful if the minor mode -`eglot-inlay-hints-mode' is turned on in a buffer." - :type 'number - :version "29.1") - -(defun eglot--inlay-hints-fully () - (eglot--widening (eglot--update-hints-1 (point-min) (point-max)))) - -(cl-defun eglot--inlay-hints-lazily (&optional (buffer (current-buffer))) - (eglot--when-live-buffer buffer - (when eglot--managed-mode - (dolist (window (get-buffer-window-list nil nil 'visible)) - (eglot--update-hints-1 (window-start window) (window-end window)))))) +(defvar-local eglot--outstanding-inlay-regions nil + "List of regions that weren't yet \"contextually\" fontified.") + +(defun eglot--update-hints (from to) + "Jit-lock function for Eglot inlay hints." + ;; HACK: Comparing `jit-lock-context-unfontify-pos' to `point-max' + ;; is our way of telling whether this call to `jit-lock-functions' + ;; happens after `jit-lock-context-timer' has just run. This is the + ;; best time to coalesce the regions in + ;; `eglot--outstanding-inlay-regions' and contact the LSP server. Any + ;; other call to this function just adds another region to that list. + (if (= jit-lock-context-unfontify-pos (point-max)) + (let* ((sorted (sort (cons (cons from to) + eglot--outstanding-inlay-regions) + (pcase-lambda (`(,a1 . ,a2) `(,b1 . ,b2)) + (if (= a1 b1) (< a2 b2) (< a1 b1))))) + (coalesced (cl-reduce (lambda (acc x) + (cond ((and acc (>= (cdr (car acc)) + (car x))) + (setcdr (car acc) (cdr x)) + acc) + (t (push x acc) acc))) + sorted + :initial-value nil))) + (dolist (r coalesced) + (eglot--update-hints-1 (max (car r) (point-min)) + (min (cdr r) (point-max)))) + (setq eglot--outstanding-inlay-regions nil)) + (push (cons from to) eglot--outstanding-inlay-regions))) (defun eglot--update-hints-1 (from to) - "Request LSP inlay hints and annotate current buffer from FROM to TO." + "Do most work for `eglot--update-hints', including LSP request." (let* ((buf (current-buffer)) (paint-hint (eglot--lambda ((InlayHint) position kind label paddingLeft paddingRight) @@ -3545,67 +3551,16 @@ eglot--update-hints-1 (mapc paint-hint hints)))) :deferred 'eglot--update-hints-1))) -(defun eglot--inlay-hints-after-scroll (window display-start) - (cl-macrolet ((wsetq (sym val) `(set-window-parameter window ',sym ,val)) - (wgetq (sym) `(window-parameter window ',sym))) - (let ((buf (window-buffer window)) - (timer (wgetq eglot--inlay-hints-timer)) - (last-display-start (wgetq eglot--last-inlay-hint-display-start))) - (when (and eglot-lazy-inlay-hints - ;; FIXME: If `window' is _not_ the selected window, - ;; then for some unknown reason probably related to - ;; the overlays added later to the buffer, the scroll - ;; function will be called indefinitely. Not sure if - ;; an Emacs bug, but prevent useless duplicate calls - ;; by saving and examining `display-start' fixes it. - (not (eql last-display-start display-start))) - (when timer (cancel-timer timer)) - (wsetq eglot--last-inlay-hint-display-start - display-start) - (wsetq eglot--inlay-hints-timer - (run-at-time - eglot-lazy-inlay-hints - nil (lambda () - (eglot--when-live-buffer buf - (when (eq buf (window-buffer window)) - (eglot--update-hints-1 (window-start window) - (window-end window)) - (wsetq eglot--inlay-hints-timer nil)))))))))) - -(defun eglot--inlay-hints-after-window-config-change () - (eglot--update-hints-1 (window-start) (window-end))) - (define-minor-mode eglot-inlay-hints-mode "Minor mode for annotating buffers with LSP server's inlay hints." :global nil (cond (eglot-inlay-hints-mode - (cond - ((not (eglot--server-capable :inlayHintProvider)) + (if (eglot--server-capable :inlayHintProvider) + (jit-lock-register #'eglot--update-hints 'contextual) (eglot--warn - "No :inlayHintProvider support. Inlay hints will not work.")) - (eglot-lazy-inlay-hints - (add-hook 'eglot--document-changed-hook - #'eglot--inlay-hints-lazily t t) - (add-hook 'window-scroll-functions - #'eglot--inlay-hints-after-scroll nil t) - (add-hook 'window-configuration-change-hook - #'eglot--inlay-hints-after-window-config-change nil t) - ;; Maybe there isn't a window yet for current buffer, - ;; so `run-at-time' ensures this runs after redisplay. - (run-at-time 0 nil #'eglot--inlay-hints-lazily)) - (t - (add-hook 'eglot--document-changed-hook - #'eglot--inlay-hints-fully nil t) - (eglot--inlay-hints-fully)))) + "No :inlayHintProvider support. Inlay hints will not work."))) (t - (remove-hook 'window-configuration-change-hook - #'eglot--inlay-hints-after-window-config-change) - (remove-hook 'eglot--document-changed-hook - #'eglot--inlay-hints-lazily t) - (remove-hook 'eglot--document-changed-hook - #'eglot--inlay-hints-fully t) - (remove-hook 'window-scroll-functions - #'eglot--inlay-hints-after-scroll t) + (jit-lock-unregister #'eglot--update-hints) (remove-overlays nil nil 'eglot--inlay-hint t))))