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

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

Date: Mon, 16 Nov 2020 21:41:08 +0100

To: David Edmondson, notmuch@notmuchmail.org

Cc:

From: Jonas Bernoulli


David Edmondson <dme@dme.org> writes:

> 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?

I kept amending to this commit over time.  I suppose this difference
came about because apparently on some days such fuzzy comments bother
me more than others.

By the way, I am fairly strict about not sneaking unrelated changes
into and such comment cosmetic are about the only exception I make.

Anyway, I've amended this commit to improve two more comments.

jrb

>
> 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: