On Mon, 09 Apr 2012, Jameson Graef Rollins <jrollins@finestructure.net> wrote: > On Sat, Apr 07 2012, Mark Walters <markwalters1009@gmail.com> wrote: >> I think this is what is making the two tests fail: they count the number >> of invocations of notmuch and in case there is one invocation of notmuch >> show and one of notmuch tag -unread message-id, where before it was just >> the single notmuch show. > > Good call, Mark. After a bit of testing it looks like that is what's > going on. I was confused, since I had thought that the call to > notmuch-show should have involved two notmuch calls originally as well, > one for retrieving the message and the other removing the unread tag. > However, it appears the messages in those tests don't have unread tags > after all. Not sure why, but that explains it. > > So I guess the upshot is that moving all the common prompting and tag > validation stuff into notmuch-tag means that in certain cases there will > be extra notmuch calls, even if no tags are changed. Is that a problem? > > What I can do, though, is add extra validation to notmuch-tag to not > actually call notmuch tag, or any of the pre- and post- tagging hooks, > if no tags are changing. This will still require one call to notmuch to > retrieve the current set of tags for the query, but at least it wont tag > or call the hooks if nothing is changing. That seems reasonable to me, > but please let me know if you think it's not. > > I've pasted below a new version of notmuch-tag that addresses these > issues. Let me know what you think, and I'll resubmit the series. > > jamie. > > > (defun notmuch-tag (query &optional tag-changes) > "Add/remove tags in TAG-CHANGES to messages matching QUERY. > > QUERY should be a string containing the search-terms. > TAG-CHANGES can take multiple forms. If TAG-CHANGES is a list of > strings of the form \"+tag\" or \"-tag\" then those are the tag > changes applied. If TAG-CHANGES is a string then it is > interpreted as a single tag change. If TAG-CHANGES is the string > \"-\" or \"+\", or null, then the user is prompted to enter the > tag changes. > > Note: Other code should always use this function alter tags of > messages instead of running (notmuch-call-notmuch-process \"tag\" ..) > directly, so that hooks specified in notmuch-before-tag-hook and > notmuch-after-tag-hook will be run." > ;; Perform some validation > (if (string-or-null-p tag-changes) > (if (or (string= tag-changes "-") (string= tag-changes "+") (null tag-changes)) > (setq tag-changes (notmuch-read-tag-changes tag-changes query)) > (setq tag-changes (list tag-changes)))) > (mapc (lambda (tag-change) > (unless (string-match-p "^[-+]\\S-+$" tag-change) > (error "Tag must be of the form `+this_tag' or `-that_tag'"))) > tag-changes) > (let* ((current-tags (notmuch-tag-completions (list query))) > (new-tags (notmuch-update-tags current-tags tag-changes))) > (if (equal current-tags new-tags) > ;; if no tags are changing, return nil > nil > (run-hooks 'notmuch-before-tag-hook) > (apply 'notmuch-call-notmuch-process "tag" > (append tag-changes (list "--" query))) > (run-hooks 'notmuch-after-tag-hook) > ;; otherwise, return the list of actual changed tags > tag-changes))) Does this actually do the right thing if tagging more than one message? It looks to me like it would go wrong if you tried +inbox to a thread where some messages already have tag inbox (but I could be confused)? Also, I was going to say that I was not sure there was much point in optimising in the emacs code when the cli does anyway, but there is a question with xapian locking: with the orginally posted patch you can't use n or p in show view while the database is locked (eg a background notmuch new) as you get "A Xapian exception occurred opening database: Unable to get write lock on ..." Possibly, you could pass a current-tags variable to notmuch tag (and it would not add anything in that list or delete anything not in the list). But the 2 code paths might be viewed as being too different to be worth unifying. Or possibly have a "tag-single-message" command? Best wishes Mark