From f47e3333b5478e43840e55710311aebdd441fc0e Mon Sep 17 00:00:00 2001 From: Jonas Bernoulli Date: Sun, 10 Jan 2021 15:01:06 +0100 Subject: [PATCH] emacs: avoid unnecessary let-bindings 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 @@ requiring external commands." ;;; 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 @@ A command that supports a prefix argument can explicitly document 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)) diff --git a/emacs/notmuch-maildir-fcc.el b/emacs/notmuch-maildir-fcc.el index c635284e..9a6365ba 100644 --- a/emacs/notmuch-maildir-fcc.el +++ b/emacs/notmuch-maildir-fcc.el @@ -207,11 +207,11 @@ This inserts the current buffer as a message into the notmuch 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 @@ It gets property PROP from PROPS or, if PROPS is nil, the current 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 @@ Return all tags if no search terms are given." "\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 @@ present or a \"-\" to indicate that the tag should be removed 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 @@ Some useful entries are: (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 @@ With a prefix argument, invert the default value of `notmuch-show-only-matching-messages' when displaying the thread." (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 elide-toggle (current-buffer) notmuch-search-query-string ;; 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 @@ 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 @@ the configured default sort order." (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 @@ -1045,13 +1044,12 @@ the new search results, then point will be placed on the same thread. Otherwise, point will be moved to attempt to be in the same relative position within the new buffer." (interactive) - (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 @@ Used as`imenu-prev-index-position-function' in notmuch buffers." "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))) ;;; _ -- 2.43.0