Received-SPF: pass (google.com: domain of pieter@praet.org designates
	10.180.99.100 as permitted sender) client-ip=10.180.99.100; 
From: Pieter Praet <pieter@praet.org>
To: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
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
Cc: Notmuch Mail <notmuch@notmuchmail.org>
Precedence: list

On Thu, 02 Feb 2012 03:25:40 +0400, Dmitry Kurochkin <dmitry.kurochkin@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@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"