[PATCH] emacs: replace setq + let with let*

[David Bremner]

Mark Walters  writes:

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

I ended up dropping this hunk because I realized it introduced a bug;
the side-effect of the setq is needed outside the unless.

pushed the two patches (as amended) in this thread.

d

[PATCH] emacs: replace setq + let with let*

[Mark Walters]

On Sun, 02 Jun 2013, david at tethera.net wrote:
> From: David Bremner 
>
> 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 ( 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 at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] emacs: replace setq + let with let*

[da...@tethera.net]

From: David Bremner 

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 ( 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
  #'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

[PATCH] emacs: replace setq + let with let*

[david]

From: David Bremner brem...@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
  #'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

Re: [PATCH] emacs: replace setq + let with let*

[Mark Walters]

On Sun, 02 Jun 2013, da...@tethera.net wrote:
 From: David Bremner brem...@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
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] emacs: replace setq + let with let*

[David Bremner]

Mark Walters markwalters1...@gmail.com writes:

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

I ended up dropping this hunk because I realized it introduced a bug;
the side-effect of the setq is needed outside the unless.

pushed the two patches (as amended) in this thread.

d



___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch