From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Danny Freeman via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs Subject: bug#59149: [SPAM UNSURE] Re: bug#59149: Feature Request: Report progress of long requests in Eglot Date: Sat, 26 Nov 2022 13:37:20 -0500 Message-ID: <87v8n18sdz.fsf@dfreeman.email> References: <87educqkar.fsf@dfreeman.email> <86cz9jmg9r.fsf@stephe-leake.org> <87k03qvmla.fsf@dfreeman.email> <86fseckwvb.fsf@stephe-leake.org> <87tu2pohub.fsf@dfreeman.email> <86ilj5k0sn.fsf@stephe-leake.org> <87wn7l316m.fsf@dfreeman.email> <86edtsk3xi.fsf@stephe-leake.org> <87o7swv3nx.fsf@gmail.com> <8635a8jb8e.fsf@stephe-leake.org> <87r0xrxb0i.fsf@dfreeman.email> <834junuhae.fsf@gnu.org> <87ilj3x9we.fsf@dfreeman.email> <87r0xqsf0y.fsf@gmail.com> Reply-To: Danny Freeman Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="30924"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 59149@debbugs.gnu.org, Eli Zaretskii , stephen_leake@stephe-leake.org, stefankangas@gmail.com To: =?UTF-8?Q?Jo=C3=A3o_?= =?UTF-8?Q?T=C3=A1vora?= Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Nov 26 19:48:24 2022 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1oz0Dw-0007q2-1c for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 26 Nov 2022 19:48:24 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oz0Db-0001kP-46; Sat, 26 Nov 2022 13:48: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 1oz0Da-0001kB-CN for bug-gnu-emacs@gnu.org; Sat, 26 Nov 2022 13:48:02 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1oz0Da-0008E4-3N for bug-gnu-emacs@gnu.org; Sat, 26 Nov 2022 13:48:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1oz0Da-0003gX-0F for bug-gnu-emacs@gnu.org; Sat, 26 Nov 2022 13:48:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Danny Freeman Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 26 Nov 2022 18:48:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 59149 X-GNU-PR-Package: emacs Original-Received: via spool by 59149-submit@debbugs.gnu.org id=B59149.166948846614087 (code B ref 59149); Sat, 26 Nov 2022 18:48:01 +0000 Original-Received: (at 59149) by debbugs.gnu.org; 26 Nov 2022 18:47:46 +0000 Original-Received: from localhost ([127.0.0.1]:41421 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oz0DJ-0003f5-1w for submit@debbugs.gnu.org; Sat, 26 Nov 2022 13:47:46 -0500 Original-Received: from out2.migadu.com ([188.165.223.204]:43647) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oz0DG-0003ee-42 for 59149@debbugs.gnu.org; Sat, 26 Nov 2022 13:47:43 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dfreeman.email; s=key1; t=1669488460; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=v755Wi8zsBMrr5yp+sqYs7PJ4p1e2jsh9l+egRdKupA=; b=mhbW+x7q4U7o4Q41BRAqJE91frD3xbxrEqQ1gDGAhyMSuIdJwRucHoK+JJ4eG/5vogy+DD FZUJ0bNPzvRnLKHnLc1c8pYoul21iOh2H2K64K5M/ytOAqx03pZwBI/FQZh57g9d6tDX4m kDk2QiLPJ1mnU8kY703wVPyQW2Souw8= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. In-reply-to: <87r0xqsf0y.fsf@gmail.com> X-Migadu-Flow: FLOW_OUT X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:249126 Archived-At: --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Jo=C3=A3o T=C3=A1vora writes: > Danny Freeman writes: > > >> +(defcustom eglot-report-progress t >> + "If non-nil, show progress of long running server work in the minibuf= fer." >> + :type 'boolean >> + :version "29.1") > > The usual question: is the version number here Eglot, the ELPA package's > upcoming version number, or is it Emacs's upcoming version number. I > think Stefan Kangas had something to say about that. Yeah, I've left this as in my updated patch. The answer is beyond my qualifications :) > >> + >> + (message-log-max nil)) > > I know that indentation was off here, and you fixed it. But please > revert this, and feel free to offer a separate cosmetic patch fixing > this and other violations. No problem. I have attached a second patch with only whitespace changes. Feel free to take it or leave it.=20 > >> (ignore-errors (delay-mode-hooks (funcall mode)))) >> (font-lock-ensure) >> (string-trim (buffer-string))))) >> @@ -2049,6 +2057,43 @@ eglot-handle-notification >> (_server (_method (eql telemetry/event)) &rest _any) >> "Handle notification telemetry/event.") ;; noop, use events buffer >>=20=20 >> +(defun eglot--progress-report-message (title message) >> + "Format a $/progress report message, given a TITLE and/or MESSAGE str= ing." >> + (cond >> + ((and title message) (format "%s %s" title message)) >> + (title title) >> + (message message))) >> + >> +(defun eglot--progress-reporter (server token) >> + "Get a prgress-reporter identified by the progress TOKEN from the SER= VER ." >> + (cdr (assoc token (eglot--progress-reporter-alist server)))) >> + >> +(defun eglot--progress-reporter-delete (server token) >> + "Delete progress-reporters identified by the progress TOKEN from the = SERVER." >> + (setf (eglot--progress-reporter-alist server) >> + (assoc-delete-all token (eglot--progress-reporter-alist > serve= r)))) > > This is just a stylistic suggestion, but these functions could all be > local inside the following eglot-handle-notification. Then you could > give them less mouthfully names. The whole progress stuff could be > examined by looking only at one function. I have inlined them with cl-flet. >> + >> +(cl-defmethod eglot-handle-notification >> + (server (_method (eql $/progress)) &key token value) >> + "Handle a $/progress notification identified by TOKEN from the SERVER= ." >> + (when eglot-report-progress >> + (cl-destructuring-bind (&key kind title percentage message) value > > I think eglot-dbind is more appropriate here. See the file for how it's > used. Done, that's a pretty neat macro. >> + (pcase kind >> + ("begin" (let* ((prefix (format (concat "[eglot] %s %s:" (when = percentage " ")) >> + (eglot-project-nickname server)= token)) >> + (pr (if percentage >> + (make-progress-reporter prefix 0 100 pe= rcentage 1 0) >> + (make-progress-reporter prefix nil nil ni= l 1 0)))) >> + (eglot--progress-reporter-delete server token) >> + (setf (eglot--progress-reporter-alist server) >> + (cons (cons token pr) (eglot--progress-reporte= r-alist server))) >> + (progress-reporter-update pr percentage (eglot--prog= ress-report-message title message)))) >> + ("report" (when-let ((pr (eglot--progress-reporter server token= ))) >> + (progress-reporter-update pr percentage (eglot--pro= gress-report-message title message)))) >> + ("end" (when-let ((pr (eglot--progress-reporter server token))) >> + (progress-reporter-done pr) >> + (eglot--progress-reporter-delete server token))))))) > > This lines are a bit too long, I think. Try to stick to 80 columns. > M-x whitespace-mode helps in seeing that. I've scaled back the width of the function a good bit. Some of the parens still spill over in some places, happy to take a second stab at it. > I also think this could probably be simplified a bit or jumbled around > to feel less repetitive. But it's not really bad as it stands, so feel > free to disregard. I'm not sure what I could do to scale this back beyond what is there now. If there is anything specific let me know. >> + >> (cl-defmethod eglot-handle-notification >> (_server (_method (eql textDocument/publishDiagnostics)) &key uri dia= gnostics >> &allow-other-keys) ; FIXME: doesn't respect `eglot-strict-mo= de' >> @@ -2172,7 +2217,7 @@ eglot--TextDocumentItem >> (append >> (eglot--VersionedTextDocumentIdentifier) >> (list :languageId >> - (eglot--language-id (eglot--current-server-or-lose)) >> + (eglot--language-id (eglot--current-server-or-lose)) > > Same here re: indentation. This was one of the lines that had a tab, I was more intentional with the indentation in the whitespace patch. >> :text >> (eglot--widening >> (buffer-substring-no-properties (point-min) (point-max)))))) > > > The patch looks generally good: thanks! If you want to do the fixes I > suggested, go ahead. Else give me some days to test it and I will > push it. > > Jo=C3=A3o Thank you! Let me know if you have any other questions or feedback and I will be happy to address it. --=20 Danny Freeman --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=0001-eglot-whitespace-cleanup.patch Content-Description: whitespace only patch >From 7227f9fdf53a1af7c3793c94904e153d49527372 Mon Sep 17 00:00:00 2001 From: dannyfreeman Date: Sat, 26 Nov 2022 13:31:50 -0500 Subject: [PATCH 1/2] ; eglot whitespace cleanup * lisp/progmodes/eglot (eglot--check-object): trailing whitespace. (eglot--format-markup): replace tab with spaces. (eglot--make-diag): trailing whitespace. (eglot--TextDocumentItem): replace tab with spaces. (eglot--lsp-xrefs-for-method): trailing whitespace. --- lisp/progmodes/eglot.el | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el index 120d4feb95..e6e97fd8c8 100644 --- a/lisp/progmodes/eglot.el +++ b/lisp/progmodes/eglot.el @@ -540,7 +540,7 @@ eglot--check-object for type = (or (cdr (assoc k types)) t) ;; FIXME: enforce nil type? unless (cl-typep v type) do (eglot--error "A `%s' must have a %s as %s, but has %s" - interface-name ))) + interface-name))) t)) (eval-and-compile @@ -1569,7 +1569,7 @@ eglot--format-markup (setq-local markdown-fontify-code-blocks-natively t) (insert string) (let ((inhibit-message t) - (message-log-max nil)) + (message-log-max nil)) (ignore-errors (delay-mode-hooks (funcall mode)))) (font-lock-ensure) (string-trim (buffer-string))))) @@ -1987,7 +1987,7 @@ 'eglot--make-diag (defalias 'eglot--diag-data 'flymake-diagnostic-data) (cl-loop for i from 1 - for type in '(eglot-note eglot-warning eglot-error ) + for type in '(eglot-note eglot-warning eglot-error) do (put type 'flymake-overlay-control `((mouse-face . highlight) (priority . ,(+ 50 i)) @@ -2172,7 +2172,7 @@ eglot--TextDocumentItem (append (eglot--VersionedTextDocumentIdentifier) (list :languageId - (eglot--language-id (eglot--current-server-or-lose)) + (eglot--language-id (eglot--current-server-or-lose)) :text (eglot--widening (buffer-substring-no-properties (point-min) (point-max)))))) @@ -2641,7 +2641,7 @@ eglot--lsp-xrefs-for-method uri range)))))) (if (vectorp response) response (and response (list response))))))) -(cl-defun eglot--lsp-xref-helper (method &key extra-params capability ) +(cl-defun eglot--lsp-xref-helper (method &key extra-params capability) "Helper for `eglot-find-declaration' & friends." (let ((eglot--lsp-xref-refs (eglot--lsp-xrefs-for-method method -- 2.38.1 --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=0002-Eglot-Display-progress-notifications-in-minibuffer-a.patch Content-Description: updated progress reporter patch >From a4d7c5028d49cd5fd6d5599e91d2bcfb250851dc Mon Sep 17 00:00:00 2001 From: dannyfreeman Date: Wed, 9 Nov 2022 08:46:45 -0500 Subject: [PATCH 2/2] Eglot: Display progress notifications in minibuffer as they arrive * lisp/progmodes/eglot.el (eglot-report-progress): New custom variable. (eglot-lsp-server): New slot for tracking active progress reporters. (eglot-handle-notification): New impl for $/progress server responses. The LSP spec describes methods for reporting progress on long running jobs to the client: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#progress https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#workDoneProgress This change reports those notifications in the minibuffer as they come in. It shows a percent indicator (if the server provides theme), or a spinner. This change should open the door for writing a "cancel long running request" command, which are identified by these progress notifications. See https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#window_workDoneProgress_cancel --- lisp/progmodes/eglot.el | 50 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el index e6e97fd8c8..e6673c37a1 100644 --- a/lisp/progmodes/eglot.el +++ b/lisp/progmodes/eglot.el @@ -386,6 +386,11 @@ eglot-menu-string "String displayed in mode line when Eglot is active." :type 'string) +(defcustom eglot-report-progress t + "If non-nil, show progress of long running server work in the minibuffer." + :type 'boolean + :version "29.1") + (defvar eglot-withhold-process-id nil "If non-nil, Eglot will not send the Emacs process id to the language server. This can be useful when using docker to run a language server.") @@ -470,6 +475,7 @@ eglot--executable-find (TextDocumentEdit (:textDocument :edits) ()) (TextEdit (:range :newText)) (VersionedTextDocumentIdentifier (:uri :version) ()) + (WorkDoneProgress (:kind) (:title :message :percentage :cancellable)) (WorkspaceEdit () (:changes :documentChanges)) (WorkspaceSymbol (:name :kind) (:containerName :location :data))) "Alist (INTERFACE-NAME . INTERFACE) of known external LSP interfaces. @@ -831,6 +837,9 @@ eglot-lsp-server (project :documentation "Project associated with server." :accessor eglot--project) + (progress-reporter-alist + :documentation "Alist of (PROGRESS-TOKEN . PROGRESS-REPORTER)." + :accessor eglot--progress-reporter-alist) (inhibit-autoreconnect :initform t :documentation "Generalized boolean inhibiting auto-reconnection if true." @@ -2049,6 +2058,47 @@ eglot-handle-notification (_server (_method (eql telemetry/event)) &rest _any) "Handle notification telemetry/event.") ;; noop, use events buffer +(cl-defmethod eglot-handle-notification + (server (_method (eql $/progress)) &key token value) + "Handle a $/progress notification identified by TOKEN from the SERVER." + (when eglot-report-progress + (cl-flet ((eglot--pr-message (title message) + (cond + ((and title message) (format "%s %s" title message)) + (title title) + (message message))) + (eglot--get-pr (server token) + (cdr (assoc token (eglot--progress-reporter-alist server)))) + (eglot--delete-pr (server token) + (setf (eglot--progress-reporter-alist server) + (assoc-delete-all + token (eglot--progress-reporter-alist server))))) + (eglot--dbind ((WorkDoneProgress) kind title percentage message) value + (pcase kind + ("begin" + (let* ((prefix (format (concat "[eglot] %s %s:" + (when percentage " ")) + (eglot-project-nickname server) + token)) + (pr (if percentage + (make-progress-reporter + prefix 0 100 percentage 1 0) + (make-progress-reporter + prefix nil nil nil 1 0)))) + (eglot--delete-pr server token) + (setf (eglot--progress-reporter-alist server) + (cons (cons token pr) (eglot--progress-reporter-alist server))) + (progress-reporter-update + pr percentage (eglot--pr-message title message)))) + ("report" + (when-let ((pr (eglot--get-pr server token))) + (progress-reporter-update + pr percentage (eglot--pr-message title message)))) + ("end" + (when-let ((pr (eglot--get-pr server token))) + (progress-reporter-done pr) + (eglot--delete-pr server token)))))))) + (cl-defmethod eglot-handle-notification (_server (_method (eql textDocument/publishDiagnostics)) &key uri diagnostics &allow-other-keys) ; FIXME: doesn't respect `eglot-strict-mode' -- 2.38.1 --=-=-=--