[PATCH v2 30/36] emacs: avoid unnecessary let-bindings

Subject: [PATCH v2 30/36] emacs: avoid unnecessary let-bindings

Date: Sun, 10 Jan 2021 15:01:06 +0100

To: notmuch@notmuchmail.org


From: Jonas Bernoulli

To some extend this is a personal preference, but the preference is
strongly dependent on whether one is used to a language that makes it
necessary to use variables like this.

This makes it perfectly clear that we are first getting and then using
a "foo":

  (use-foo (get-foo))

Sure this has to be read "inside out", but that's something one better
gets used to quickly when dealing with lisp.  I don't understand why
one would want to write this instead:

  (let ((the-foo (get-foo)))
    (use-foo the-foo))

Both `get-foo' and `use-foo' are named in a way that make it very
clear that we are dealing with a "foo".  Storing the value in an
additional variable `the-foo' does not make this any more clear.

On the contrary I makes the reader wonder why the author choose to
use a variable.  Is the value used more than once?  Is the value
being retrieved in one context and then used in another (e.g. when
the current buffer changes)?
 emacs/notmuch-address.el     |  4 +--
 emacs/notmuch-lib.el         |  6 ++---
 emacs/notmuch-maildir-fcc.el | 10 ++++----
 emacs/notmuch-show.el        | 14 +++++-----
 emacs/notmuch-tag.el         | 10 ++++----
 emacs/notmuch-tree.el        |  5 ++--
 emacs/notmuch.el             | 50 +++++++++++++++++-------------------
 7 files changed, 48 insertions(+), 51 deletions(-)

diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
index 1f22e377..f313c415 100644
--- a/emacs/notmuch-address.el
+++ b/emacs/notmuch-address.el
@@ -260,8 +260,8 @@ (defun notmuch-address-expand-name ()
 ;;; Harvest
 (defun notmuch-address-harvest-addr (result)
-  (let ((name-addr (plist-get result :name-addr)))
-    (puthash name-addr t notmuch-address-completions)))
+  (puthash (plist-get result :name-addr)
+	   t notmuch-address-completions))
 (defun notmuch-address-harvest-filter (proc string)
   (when (buffer-live-p (process-buffer proc))
diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 2fd9a27d..cbac8859 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -416,9 +416,9 @@ (defun notmuch-help ()
 its prefixed behavior by setting the 'notmuch-prefix-doc property
 of its command symbol."
-  (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))
diff --git a/emacs/notmuch-maildir-fcc.el b/emacs/notmuch-maildir-fcc.el
index efbb37f1..63e5514c 100644
--- a/emacs/notmuch-maildir-fcc.el
+++ b/emacs/notmuch-maildir-fcc.el
@@ -207,11 +207,11 @@ (defun notmuch-maildir-notmuch-insert-current-buffer (folder &optional create ta
 database in folder FOLDER. If CREATE is non-nil it will supply
 the --create-folder flag to create the folder if necessary. TAGS
 should be a list of tag changes to apply to the inserted message."
-  (let* ((args (append (and create (list "--create-folder"))
-		       (list (concat "--folder=" folder))
-		       tags)))
-    (apply 'notmuch-call-notmuch-process
-	   :stdin-string (buffer-string) "insert" args)))
+  (apply 'notmuch-call-notmuch-process
+	 :stdin-string (buffer-string) "insert"
+	 (append (and create (list "--create-folder"))
+		 (list (concat "--folder=" folder))
+		 tags)))
 (defun notmuch-maildir-fcc-with-notmuch-insert (fcc-header &optional create)
   "Store message with notmuch insert.
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 48374b38..27925669 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1666,13 +1666,13 @@ (defun notmuch-show-get-prop (prop &optional props)
 message in either tree or show. This means that several utility
 functions in notmuch-show can be used directly by notmuch-tree as
 they just need the correct message properties."
-  (let ((props (or props
-		   (cond ((eq major-mode 'notmuch-show-mode)
-			  (notmuch-show-get-message-properties))
-			 ((eq major-mode 'notmuch-tree-mode)
-			  (notmuch-tree-get-message-properties))
-			 (t nil)))))
-    (plist-get props prop)))
+  (plist-get (or props
+		 (cond ((eq major-mode 'notmuch-show-mode)
+			(notmuch-show-get-message-properties))
+		       ((eq major-mode 'notmuch-tree-mode)
+			(notmuch-tree-get-message-properties))
+		       (t nil)))
+	     prop))
 (defun notmuch-show-get-message-id (&optional bare)
   "Return an id: query for the Message-Id of the current message.
diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
index c006026c..3c958dd4 100644
--- a/emacs/notmuch-tag.el
+++ b/emacs/notmuch-tag.el
@@ -406,8 +406,9 @@ (defun notmuch-tag-completions (&rest search-terms)
    "\n+" t))
 (defun notmuch-select-tag-with-completion (prompt &rest search-terms)
-  (let ((tag-list (apply #'notmuch-tag-completions search-terms)))
-    (completing-read prompt tag-list nil nil nil 'notmuch-select-tag-history)))
+  (completing-read prompt
+		   (apply #'notmuch-tag-completions search-terms)
+		   nil nil nil 'notmuch-select-tag-history))
 (defun notmuch-read-tag-changes (current-tags &optional prompt initial-input)
   "Prompt for tag changes in the minibuffer.
@@ -455,10 +456,9 @@ (defun notmuch-update-tags (tags tag-changes)
 from TAGS if present."
   (let ((result-tags (copy-sequence tags)))
     (dolist (tag-change tag-changes)
-      (let ((op (aref tag-change 0))
-	    (tag (and (not (string= tag-change ""))
+      (let ((tag (and (not (string= tag-change ""))
 		      (substring tag-change 1))))
-	(cl-case op
+	(cl-case (aref tag-change 0)
 	  (?+ (unless (member tag result-tags)
 		(push tag result-tags)))
 	  (?- (setq result-tags (delete tag result-tags)))
diff --git a/emacs/notmuch-tree.el b/emacs/notmuch-tree.el
index a06afc2d..51a43edd 100644
--- a/emacs/notmuch-tree.el
+++ b/emacs/notmuch-tree.el
@@ -401,9 +401,8 @@ (defun notmuch-tree-set-prop (prop val &optional props)
     (notmuch-tree-set-message-properties props)))
 (defun notmuch-tree-get-prop (prop &optional props)
-  (let ((props (or props
-		   (notmuch-tree-get-message-properties))))
-    (plist-get props prop)))
+  (plist-get (or props (notmuch-tree-get-message-properties))
+	     prop))
 (defun notmuch-tree-set-tags (tags)
   "Set the tags of the current message."
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 40b730df..3436e1fc 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -521,17 +521,16 @@ (defun notmuch-search-show-thread (&optional elide-toggle)
 `notmuch-show-only-matching-messages' when displaying the
   (interactive "P")
