Re: [PATCH 27/27] emacs: various cosmetic improvements

Subject: Re: [PATCH 27/27] emacs: various cosmetic improvements

Date: Sun, 15 Nov 2020 22:21:23 +0000

To: Jonas Bernoulli, notmuch@notmuchmail.org

Cc:

From: David Edmondson


On Sunday, 2020-11-08 at 20:03:11 +01, Jonas Bernoulli wrote:

There were some places here where you capitalised comments (more
generally, turned them into English prose) when they were moving for
$reasons, and others where you left them alone. Was there a rationale
for the different treatment?

Reviewed-by: David Edmondson <dme@dme.org>

> ---
>  emacs/notmuch-address.el | 31 ++++++--------
>  emacs/notmuch-hello.el   | 28 +++++--------
>  emacs/notmuch-lib.el     | 38 +++++++++--------
>  emacs/notmuch-mua.el     | 10 ++---
>  emacs/notmuch-tag.el     |  2 +-
>  emacs/notmuch.el         | 88 ++++++++++++++++++++--------------------
>  6 files changed, 91 insertions(+), 106 deletions(-)
>
> diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
> index 21d1d82f..0dedd5d5 100644
> --- a/emacs/notmuch-address.el
> +++ b/emacs/notmuch-address.el
> @@ -21,6 +21,8 @@
>  
>  ;;; Code:
>  
> +(eval-when-compile (require 'cl-lib))
> +
>  (require 'message)
>  (require 'notmuch-parser)
>  (require 'notmuch-lib)
> @@ -154,15 +156,12 @@ (defcustom notmuch-address-use-company t
>    :group 'notmuch-address)
>  
>  (defun notmuch-address-setup ()
> -  (let* ((setup-company (and notmuch-address-use-company
> -			     (require 'company nil t)))
> -	 (pair (cons notmuch-address-completion-headers-regexp
> -		     #'notmuch-address-expand-name)))
> -    (when setup-company
> -      (notmuch-company-setup))
> -    (unless (member pair message-completion-alist)
> -      (setq message-completion-alist
> -	    (push pair message-completion-alist)))))
> +  (when (and notmuch-address-use-company
> +	     (require 'company nil t))
> +    (notmuch-company-setup))
> +  (cl-pushnew (cons notmuch-address-completion-headers-regexp
> +		    #'notmuch-address-expand-name)
> +	      message-completion-alist :test #'equal))
>  
>  (defun notmuch-address-toggle-internal-completion ()
>    "Toggle use of internal completion for current buffer.
> @@ -251,11 +250,8 @@ (defun notmuch-address-expand-name ()
>     (t nil)))
>  
>  (defun notmuch-address-harvest-addr (result)
> -  (let ((name-addr (plist-get result :name-addr)))
> -    (puthash name-addr t notmuch-address-completions)))
> -
> -(defun notmuch-address-harvest-handle-result (obj)
> -  (notmuch-address-harvest-addr obj))
> +  (puthash (plist-get result :name-addr)
> +	   t notmuch-address-completions))
>  
>  (defun notmuch-address-harvest-filter (proc string)
>    (when (buffer-live-p (process-buffer proc))
> @@ -264,7 +260,7 @@ (defun notmuch-address-harvest-filter (proc string)
>  	(goto-char (point-max))
>  	(insert string))
>        (notmuch-sexp-parse-partial-list
> -       'notmuch-address-harvest-handle-result (process-buffer proc)))))
> +       'notmuch-address-harvest-addr (process-buffer proc)))))
>  
>  (defvar notmuch-address-harvest-procs '(nil . nil)
>    "The currently running harvests.
> @@ -375,7 +371,7 @@ (defun notmuch-address--load-address-hash ()
>  (defun notmuch-address--save-address-hash ()
>    (when notmuch-address-save-filename
>      (if (or (not (file-exists-p notmuch-address-save-filename))
> -	    ;; The file exists, check it is a file we saved
> +	    ;; The file exists, check it is a file we saved.
>  	    (notmuch-address--get-address-hash))
>  	(with-temp-file notmuch-address-save-filename
>  	  (let ((save-plist
> @@ -398,8 +394,7 @@ (defun notmuch-address-harvest-trigger ()
>         nil nil
>         (lambda (proc event)
>  	 ;; If harvest fails, we want to try
> -	 ;; again when the trigger is next
> -	 ;; called
> +	 ;; again when the trigger is next called.
>  	 (if (string= event "finished\n")
>  	     (progn
>  	       (notmuch-address--save-address-hash)
> diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
> index fa31694f..80af7544 100644
> --- a/emacs/notmuch-hello.el
> +++ b/emacs/notmuch-hello.el
> @@ -402,8 +402,7 @@ (defun notmuch-hello-add-saved-search (widget)
>      ;; If an existing saved search with this name exists, remove it.
>      (setq notmuch-saved-searches
>  	  (cl-loop for elem in notmuch-saved-searches
> -		   if (not (equal name
> -				  (notmuch-saved-search-get elem :name)))
> +		   unless (equal name (notmuch-saved-search-get elem :name))
>  		   collect elem))
>      ;; Add the new one.
>      (customize-save-variable 'notmuch-saved-searches
> @@ -446,18 +445,14 @@ (defun notmuch-hello-reflect (list ncols)
>  	     append (notmuch-hello-reflect-generate-row ncols nrows row list))))
>  
>  (defun notmuch-hello-widget-search (widget &rest ignore)
> -  (cond
> -   ((eq (widget-get widget :notmuch-search-type) 'tree)
> -    (notmuch-tree (widget-get widget
> -			      :notmuch-search-terms)))
> -   ((eq (widget-get widget :notmuch-search-type) 'unthreaded)
> -    (notmuch-unthreaded (widget-get widget
> -				    :notmuch-search-terms)))
> +  (cl-case (widget-get widget :notmuch-search-type)
> +   (tree
> +    (notmuch-tree (widget-get widget :notmuch-search-terms)))
> +   (unthreaded
> +    (notmuch-unthreaded (widget-get widget :notmuch-search-terms)))
>     (t
> -    (notmuch-search (widget-get widget
> -				:notmuch-search-terms)
> -		    (widget-get widget
> -				:notmuch-search-oldest-first)))))
> +    (notmuch-search (widget-get widget :notmuch-search-terms)
> +		    (widget-get widget :notmuch-search-oldest-first)))))
>  
>  (defun notmuch-saved-search-count (search)
>    (car (process-lines notmuch-command "count" search)))
> @@ -689,9 +684,7 @@ (define-derived-mode notmuch-hello-mode fundamental-mode "notmuch-hello"
>  Complete list of currently available key bindings:
>  
>  \\{notmuch-hello-mode-map}"
> -  (setq notmuch-buffer-refresh-function #'notmuch-hello-update)
> -  ;;(setq buffer-read-only t)
> -  )
> +  (setq notmuch-buffer-refresh-function #'notmuch-hello-update))
>  
>  (defun notmuch-hello-generate-tag-alist (&optional hide-tags)
>    "Return an alist from tags to queries to display in the all-tags section."
> @@ -775,8 +768,7 @@ (defun notmuch-hello-insert-search ()
>    ;; instead of a space to make `show-trailing-whitespace'
>    ;; happy, i.e. avoid it marking the whole line as trailing
>    ;; spaces.
> -  (widget-insert ".")
> -  (put-text-property (1- (point)) (point) 'invisible t)
> +  (widget-insert (propertize "." 'invisible t))
>    (widget-insert "\n"))
>  
>  (defun notmuch-hello-insert-recent-searches ()
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index e23999ad..aa48b949 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -186,8 +186,8 @@ (defun notmuch-command-to-string (&rest args)
>  
>  Otherwise the output will be returned."
>    (with-temp-buffer
> -    (let* ((status (apply #'call-process notmuch-command nil t nil args))
> -	   (output (buffer-string)))
> +    (let ((status (apply #'call-process notmuch-command nil t nil args))
> +	  (output (buffer-string)))
>        (notmuch-check-exit-status status (cons notmuch-command args) output)
>        output)))
>  
> @@ -239,8 +239,9 @@ (defun notmuch-config-get (item)
>    (let* ((val (notmuch-command-to-string "config" "get" item))
>  	 (len (length val)))
>      ;; Trim off the trailing newline (if the value is empty or not
> -    ;; configured, there will be no newline)
> -    (if (and (> len 0) (= (aref val (- len 1)) ?\n))
> +    ;; configured, there will be no newline).
> +    (if (and (> len 0)
> +	     (= (aref val (- len 1)) ?\n))
>  	(substring val 0 -1)
>        val)))
>  
> @@ -403,9 +404,9 @@ (defun notmuch-help ()
>  its prefixed behavior by setting the 'notmuch-prefix-doc property
>  of its command symbol."
>    (interactive)
> -  (let* ((mode major-mode)
> -	 (doc (substitute-command-keys
> -	       (notmuch-substitute-command-keys (documentation mode t)))))
> +  (let ((doc (substitute-command-keys
> +	      (notmuch-substitute-command-keys
> +	       (documentation major-mode t)))))
>      (with-current-buffer (generate-new-buffer "*notmuch-help*")
>        (insert doc)
>        (goto-char (point-min))
> @@ -467,8 +468,8 @@ (defun notmuch-refresh-all-buffers ()
>  	  (notmuch-refresh-this-buffer))))))
>  
>  (defun notmuch-prettify-subject (subject)
> -  ;; This function is used by `notmuch-search-process-filter' which
> -  ;; requires that we not disrupt its' matching state.
> +  ;; This function is used by `notmuch-search-process-filter',
> +  ;; which requires that we not disrupt its matching state.
>    (save-match-data
>      (if (and subject
>  	     (string-match "^[ \t]*$" subject))
> @@ -525,13 +526,12 @@ (defun notmuch-common-do-stash (text)
>  ;;
>  
>  (defun notmuch-plist-delete (plist property)
> -  (let* ((xplist (cons nil plist))
> -	 (pred xplist))
> -    (while (cdr pred)
> -      (when (eq (cadr pred) property)
> -	(setcdr pred (cdddr pred)))
> -      (setq pred (cddr pred)))
> -    (cdr xplist)))
> +  (let (p)
> +    (while plist
> +      (unless (eq property (car plist))
> +	(setq p (plist-put p (car plist) (cadr plist))))
> +      (setq plist (cddr plist)))
> +    p))
>  
>  (defun notmuch-match-content-type (t1 t2)
>    "Return t if t1 and t2 are matching content types, taking wildcards into account."
> @@ -540,8 +540,10 @@ (defun notmuch-match-content-type (t1 t2)
>      (if (or (string= (cadr st1) "*")
>  	    (string= (cadr st2) "*"))
>  	;; Comparison of content types should be case insensitive.
> -	(string= (downcase (car st1)) (downcase (car st2)))
> -      (string= (downcase t1) (downcase t2)))))
> +	(string= (downcase (car st1))
> +		 (downcase (car st2)))
> +      (string= (downcase t1)
> +	       (downcase t2)))))
>  
>  (defvar notmuch-multipart/alternative-discouraged
>    '(;; Avoid HTML parts.
> diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
> index f9843546..8501e294 100644
> --- a/emacs/notmuch-mua.el
> +++ b/emacs/notmuch-mua.el
> @@ -347,12 +347,10 @@ (defun notmuch-mua-pop-to-buffer (name switch-function)
>  		(select-window window))
>  	    (funcall switch-function buffer)
>  	    (set-buffer buffer))
> -	  (when (and (buffer-modified-p)
> -		     (not (prog1
> -			      (y-or-n-p
> -			       "Message already being composed; erase? ")
> -			    (message nil))))
> -	    (error "Message being composed")))
> +	  (when (buffer-modified-p)
> +	    (if (y-or-n-p "Message already being composed; erase? ")
> +		(message nil)
> +	      (error "Message being composed"))))
>        (funcall switch-function name)
>        (set-buffer name))
>      (erase-buffer)
> diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
> index 925de78c..75e864a4 100644
> --- a/emacs/notmuch-tag.el
> +++ b/emacs/notmuch-tag.el
> @@ -472,7 +472,7 @@ (defun notmuch-tag (query tag-changes)
>  	tag-changes)
>    (unless query
>      (error "Nothing to tag!"))
> -  (unless (null tag-changes)
> +  (when tag-changes
>      (run-hooks 'notmuch-before-tag-hook)
>      (if (<= (length query) notmuch-tag-argument-limit)
>  	(apply 'notmuch-call-notmuch-process "tag"
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index b221be05..8157bbf4 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -169,7 +169,7 @@ (defvar notmuch-search-mode-map
>    (let ((map (make-sparse-keymap)))
>      (set-keymap-parent map notmuch-common-keymap)
>      (define-key map "x" 'notmuch-bury-or-kill-this-buffer)
> -    (define-key map (kbd "<DEL>") 'notmuch-search-scroll-down)
> +    (define-key map (kbd "DEL") 'notmuch-search-scroll-down)
>      (define-key map "b" 'notmuch-search-scroll-down)
>      (define-key map " " 'notmuch-search-scroll-up)
>      (define-key map "<" 'notmuch-search-first-thread)
> @@ -923,40 +923,39 @@ (defun notmuch-read-query (prompt)
>    "Read a notmuch-query from the minibuffer with completion.
>  
>  PROMPT is the string to prompt with."
> -  (let*
> -      ((all-tags
> -	(mapcar (lambda (tag) (notmuch-escape-boolean-term tag))
> -		(process-lines notmuch-command "search" "--output=tags" "*")))
> -       (completions
> -	(append (list "folder:" "path:" "thread:" "id:" "date:" "from:" "to:"
> -		      "subject:" "attachment:")
> -		(mapcar (lambda (tag) (concat "tag:" tag)) all-tags)
> -		(mapcar (lambda (tag) (concat "is:" tag)) all-tags)
> -		(mapcar (lambda (mimetype) (concat "mimetype:" mimetype))
> -			(mailcap-mime-types)))))
> -    (let ((keymap (copy-keymap minibuffer-local-map))
> -	  (current-query (cl-case major-mode
> -			   (notmuch-search-mode (notmuch-search-get-query))
> -			   (notmuch-show-mode (notmuch-show-get-query))
> -			   (notmuch-tree-mode (notmuch-tree-get-query))))
> -	  (minibuffer-completion-table
> -	   (completion-table-dynamic
> -	    (lambda (string)
> -	      ;; generate a list of possible completions for the current input
> -	      (cond
> -	       ;; this ugly regexp is used to get the last word of the input
> -	       ;; possibly preceded by a '('
> -	       ((string-match "\\(^\\|.* (?\\)\\([^ ]*\\)$" string)
> -		(mapcar (lambda (compl)
> -			  (concat (match-string-no-properties 1 string) compl))
> -			(all-completions (match-string-no-properties 2 string)
> -					 completions)))
> -	       (t (list string)))))))
> -      ;; this was simpler than convincing completing-read to accept spaces:
> -      (define-key keymap (kbd "TAB") 'minibuffer-complete)
> -      (let ((history-delete-duplicates t))
> -	(read-from-minibuffer prompt nil keymap nil
> -			      'notmuch-search-history current-query nil)))))
> +  (let* ((all-tags
> +	  (mapcar (lambda (tag) (notmuch-escape-boolean-term tag))
> +		  (process-lines notmuch-command "search" "--output=tags" "*")))
> +	 (completions
> +	  (append (list "folder:" "path:" "thread:" "id:" "date:" "from:" "to:"
> +			"subject:" "attachment:")
> +		  (mapcar (lambda (tag) (concat "tag:" tag)) all-tags)
> +		  (mapcar (lambda (tag) (concat "is:" tag)) all-tags)
> +		  (mapcar (lambda (mimetype) (concat "mimetype:" mimetype))
> +			  (mailcap-mime-types))))
> +	 (keymap (copy-keymap minibuffer-local-map))
> +	 (current-query (cl-case major-mode
> +			  (notmuch-search-mode (notmuch-search-get-query))
> +			  (notmuch-show-mode (notmuch-show-get-query))
> +			  (notmuch-tree-mode (notmuch-tree-get-query))))
> +	 (minibuffer-completion-table
> +	  (completion-table-dynamic
> +	   (lambda (string)
> +	     ;; generate a list of possible completions for the current input
> +	     (cond
> +	      ;; this ugly regexp is used to get the last word of the input
> +	      ;; possibly preceded by a '('
> +	      ((string-match "\\(^\\|.* (?\\)\\([^ ]*\\)$" string)
> +	       (mapcar (lambda (compl)
> +			 (concat (match-string-no-properties 1 string) compl))
> +		       (all-completions (match-string-no-properties 2 string)
> +					completions)))
> +	      (t (list string)))))))
> +    ;; This was simpler than convincing completing-read to accept spaces:
> +    (define-key keymap (kbd "TAB") 'minibuffer-complete)
> +    (let ((history-delete-duplicates t))
> +      (read-from-minibuffer prompt nil keymap nil
> +			    'notmuch-search-history current-query nil))))
>  
>  (defun notmuch-search-get-query ()
>    "Return the current query in this search buffer."
> @@ -1006,10 +1005,9 @@ (defun notmuch-search (&optional query oldest-first target-thread target-line no
>      (setq notmuch-search-target-thread target-thread)
>      (setq notmuch-search-target-line target-line)
>      (notmuch-tag-clear-cache)
> -    (let ((proc (get-buffer-process (current-buffer)))
> -	  (inhibit-read-only t))
> -      (when proc
> -	(error "notmuch search process already running for query `%s'" query))
> +    (when (get-buffer-process buffer)
> +      (error "notmuch search process already running for query `%s'" query))
> +    (let ((inhibit-read-only t))
>        (erase-buffer)
>        (goto-char (point-min))
>        (save-excursion
> @@ -1019,12 +1017,12 @@ (defun notmuch-search (&optional query oldest-first target-thread target-line no
>  		     (if oldest-first
>  			 "--sort=oldest-first"
>  		       "--sort=newest-first")
> -		     query))
> -	      ;; Use a scratch buffer to accumulate partial output.
> -	      ;; This buffer will be killed by the sentinel, which
> -	      ;; should be called no matter how the process dies.
> -	      (parse-buf (generate-new-buffer " *notmuch search parse*")))
> -	  (process-put proc 'parse-buf parse-buf)
> +		     query)))
> +	  ;; Use a scratch buffer to accumulate partial output.
> +	  ;; This buffer will be killed by the sentinel, which
> +	  ;; should be called no matter how the process dies.
> +	  (process-put proc 'parse-buf
> +		       (generate-new-buffer " *notmuch search parse*"))
>  	  (set-process-filter proc 'notmuch-search-process-filter)
>  	  (set-process-query-on-exit-flag proc nil))))
>      (run-hooks 'notmuch-search-hook)))
> -- 
> 2.29.1
> _______________________________________________
> notmuch mailing list -- notmuch@notmuchmail.org
> To unsubscribe send an email to notmuch-leave@notmuchmail.org

dme.
-- 
I just bite it, it's for the look I don't light it.
_______________________________________________
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-leave@notmuchmail.org

Thread: