* Re: [NonGNU ELPA] New package: eldoc-diffstat
2024-12-09 20:31 [NonGNU ELPA] New package: eldoc-diffstat Johann Klähn
2024-12-13 20:23 ` Tassilo Horn
@ 2024-12-14 10:11 ` Philip Kaludercic
1 sibling, 0 replies; 4+ messages in thread
From: Philip Kaludercic @ 2024-12-14 10:11 UTC (permalink / raw)
To: Johann Klähn; +Cc: emacs-devel, Tassilo Horn
[-- Attachment #1: Type: text/plain, Size: 202 bytes --]
Johann Klähn <johann@jklaehn.de> 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:
[-- Attachment #2: Type: text/plain, Size: 7786 bytes --]
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
[-- Attachment #3: Type: text/plain, Size: 1189 bytes --]
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ähn <xyz@localhost>, Thu, 3 Oct 2024 15:45:10 +0200: Minor refactoring
> 1 file changed, 93 insertions(+), 62 deletions(-)
> eldoc-diffstat.el | 155 ++++++++++++++++++++++++++++++++----------------------
>
> It's adapted from Tassilo Horn's 2022 blog post “Using Eldoc with Magit
> (asynchronously!)”: https://www.tsdh.org/posts/2022-07-20-using-eldoc-with-magit-async.html
>
> Thanks
> Johann
^ permalink raw reply related [flat|nested] 4+ messages in thread