From: "João Távora" <joaotavora@gmail.com>
To: Danny Freeman <danny@dfreeman.email>
Cc: 59149@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org>,
stephen_leake@stephe-leake.org, stefankangas@gmail.com
Subject: bug#59149: [SPAM UNSURE] Re: bug#59149: Feature Request: Report progress of long requests in Eglot
Date: Sat, 26 Nov 2022 01:03:41 +0000 [thread overview]
Message-ID: <87r0xqsf0y.fsf@gmail.com> (raw)
In-Reply-To: <87ilj3x9we.fsf@dfreeman.email> (Danny Freeman's message of "Fri, 25 Nov 2022 11:41:46 -0500")
Danny Freeman <danny@dfreeman.email> writes:
> +(defcustom eglot-report-progress t
> + "If non-nil, show progress of long running server work in the minibuffer."
> + :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.
> +
> + (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.
> (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
>
> +(defun eglot--progress-report-message (title message)
> + "Format a $/progress report message, given a TITLE and/or MESSAGE string."
> + (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 SERVER ."
> + (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 > server))))
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.
> +
> +(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.
> + (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--progress-reporter-delete server token)
> + (setf (eglot--progress-reporter-alist server)
> + (cons (cons token pr) (eglot--progress-reporter-alist server)))
> + (progress-reporter-update pr percentage (eglot--progress-report-message title message))))
> + ("report" (when-let ((pr (eglot--progress-reporter server token)))
> + (progress-reporter-update pr percentage (eglot--progress-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 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.
> +
> (cl-defmethod eglot-handle-notification
> (_server (_method (eql textDocument/publishDiagnostics)) &key uri diagnostics
> &allow-other-keys) ; FIXME: doesn't respect `eglot-strict-mode'
> @@ -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.
> :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ão
next prev parent reply other threads:[~2022-11-26 1:03 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-09 14:13 bug#59149: Feature Request: Report progress of long requests in Eglot Danny Freeman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-11-10 15:50 ` João Távora
2022-11-11 13:07 ` Danny Freeman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-11-19 9:42 ` Stephen Leake
2022-11-19 18:03 ` Danny Freeman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-11-21 18:04 ` Stephen Leake
2022-11-23 14:12 ` Danny Freeman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-11-23 18:01 ` Stephen Leake
2022-11-23 19:36 ` Danny Freeman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-11-23 19:56 ` João Távora
2022-11-24 11:06 ` bug#59149: [SPAM UNSURE] " Stephen Leake
2022-11-24 14:16 ` João Távora
2022-11-24 21:25 ` Stephen Leake
2022-11-25 16:11 ` João Távora
2022-11-25 16:15 ` Danny Freeman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-11-25 16:31 ` Eli Zaretskii
2022-11-25 16:41 ` Danny Freeman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-11-25 16:44 ` Danny Freeman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-11-26 1:03 ` João Távora [this message]
2022-11-26 18:37 ` Danny Freeman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-11-26 19:46 ` Stefan Kangas
2022-12-01 13:29 ` Danny Freeman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-03 13:23 ` João Távora
2022-12-09 13:06 ` João Távora
2022-12-09 13:38 ` Danny Freeman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-11-22 18:45 ` Stephen Leake
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87r0xqsf0y.fsf@gmail.com \
--to=joaotavora@gmail.com \
--cc=59149@debbugs.gnu.org \
--cc=danny@dfreeman.email \
--cc=eliz@gnu.org \
--cc=stefankangas@gmail.com \
--cc=stephen_leake@stephe-leake.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.