From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Philip Kaludercic Newsgroups: gmane.emacs.devel Subject: Re: [NonGNU ELPA] New package: eldoc-diffstat Date: Sat, 14 Dec 2024 10:11:53 +0000 Message-ID: <87wmg2shee.fsf@posteo.net> References: <87y10osimb.fsf@jklaehn.de> 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="33770"; mail-complaints-to="usenet@ciao.gmane.io" Cc: emacs-devel@gnu.org, Tassilo Horn To: Johann =?utf-8?Q?Kl=C3=A4hn?= Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Sat Dec 14 11:12:23 2024 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 1tMP8J-0008eU-LB for ged-emacs-devel@m.gmane-mx.org; Sat, 14 Dec 2024 11:12:23 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1tMP7x-0000Pb-KU; Sat, 14 Dec 2024 05:12:01 -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 1tMP7v-0000P2-AE for emacs-devel@gnu.org; Sat, 14 Dec 2024 05:11:59 -0500 Original-Received: from mout02.posteo.de ([185.67.36.66]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1tMP7s-0002iE-RD for emacs-devel@gnu.org; Sat, 14 Dec 2024 05:11:59 -0500 Original-Received: from submission (posteo.de [185.67.36.169]) by mout02.posteo.de (Postfix) with ESMTPS id 4911E240101 for ; Sat, 14 Dec 2024 11:11:54 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.net; s=2017; t=1734171114; bh=GJ515VnVYduKuPAKHYBYX0NgdJbrnudOrpAcpOnRZYo=; h=From:To:Cc:Subject:Autocrypt:OpenPGP:Date:Message-ID:MIME-Version: Content-Type:From; b=dlTAISaU3p9aJUu7a6liO2oSKwFtxgDh8ndNxmylruynvNYJCC01rD5sQAYHfjS/P ztBgAscBR03RvZO2+085DmHjpTJVy+VNKRTTMdNBTaB+r/8w50+CbEXayd9td23C1Y hSXWmgm0+QzPQpX3Y8Z+CViS06MCnC3dEDUAtDujybghHcwljVrn4SIP/a9IFu1WHC TDrWFhN4n6V4tFTQtbdBfIB3dpplRQbUCiKVGPVz9odbhNnF4AEdbh8QsyWgub4Lgz 7Led/ZJMGx+xDiFNBQ7gpOEM0WM5G2wwz3FLD0PV+2W+25jrnn+X8dviXjKDS3rN8L Vzny2ppvTDZHA== Original-Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4Y9MST4xXpz9rxG; Sat, 14 Dec 2024 11:11:53 +0100 (CET) In-Reply-To: <87y10osimb.fsf@jklaehn.de> ("Johann =?utf-8?Q?Kl=C3=A4hn=22'?= =?utf-8?Q?s?= message of "Mon, 09 Dec 2024 21:31:56 +0100") Autocrypt: addr=philipk@posteo.net; keydata= mDMEZBBQQhYJKwYBBAHaRw8BAQdAHJuofBrfqFh12uQu0Yi7mrl525F28eTmwUDflFNmdui0QlBo aWxpcCBLYWx1ZGVyY2ljIChnZW5lcmF0ZWQgYnkgYXV0b2NyeXB0LmVsKSA8cGhpbGlwa0Bwb3N0 ZW8ubmV0PoiWBBMWCAA+FiEEDg7HY17ghYlni8XN8xYDWXahwukFAmQQUEICGwMFCQHhM4AFCwkI BwIGFQoJCAsCBBYCAwECHgECF4AACgkQ8xYDWXahwulikAEA77hloUiSrXgFkUVJhlKBpLCHUjA0 mWZ9j9w5d08+jVwBAK6c4iGP7j+/PhbkxaEKa4V3MzIl7zJkcNNjHCXmvFcEuDgEZBBQQhIKKwYB BAGXVQEFAQEHQI5NLiLRjZy3OfSt1dhCmFyn+fN/QKELUYQetiaoe+MMAwEIB4h+BBgWCAAmFiEE Dg7HY17ghYlni8XN8xYDWXahwukFAmQQUEICGwwFCQHhM4AACgkQ8xYDWXahwukm+wEA8cml4JpK NeAu65rg+auKrPOP6TP/4YWRCTIvuYDm0joBALw98AMz7/qMHvSCeU/hw9PL6u6R2EScxtpKnWof z4oM OpenPGP: id=philipk@posteo.net; url="https://keys.openpgp.org/vks/v1/by-email/philipk@posteo.net"; preference=signencrypt Received-SPF: pass client-ip=185.67.36.66; envelope-from=philipk@posteo.net; helo=mout02.posteo.de X-Spam_score_int: -43 X-Spam_score: -4.4 X-Spam_bar: ---- X-Spam_report: (-4.4 / 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, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, RCVD_IN_VALIDITY_CERTIFIED_BLOCKED=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=unavailable 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:326475 Archived-At: --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Johann Kl=C3=A4hn writes: > I would like to submit a new package to NonGNU ELPA: eldoc-diffstat. > > https://github.com/kljohann/eldoc-diffstat/ I also have a few comments: --=-=-= Content-Type: text/plain Content-Disposition: inline diff --git a/eldoc-diffstat.el b/eldoc-diffstat.el index 6433e3e..5bbcd33 100644 --- a/eldoc-diffstat.el +++ b/eldoc-diffstat.el @@ -55,10 +55,10 @@ a property to the process object.") (defconst eldoc-diffstat--commands '((Git "git" "--no-pager" "show" "--color=always" - "--format=format:%an <%ae>, %aD:%n%s" "--stat=80") + "--format=format:%an <%ae>, %aD:%n%s" "--stat=80") (Hg "hg" "--pager=never" "log" "--color=always" - "--template" "{author}, {date|rfc822date}:\n{desc|firstline}\n" - "--stat" "--rev")) + "--template" "{author}, {date|rfc822date}:\n{desc|firstline}\n" + "--stat" "--rev")) "Alist mapping VCS backend to the command to use for computing the diffstat.") ;;;###autoload @@ -66,7 +66,7 @@ a property to the process object.") "Configure eldoc buffer-locally to display diffstat for revision at point." (interactive) (add-hook 'eldoc-documentation-functions - #'eldoc-diffstat--docstring nil 'local) + #'eldoc-diffstat--docstring nil 'local) (unless (bound-and-true-p eldoc-mode) (eldoc-mode))) @@ -74,11 +74,11 @@ a property to the process object.") "Display diffstat for revision at point by calling CALLBACK. Intended for `eldoc-documentation-functions'." (when-let* ((info (eldoc-diffstat--get-revision-info)) - (backend (car info)) - (revision (cdr info)) - (command (alist-get backend eldoc-diffstat--commands))) - (if-let ((result (eldoc-diffstat--get-cache info))) - (funcall callback result) + (backend (car info)) + (revision (cdr info)) + (command (alist-get backend eldoc-diffstat--commands))) + (if-let* ((result (eldoc-diffstat--get-cache info))) + (funcall callback result) (eldoc-diffstat--docstring-1 (append command (list revision)) callback info)))) (defun eldoc-diffstat--get-revision-info () @@ -87,34 +87,34 @@ The returned record should be a cons cell of the form (BACKEND . REVISION) where BACKEND is a symbol representing the version control system and REVISION is a string identifying the specific revision." (cond - ((when-let (((fboundp 'magit-stash-at-point)) - (revision (magit-stash-at-point))) + ((when-let* (((fboundp 'magit-stash-at-point)) + (revision (magit-stash-at-point))) (cons 'Git revision))) - ((when-let (((fboundp 'magit-commit-at-point)) - (revision (magit-commit-at-point))) + ((when-let* (((fboundp 'magit-commit-at-point)) + (revision (magit-commit-at-point))) (cons 'Git revision))) ((and (derived-mode-p 'git-rebase-mode) - (fboundp 'git-rebase-current-line) - (with-slots (action-type target) - (git-rebase-current-line) - (and (eq action-type 'commit) - (cons 'Git target))))) + (fboundp 'git-rebase-current-line) + (with-slots (action-type target) + (git-rebase-current-line) + (and (eq action-type 'commit) + (cons 'Git target))))) ((and (derived-mode-p 'vc-annotate-mode) - (boundp 'vc-annotate-backend) - (fboundp 'vc-annotate-extract-revision-at-line)) + (boundp 'vc-annotate-backend) + (fboundp 'vc-annotate-extract-revision-at-line)) (cons vc-annotate-backend - (car (vc-annotate-extract-revision-at-line)))) + (car (vc-annotate-extract-revision-at-line)))) ((and (derived-mode-p 'log-view-mode) - (boundp 'log-view-vc-backend)) + (boundp 'log-view-vc-backend)) (cons log-view-vc-backend - (log-view-current-tag))))) + (log-view-current-tag))))) (defun eldoc-diffstat--get-cache (revision-info) "Retrieve cached diffstat result for REVISION-INFO if available." (when-let* ((proc eldoc-diffstat--process) - ((processp proc)) - (cached-result (process-get proc :cached-result)) - ((equal revision-info (car cached-result)))) + ((processp proc)) + (cached-result (process-get proc :cached-result)) + ((equal revision-info (car cached-result)))) (cdr cached-result))) (defun eldoc-diffstat--docstring-1 (command callback revision-info) @@ -126,7 +126,7 @@ caching the result, see `eldoc-diffstat--get-cache' for details." (when (processp eldoc-diffstat--process) (when (process-live-p eldoc-diffstat--process) (let (confirm-kill-processes) - (kill-process eldoc-diffstat--process))) + (kill-process eldoc-diffstat--process))) (kill-buffer (process-buffer eldoc-diffstat--process))) (setq @@ -137,8 +137,9 @@ caching the result, see `eldoc-diffstat--get-cache' for details." :noquery t :command command :sentinel - (lambda (&rest args) - (apply #'eldoc-diffstat--sentinel callback args)))) + (apply-partially #'eldoc-diffstat--sentinel callback))) + ;; Is it an issue that there is the slight possibility of a race + ;; condition here? (process-put eldoc-diffstat--process :revision-info revision-info) ;; Signal that the doc string is computed asynchronously. @@ -150,41 +151,47 @@ caching the result, see `eldoc-diffstat--get-cache' for details." (with-current-buffer (process-buffer proc) (eldoc-diffstat--format-output-buffer) (let ((result (buffer-string)) - (revision-info (process-get eldoc-diffstat--process :revision-info))) - (process-put eldoc-diffstat--process :cached-result - (cons revision-info result)) - (funcall callback result))))) + (revision-info (process-get eldoc-diffstat--process :revision-info))) + (process-put eldoc-diffstat--process :cached-result + (cons revision-info result)) + (funcall callback result))))) (defun eldoc-diffstat--format-output-buffer () - "Format the diffstat output." + "Format the diffstat output in the current buffer." ;; Translate color control sequences into text properties. (let ((ansi-color-apply-face-function - (lambda (beg end face) - (put-text-property beg end 'face face)))) + (lambda (beg end face) + (put-text-property beg end 'face face)))) (ansi-color-apply-on-region (point-min) (point-max))) - ;; Delete trailing blank lines. - (goto-char (point-max)) - (delete-blank-lines) - - ;; Make first line bold. (goto-char (point-min)) - (put-text-property (point) - (line-end-position) - 'face 'bold) - - ;; Join second line. - (forward-line) - (join-line) - - ;; Move summary to the top and make it italic. - (forward-line) - (reverse-region (point) (point-max)) - (put-text-property (point) - (line-end-position) - 'face 'italic) - (forward-line) - (reverse-region (point) (point-max))) + (unless (looking-at + (rx buffer-start point + ;; Commit author and date: + (group (+ not-newline)) (group ?\n) + ;; First lie of the commit message + (group (+ not-newline)) (group ?\n) + ;; File changes + (group (+? anything)) + ;; Commit Summary + (group (+ not-newline)) + (* space) buffer-end)) + ;; If this occurs, then we know that the buffer doesn't look have + ;; the expected textual form! + (error "Malformed diffstat")) + + (put-text-property ;make first line bold + (match-beginning 1) (match-end 1) + 'face 'bold) + (replace-match " " nil nil nil 2) ;join the first two lines + (reverse-region ;reverse the diffstat order + (match-beginning 5) (match-end 5)) + (let ((summary (delete-and-extract-region + (match-beginning 6) + (match-end 6)))) + (replace-match ;move summary up and italicise + (concat "\n" (propertize summary 'face 'italic) "\n") + nil nil nil 4))) (provide 'eldoc-diffstat) ;;; eldoc-diffstat.el ends here --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable To summarise, when-let/if-let have been deprecated in Emacs 31 so it is best to stick to the star'ed variants. The rewriting of `eldoc-diffstat--format-output-buffer' might be controversial, but I feel that using a regular expression to destruct the buffer feels more robust. It would also be nice if you could add a .elpaignore file to exclude the screenshot from being bundled with the tarball. > It provides a way to display VCS diffstat information via eldoc. I.e., > if point rests on a commit in a magit, vc-annotate, or log-view buffer, > the Git or Mercurial diffstat summary will be shown in the echo area. > For example (though it uses ansi-colors and fontification, so you might > prefer to look at the screenshot in the repo): > > Johann Kl=C3=A4hn , Thu, 3 Oct 2024 15:45:10 +0200: Mino= r refactoring > 1 file changed, 93 insertions(+), 62 deletions(-) > eldoc-diffstat.el | 155 ++++++++++++++++++++++++++++++++--------------= -------- > > It's adapted from Tassilo Horn's 2022 blog post =E2=80=9CUsing Eldoc with= Magit > (asynchronously!)=E2=80=9D: https://www.tsdh.org/posts/2022-07-20-using-e= ldoc-with-magit-async.html > > Thanks > Johann --=-=-=--