Thanks for the review. A couple of comments below: On Sat, 01 Jun 2013, David Bremner <david at tethera.net> wrote: > Mark Walters <markwalters1009 at gmail.com> writes: > >> Previously pick had no actions based on the entire thread: this adds >> some. Note in this version '*' is bound to `tag thread' which is not >> consistent with search or show. However it still might be the most >> natural thing (as it is similar to running * in the show pane). > > Hmm. not sure about this. But since nobody complained in six months, > let's go with it unless they wake up.
Ok >> + (setq tag-changes (funcall 'notmuch-tag >> (notmuch-pick-get-messages-ids-thread-search) tag-changes)) > > Why "(funcall 'notmuch-tag ...) instead of a "(notmuch-tag ...)" here? > Personally I'd use let instead of setq here, but ymmv. I agree with both of these but this is almost identical code to that in notmuch-show-tag-all. >> +Archive each message currently shown by applying the tag changes >> +in `notmuch-archive-tags' to each (remove the \"inbox\" tag by >> +default). If a prefix argument is given, the messages will be >> +\"unarchived\", i.e. the tag changes in `notmuch-archive-tags' >> +will be reversed. > > It seems to me the default value for notmuch-archive-tags should not be > hard-coded into this docstring, especially since notmuch-archive-tags > turns into a link in the help text. I am happy either way on this one but again this is the same as notmuch-show-archive-thread. I think it's probably best to keep show and pick the same where they are similar but I can change the above if you disagree. Best wishes Mark