[PATCH v2 4/6] emacs: add optional predicate arg to `notmuch-show-mapc'
Mark Walters writes: > The original function feels a little fragile to me as to what happens if > predicate or function move point. Eg what happens if function collapses > the message: where does point go, and so where does > notmuch-show-goto-message-next go. Is this just my naivete as a lisp > beginner? Is there someway of writing it so the user doesn't need to > worry about such things? Although collapsing the message doesn't seem to move point, it would probably be a good idea to wrap the calls to predicate and function in save-excursion, as a guard against subtle and hard-to-spot bugs with operations not being applied to all the right messages.. Ethan
[PATCH v2 4/6] emacs: add optional predicate arg to `notmuch-show-mapc'
Pieter Praet writes: > * emacs/notmuch-show.el (notmuch-show-mapc): > > If provided with optional argument PREDICATE, only call > FUNCTION if calling PREDICATE returns non-nil. > > Also correct original docstring: 's/thread/buffer/'. > --- This patch was marked stale, but isn't. > -(defun notmuch-show-mapc (function) > - "Iterate through all messages in the current thread with > +(defun notmuch-show-mapc (function predicate) > + "Iterate through all messages in the current buffer with > `notmuch-show-goto-message-next' and call FUNCTION for side > -effects." > +effects. > + > +If provided with optional argument PREDICATE, only call > +FUNCTION if calling PREDICATE returns non-nil." >(save-excursion > (goto-char (point-min)) > -(loop do (funcall function) > +(loop do (if predicate > + (if (funcall predicate) > + (funcall function)) > +(funcall function)) I don't like the way this if-structure looks, since I have to squint to see whether the "else" clause matches the inner or the outer "if". Maybe change the inner "if" to a "when" or an "and"? Ethan
Re: [PATCH v2 4/6] emacs: add optional predicate arg to `notmuch-show-mapc'
Pieter Praet pie...@praet.org writes: * emacs/notmuch-show.el (notmuch-show-mapc): If provided with optional argument PREDICATE, only call FUNCTION if calling PREDICATE returns non-nil. Also correct original docstring: 's/thread/buffer/'. --- This patch was marked stale, but isn't. -(defun notmuch-show-mapc (function) - Iterate through all messages in the current thread with +(defun notmuch-show-mapc (function optional predicate) + Iterate through all messages in the current buffer with `notmuch-show-goto-message-next' and call FUNCTION for side -effects. +effects. + +If provided with optional argument PREDICATE, only call +FUNCTION if calling PREDICATE returns non-nil. (save-excursion (goto-char (point-min)) -(loop do (funcall function) +(loop do (if predicate + (if (funcall predicate) + (funcall function)) +(funcall function)) I don't like the way this if-structure looks, since I have to squint to see whether the else clause matches the inner or the outer if. Maybe change the inner if to a when or an and? Ethan ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2 4/6] emacs: add optional predicate arg to `notmuch-show-mapc'
Mark Walters markwalters1...@gmail.com writes: The original function feels a little fragile to me as to what happens if predicate or function move point. Eg what happens if function collapses the message: where does point go, and so where does notmuch-show-goto-message-next go. Is this just my naivete as a lisp beginner? Is there someway of writing it so the user doesn't need to worry about such things? Although collapsing the message doesn't seem to move point, it would probably be a good idea to wrap the calls to predicate and function in save-excursion, as a guard against subtle and hard-to-spot bugs with operations not being applied to all the right messages.. Ethan ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 4/6] emacs: add optional predicate arg to `notmuch-show-mapc'
On Fri, 24 Feb 2012 23:30:38 +0100, Pieter Praet wrote: > * emacs/notmuch-show.el (notmuch-show-mapc): > > If provided with optional argument PREDICATE, only call > FUNCTION if calling PREDICATE returns non-nil. > > Also correct original docstring: 's/thread/buffer/'. > --- > emacs/notmuch-show.el | 14 ++ > 1 files changed, 10 insertions(+), 4 deletions(-) > > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el > index aa9ccee..6adbdc0 100644 > --- a/emacs/notmuch-show.el > +++ b/emacs/notmuch-show.el > @@ -1272,13 +1272,19 @@ (defun notmuch-show-goto-message-previous () > (notmuch-show-move-to-message-top) > t)) > > -(defun notmuch-show-mapc (function) > - "Iterate through all messages in the current thread with > +(defun notmuch-show-mapc (function predicate) > + "Iterate through all messages in the current buffer with > `notmuch-show-goto-message-next' and call FUNCTION for side > -effects." > +effects. > + > +If provided with optional argument PREDICATE, only call > +FUNCTION if calling PREDICATE returns non-nil." >(save-excursion > (goto-char (point-min)) > -(loop do (funcall function) > +(loop do (if predicate > + (if (funcall predicate) > + (funcall function)) > +(funcall function)) > while (notmuch-show-goto-message-next > > ;; Functions relating to the visibility of messages and their The change looks fine. The original function feels a little fragile to me as to what happens if predicate or function move point. Eg what happens if function collapses the message: where does point go, and so where does notmuch-show-goto-message-next go. Is this just my naivete as a lisp beginner? Is there someway of writing it so the user doesn't need to worry about such things? Best wishes Mark (sorry for the duplicate mail: I sent the first message from the wrong address)
Re: [PATCH v2 4/6] emacs: add optional predicate arg to `notmuch-show-mapc'
On Fri, 24 Feb 2012 23:30:38 +0100, Pieter Praet pie...@praet.org wrote: * emacs/notmuch-show.el (notmuch-show-mapc): If provided with optional argument PREDICATE, only call FUNCTION if calling PREDICATE returns non-nil. Also correct original docstring: 's/thread/buffer/'. --- emacs/notmuch-show.el | 14 ++ 1 files changed, 10 insertions(+), 4 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index aa9ccee..6adbdc0 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -1272,13 +1272,19 @@ (defun notmuch-show-goto-message-previous () (notmuch-show-move-to-message-top) t)) -(defun notmuch-show-mapc (function) - Iterate through all messages in the current thread with +(defun notmuch-show-mapc (function optional predicate) + Iterate through all messages in the current buffer with `notmuch-show-goto-message-next' and call FUNCTION for side -effects. +effects. + +If provided with optional argument PREDICATE, only call +FUNCTION if calling PREDICATE returns non-nil. (save-excursion (goto-char (point-min)) -(loop do (funcall function) +(loop do (if predicate + (if (funcall predicate) + (funcall function)) +(funcall function)) while (notmuch-show-goto-message-next ;; Functions relating to the visibility of messages and their The change looks fine. The original function feels a little fragile to me as to what happens if predicate or function move point. Eg what happens if function collapses the message: where does point go, and so where does notmuch-show-goto-message-next go. Is this just my naivete as a lisp beginner? Is there someway of writing it so the user doesn't need to worry about such things? Best wishes Mark (sorry for the duplicate mail: I sent the first message from the wrong address) ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 4/6] emacs: add optional predicate arg to `notmuch-show-mapc'
* emacs/notmuch-show.el (notmuch-show-mapc): If provided with optional argument PREDICATE, only call FUNCTION if calling PREDICATE returns non-nil. Also correct original docstring: 's/thread/buffer/'. --- emacs/notmuch-show.el | 14 ++ 1 files changed, 10 insertions(+), 4 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index aa9ccee..6adbdc0 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -1272,13 +1272,19 @@ (defun notmuch-show-goto-message-previous () (notmuch-show-move-to-message-top) t)) -(defun notmuch-show-mapc (function) - "Iterate through all messages in the current thread with +(defun notmuch-show-mapc (function predicate) + "Iterate through all messages in the current buffer with `notmuch-show-goto-message-next' and call FUNCTION for side -effects." +effects. + +If provided with optional argument PREDICATE, only call +FUNCTION if calling PREDICATE returns non-nil." (save-excursion (goto-char (point-min)) -(loop do (funcall function) +(loop do (if predicate +(if (funcall predicate) +(funcall function)) + (funcall function)) while (notmuch-show-goto-message-next ;; Functions relating to the visibility of messages and their -- 1.7.8.1
[PATCH v2 4/6] emacs: add optional predicate arg to `notmuch-show-mapc'
* emacs/notmuch-show.el (notmuch-show-mapc): If provided with optional argument PREDICATE, only call FUNCTION if calling PREDICATE returns non-nil. Also correct original docstring: 's/thread/buffer/'. --- emacs/notmuch-show.el | 14 ++ 1 files changed, 10 insertions(+), 4 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index aa9ccee..6adbdc0 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -1272,13 +1272,19 @@ (defun notmuch-show-goto-message-previous () (notmuch-show-move-to-message-top) t)) -(defun notmuch-show-mapc (function) - Iterate through all messages in the current thread with +(defun notmuch-show-mapc (function optional predicate) + Iterate through all messages in the current buffer with `notmuch-show-goto-message-next' and call FUNCTION for side -effects. +effects. + +If provided with optional argument PREDICATE, only call +FUNCTION if calling PREDICATE returns non-nil. (save-excursion (goto-char (point-min)) -(loop do (funcall function) +(loop do (if predicate +(if (funcall predicate) +(funcall function)) + (funcall function)) while (notmuch-show-goto-message-next ;; Functions relating to the visibility of messages and their -- 1.7.8.1 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
