On Sun, 08 Apr 2012, Jameson Graef Rollins <jroll...@finestructure.net> wrote: > The main change here is to modify argument parsing so as to not force > tag-changes to be a list, and to let notmuch-tag handle prompting the > user when required. doc strings are also updated and cleaned up. > --- > emacs/notmuch.el | 36 +++++++++++++----------------------- > 1 files changed, 13 insertions(+), 23 deletions(-) > > diff --git a/emacs/notmuch.el b/emacs/notmuch.el > index 9aec96d..a03a526 100644 > --- a/emacs/notmuch.el > +++ b/emacs/notmuch.el > @@ -523,13 +523,10 @@ and will also appear in a buffer named \"*Notmuch > errors*\"." > See `notmuch-search-tag-region' for details." > (apply 'notmuch-search-tag-region (point) (point) tag-changes)) > > -(defun notmuch-search-tag-region (beg end &rest tag-changes) > - "Change tags for threads in the given region. > - > -TAGS is a list of tag operations for `notmuch-tag'. The tags are > -added or removed for all threads in the region from BEG to END." > +(defun notmuch-search-tag-region (beg end &optional tag-changes) > + "Change tags for threads in the given region." > (let ((search-string (notmuch-search-find-thread-id-region-search beg > end))) > - (apply 'notmuch-tag search-string tag-changes) > + (setq tag-changes (funcall 'notmuch-tag search-string tag-changes)) > (save-excursion > (let ((last-line (line-number-at-pos end)) > (max-line (- (line-number-at-pos (point-max)) 2))) > @@ -539,14 +536,14 @@ added or removed for all threads in the region from BEG > to END." > (notmuch-update-tags (notmuch-search-get-tags) tag-changes)) > (forward-line)))))) > > -(defun notmuch-search-tag (&optional initial-input) > - "Change tags for the currently selected thread or region." > +(defun notmuch-search-tag (&optional tag-changes) > + "Change tags for the currently selected thread or region. > + > +See `notmuch-tag' for information on the format of TAG-CHANGES." > (interactive) > (let* ((beg (if (region-active-p) (region-beginning) (point))) > - (end (if (region-active-p) (region-end) (point))) > - (search-string (notmuch-search-find-thread-id-region-search beg end)) > - (tags (notmuch-read-tag-changes initial-input search-string))) > - (apply 'notmuch-search-tag-region beg end tags))) > + (end (if (region-active-p) (region-end) (point)))) > + (funcall 'notmuch-search-tag-region beg end tag-changes))) > > (defun notmuch-search-add-tag () > "Same as `notmuch-search-tag' but sets initial input to '+'." > @@ -790,18 +787,11 @@ non-authors is found, assume that all of the authors > match." > (goto-char found-target))) > (delete-process proc)))) > > -(defun notmuch-search-tag-all (&rest tag-changes) > - "Add/remove tags from all matching messages. > +(defun notmuch-search-tag-all (&optional tag-changes) > + "Add/remove tags from all messages in current search buffer. > > -This command adds or removes tags from all messages matching the > -current search terms. When called interactively, this command > -will prompt for tags to be added or removed. Tags prefixed with > -'+' will be added and tags prefixed with '-' will be removed. > - > -Each character of the tag name may consist of alphanumeric > -characters as well as `_.+-'. > -" > - (interactive (notmuch-read-tag-changes)) > +See `notmuch-tag' for information on the format of TAG-CHANGES." > + (interactive) > (apply 'notmuch-tag notmuch-search-query-string tag-changes))
My only comment here is that I wonder about the tag-all command. I can see that this is a performance win over applying tag region to the whole buffer but is it another place where we have a race with incoming mail (ie we tag everything that now matches rather than everything in the buffer). A second advantage with the tag-region form is that it automatically does excludes. Currently notmuch-tag does not do excludes as it looked very hairy to implement correctly: thus the tag-all command could spend a lot of time tagging excluded messages. OTOH this seems to be a "feature" of the existing code so a "fix" is not required for this series. (Note I would not consider this a full review as my lisp is not good enough; eg I am unsure why the change from apply to funcall was needed). Best wishes Mark > > (defun notmuch-search-buffer-title (query) > -- > 1.7.9.1 > > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch _______________________________________________ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch