On Thu, 02 Feb 2012 03:25:40 +0400, Dmitry Kurochkin <dmitry.kurochkin at gmail.com> wrote: > > > [...] > > > > OK, how about this?: > > > > Looks good. Minor comments below. > > > #+begin_src sh > > test_begin_subtest "Add/remove tags to/from all matching messages." > > We should add notmuch-search here, because similar functionality would > be available in notmuch-show soon. Also, I suggest replacing > "add/remove tags to/from" with "change tags". Consider: > > Change tags for all matching threads in notmuch-search. >
Agreed, though I think you meant "matching messages". Although, Austin has suggested [1] having `notmuch-search-operate-all' (s/operate/tag) work on threads (or all messages therein, at least), as opposed to only on *matched* messages, with which I agree(d) [2]. This would also get rid of the regexp issue @ [3], allowing us to fix the race condition without having to put `notmuch-search' on a JSON diet first. And OTOH, David has asked [4] for `notmuch-show-tag-all' to work only on visible (uncollapsed) messages, so wrt keeping `notmuch-search-tag-all' and `notmuch-show-tag-all' symmetrical, it seems we're at an impasse :) > > old="inbox" ; new="xobni" ; filter="AND from:cworth" > > I would prefer this to be on separate lines. > > > o1=$(notmuch count tag:"${old}" "${filter}") ; n1=$(notmuch count > > tag:"${new}" "${filter}") > > I would definately prefer this to be on separate lines. > > Also, please consider s/o1/old_count_1, s/n1/new_count_1/ and so on. > > > test "${o1}" == "0" && o1="Need more matches!" # prevent false positives > > test_emacs "(notmuch-search \"tag:${old} ${filter}\") > > (notmuch-test-wait) > > (notmuch-search-operate-all \"+${new}\" \"-${old}\")" > > o2=$(notmuch count tag:"${old}" "${filter}") ; n2=$(notmuch count > > tag:"${new}" "${filter}") > > Same comment about separate lines. > > > notmuch tag -"${new}" +"${old}" -- tag:"${new}" "${filter}" AND NOT > > tag:"${old}"} # restore db state! > > Why "AND NOT tag:$old" is needed here? > To be extra sure we're only touching the messages which were altered in the previous segment, but it's probably rather useless. Removed. > Since the line is too long, please move the comment to a separate line > above. > > Since we are testing Emacs UI, should we restore db though Emacs as > well? > Hmm, comes with a performance hit (which is why I avoided it initially), but a fairly minor one at that. Done. > > o3=$(notmuch count tag:"${old}" "${filter}") ; n3=$(notmuch count > > tag:"${new}" "${filter}") > > Same comment about separate lines. > > > output=" > > before: ${old}=$o1 ${new}=$n1 > > after: ${old}=$o2 ${new}=$n2 > > restored: ${old}=$o3 ${new}=$n3" > > expected=" > > before: ${old}=$o1 ${new}=0 > > after: ${old}=0 ${new}=$o1 > > restored: ${old}=$o1 ${new}=0" > > I would change "=" to ":". > Done, as well as everything related to separating lines and renaming variables. Fresh patch submitted in its original thread [5]. > Regards, > Dmitry > > > [...] Peace -- Pieter [1] id:"20111112163502.GE2658 at mit.edu" [2] id:"871ut8f9ya.fsf at praet.org" [3] id:"1310416993-31031-1-git-send-email-pieter at praet.org" [4] id:"87wr7xqpuf.fsf at rocinante.cs.unb.ca" [5] id:"1329683908-5435-1-git-send-email-pieter at praet.org"