-  (let ((thread-id (notmuch-search-find-thread-id))
-	(subject (notmuch-search-find-subject)))
-    (if (> (length thread-id) 0)
+  (let ((thread-id (notmuch-search-find-thread-id)))
+    (if thread-id
 	(notmuch-show thread-id
 		      ;; Name the buffer based on the subject.
-		      (concat "*"
-			      (truncate-string-to-width subject 30 nil nil t)
-			      "*"))
+		      (format "*%s*" (truncate-string-to-width
+				      (notmuch-search-find-subject)
+				      30 nil nil t)))
       (message "End of search results."))))
 (defun notmuch-tree-from-search-current-query ()
@@ -556,20 +555,21 @@ (defun notmuch-tree-from-search-thread ()
 (defun notmuch-search-reply-to-thread (&optional prompt-for-sender)
   "Begin composing a reply-all to the entire current thread in a new buffer."
   (interactive "P")
-  (let ((message-id (notmuch-search-find-thread-id)))
-    (notmuch-mua-new-reply message-id prompt-for-sender t)))
+  (notmuch-mua-new-reply (notmuch-search-find-thread-id)
+			 prompt-for-sender t))
 (defun notmuch-search-reply-to-thread-sender (&optional prompt-for-sender)
   "Begin composing a reply to the entire current thread in a new buffer."
   (interactive "P")
-  (let ((message-id (notmuch-search-find-thread-id)))
-    (notmuch-mua-new-reply message-id prompt-for-sender nil)))
+  (notmuch-mua-new-reply (notmuch-search-find-thread-id)
+			 prompt-for-sender nil))
 ;;; Tags
 (defun notmuch-search-set-tags (tags &optional pos)
-  (let ((new-result (plist-put (notmuch-search-get-result pos) :tags tags)))
-    (notmuch-search-update-result new-result pos)))
+  (notmuch-search-update-result
+   (plist-put (notmuch-search-get-result pos) :tags tags)
+   pos))
 (defun notmuch-search-get-tags (&optional pos)
   (plist-get (notmuch-search-get-result pos) :tags))
@@ -1013,10 +1013,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)
-    (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))
       (goto-char (point-min))
@@ -1045,13 +1044,12 @@ (defun notmuch-search-refresh-view ()
 thread. Otherwise, point will be moved to attempt to be in the
 same relative position within the new buffer."
-  (let ((target-line (line-number-at-pos))
-	(oldest-first notmuch-search-oldest-first)
-	(target-thread (notmuch-search-find-thread-id 'bare))
-	(query notmuch-search-query-string))
-    ;; notmuch-search erases the current buffer.
-    (notmuch-search query oldest-first target-thread target-line t)
-    (goto-char (point-min))))
+  (notmuch-search notmuch-search-query-string
+		  notmuch-search-oldest-first
+		  (notmuch-search-find-thread-id 'bare)
+		  (line-number-at-pos)
+		  t)
+  (goto-char (point-min)))
 (defun notmuch-search-toggle-order ()
   "Toggle the current search order.
@@ -1160,9 +1158,9 @@ (defun notmuch-search-imenu-extract-index-name-function ()
   "Return imenu name for line at point.
 Used as `imenu-extract-index-name-function' in notmuch buffers.
 Point should be at the beginning of the line."
-  (let ((subject (notmuch-search-find-subject))
-	(author (notmuch-search-find-authors)))
-    (format "%s (%s)" subject author)))
+  (format "%s (%s)"
+	  (notmuch-search-find-subject)
+	  (notmuch-search-find-authors)))
 ;;; _
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-leave@notmuchmail.org