[PATCH] emacs: replace setq + let with let*
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*
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 (&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 at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] emacs: replace setq + let with let*
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 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] emacs: replace setq + let with let*
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 (&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
Re: [PATCH] emacs: replace setq + let with let*
On Sun, 02 Jun 2013, da...@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 (&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
[PATCH] emacs: replace setq + let with let*
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 (&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