From: Mark Walters <markwalters1009@gmail.com>
To: david@tethera.net, notmuch@notmuchmail.org
Subject: Re: [PATCH] emacs: replace setq + let with let*
In-Reply-To: <1370185351-23834-1-git-send-email-david@tethera.net>
References: <1370175370-13561-1-git-send-email-david@tethera.net>
	<1370185351-23834-1-git-send-email-david@tethera.net>
User-Agent: Notmuch/0.14+255~gff3cc55 (http://notmuchmail.org) Emacs/23.4.1
	(i486-pc-linux-gnu)
Date: Sun, 02 Jun 2013 18:25:36 +0100
Message-ID: <878v2sv2zz.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: David Bremner <bremner@debian.org>
Precedence: list


On Sun, 02 Jun 2013, david@tethera.net wrote:
> From: David Bremner <bremner@debian.org>
>
> I found several places where a setq is immediately followed by a let
> or a let*. This seems to be the pessimal combination, with the
> implicit scope of the setq combined with the extra indentation of the let.
> I combined these cases into a single let* which I think is easier to read.
> In two places I turned a single clause let into a let*.
> ---
>  emacs/notmuch-hello.el | 4 ++--
>  emacs/notmuch-show.el  | 4 ++--
>  emacs/notmuch.el       | 5 ++---
>  3 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
> index c1c6f4b..15e3614 100644
> --- a/emacs/notmuch-hello.el
> +++ b/emacs/notmuch-hello.el
> @@ -260,8 +260,8 @@ afterwards.")
>  (defun notmuch-hello-search (&optional search)
>    (interactive)
>    (unless (null search)
> -    (setq search (notmuch-hello-trim search))
> -    (let ((history-delete-duplicates t))
> +    (let* ((search (notmuch-hello-trim search))
> +	   (history-delete-duplicates t))
>        (add-to-history 'notmuch-search-history search)))
>    (notmuch-search search notmuch-search-oldest-first nil nil

These look good to me except I don't see why the above is a let* not a
let?

Best wishes

Mark

>  		  #'notmuch-hello-search-continuation))
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 18b4671..e8c8343 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -1734,8 +1734,8 @@ TAG-CHANGES is a list of tag operations for `notmuch-tag'."
>  
>  See `notmuch-tag' for information on the format of TAG-CHANGES."
>    (interactive)
> -  (setq tag-changes (notmuch-tag (notmuch-show-get-message-id) tag-changes))
> -  (let* ((current-tags (notmuch-show-get-tags))
> +  (let* ((tag-changes (notmuch-tag (notmuch-show-get-message-id) tag-changes))
> +	 (current-tags (notmuch-show-get-tags))
>  	 (new-tags (notmuch-update-tags current-tags tag-changes)))
>      (unless (equal current-tags new-tags)
>        (notmuch-show-set-tags new-tags))))
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index af107e2..edb5a1c 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -904,9 +904,8 @@ Other optional parameters are used as follows:
>    target-line: The line number to move to if the target thread does not
>                 appear in the search results."
>    (interactive)
> -  (if (null query)
> -      (setq query (notmuch-read-query "Notmuch search: ")))
> -  (let ((buffer (get-buffer-create (notmuch-search-buffer-title query))))
> +  (let* ((query (or query (notmuch-read-query "Notmuch search: ")))
> +	 (buffer (get-buffer-create (notmuch-search-buffer-title query))))
>      (switch-to-buffer buffer)
>      (notmuch-search-mode)
>      ;; Don't track undo information for this buffer
> -- 
> 1.8.2.rc2
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch