From c097e325a6c781ff0eaccf27fade9bc4e4a6364c Mon Sep 17 00:00:00 2001 From: Pieter Praet Date: Sun, 19 Feb 2012 21:42:46 +0100 Subject: [PATCH] Re: [PATCH v4] test: emacs: add test for `notmuch-search-operate-all' --- ff/4c1c3405abd1bb15159137742c8a8e30da6b16 | 181 ++++++++++++++++++++++ 1 file changed, 181 insertions(+) create mode 100644 ff/4c1c3405abd1bb15159137742c8a8e30da6b16 diff --git a/ff/4c1c3405abd1bb15159137742c8a8e30da6b16 b/ff/4c1c3405abd1bb15159137742c8a8e30da6b16 new file mode 100644 index 000000000..d9ba3cda5 --- /dev/null +++ b/ff/4c1c3405abd1bb15159137742c8a8e30da6b16 @@ -0,0 +1,181 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id B7977431FAF + for ; Sun, 19 Feb 2012 12:45:04 -0800 (PST) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: -0.7 +X-Spam-Level: +X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5 + tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled +Received: from olra.theworths.org ([127.0.0.1]) + by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id 43W9dHGgZyK9 for ; + Sun, 19 Feb 2012 12:45:03 -0800 (PST) +Received: from mail-wi0-f181.google.com (mail-wi0-f181.google.com + [209.85.212.181]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) + (No client certificate requested) + by olra.theworths.org (Postfix) with ESMTPS id 88B86431FAE + for ; Sun, 19 Feb 2012 12:45:03 -0800 (PST) +Received: by wibhi8 with SMTP id hi8so3108478wib.26 + for ; Sun, 19 Feb 2012 12:45:02 -0800 (PST) +Received-SPF: pass (google.com: domain of pieter@praet.org designates + 10.180.99.100 as permitted sender) client-ip=10.180.99.100; +Authentication-Results: mr.google.com; + spf=pass (google.com: domain of pieter@praet.org + designates 10.180.99.100 as permitted sender) + smtp.mail=pieter@praet.org +Received: from mr.google.com ([10.180.99.100]) + by 10.180.99.100 with SMTP id ep4mr11936518wib.7.1329684302310 + (num_hops = 1); Sun, 19 Feb 2012 12:45:02 -0800 (PST) +Received: by 10.180.99.100 with SMTP id ep4mr9994796wib.7.1329684302229; + Sun, 19 Feb 2012 12:45:02 -0800 (PST) +Received: from localhost (104.218-242-81.adsl-dyn.isp.belgacom.be. + [81.242.218.104]) + by mx.google.com with ESMTPS id s2sm29263562wix.3.2012.02.19.12.45.01 + (version=TLSv1/SSLv3 cipher=OTHER); + Sun, 19 Feb 2012 12:45:01 -0800 (PST) +From: Pieter Praet +To: Dmitry Kurochkin +Subject: Re: [PATCH v4] test: emacs: add test for `notmuch-search-operate-all' +In-Reply-To: <8762fqm8zv.fsf@gmail.com> +References: <1327599289-16865-1-git-send-email-dmitry.kurochkin@gmail.com> + <1327909550-6788-1-git-send-email-pieter@praet.org> + <87y5spober.fsf@gmail.com> <877h067jn3.fsf@praet.org> + <87hazamywi.fsf@gmail.com> <87liomb8gv.fsf@praet.org> + <8762fqm8zv.fsf@gmail.com> +User-Agent: Notmuch/0.11.1+210~g6afc43e (http://notmuchmail.org) Emacs/23.3.1 + (x86_64-unknown-linux-gnu) +Date: Sun, 19 Feb 2012 21:42:46 +0100 +Message-ID: <87obsusgft.fsf@praet.org> +MIME-Version: 1.0 +Content-Type: text/plain; charset=us-ascii +X-Gm-Message-State: + ALoCoQlivaqhpLNpc1N8xCNycxccdh5Dwn3AyrCcOpT97h8nmHNrd5QCxylSVkNi8OFmNBZtZSuq +Cc: Notmuch Mail +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.13 +Precedence: list +List-Id: "Use and development of the notmuch mail system." + +List-Unsubscribe: , + +List-Archive: +List-Post: +List-Help: +List-Subscribe: , + +X-List-Received-Date: Sun, 19 Feb 2012 20:45:05 -0000 + +On Thu, 02 Feb 2012 03:25:40 +0400, Dmitry Kurochkin 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@mit.edu" +[2] id:"871ut8f9ya.fsf@praet.org" +[3] id:"1310416993-31031-1-git-send-email-pieter@praet.org" +[4] id:"87wr7xqpuf.fsf@rocinante.cs.unb.ca" +[5] id:"1329683908-5435-1-git-send-email-pieter@praet.org" -- 2.26.2