From: Carl Worth <cworth@cworth.org>
To: Jesse Rosenthal <jrosenthal@jhu.edu>, notmuch@notmuchmail.org
Subject: Re: [notmuch] [PATCH v2] notmuch.el: add functionality in notmuch
	search mode to add or remove tags by region
In-Reply-To: <87sk90ragj.fsf@jhu.edu>
References: <m1636w7au5.fsf@watt.gilman.jhu.edu> <87sk90ragj.fsf@jhu.edu>
Date: Wed, 07 Apr 2010 14:10:38 -0700
Message-ID: <87aatfq88h.fsf@yoom.home.cworth.org>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="=-=-=";
	micalg=pgp-sha1; protocol="application/pgp-signature"
Precedence: list

--=-=-=
Content-Transfer-Encoding: quoted-printable

On Tue, 16 Feb 2010 19:07:40 -0500, Jesse Rosenthal <jrosenthal@jhu.edu> wr=
ote:
> This patch adds `-region' versions of the `notmuch-search-' commands to f=
ind
> properties. It also splits up  `notmuch-add/remove-tags' into both a
> `-thread' and a `-region' version. (This makes us modify
> `notmuch-search-archive-thread' to use the
> `notmuch-search-remove-tag-thread' function, instead of
> `notmuch-search-remove-tag', for consistency.) The add/remove-tag command
> called by pressing `+' or `-' will then choose accordingly, based on whet=
her
> region is active.
>=20
> This version fixes a couple of errors in the first version, which led to
> incorrect marking of some tags in the search view (though the actual
> tagging was still correct). It's also based on current master.
>=20
> I'm not sure any more if region selection is actually the correct way to
> do this, or if a mutt-style message-marking method would be better. But
> I didn't want a buggy incorrect version out there.

I think this feature is very useful, and that the region is definitely
an appropriate way to implement it, (doing region-based operations is
very natural for emacs users). Mutt-style marking could be implemented
as well, but that would be separate I think.

I tested this patch a bit and added one small cleanup to the
documentation (see below).

I also don't like the duplication of code in
notmuch-search-add-tag-thread and notmuch-search-add-tag-region, (and
the same in the remove case). Fortunately, I think this easy to avoid by
simply making notmuch-search-add-tag-thread call:

       (notmuch-search-add-tag-region tag (point) (point))

and the same in the remove case.

But I haven't pushed this patch yet for a flaw in the case of selecting
beyond the last thread, (such as selecting to the line that includes the
"End of search results). If I try an operation on that line, it will act
like it works, (it displays the new tags by all threads), but then a
refresh makes them all disappear again. Presumably the "notmuch tag"
command is failing, this isn't being noticed, and the code marches on to
update the representation of the tags.

And presumably the notmuch tag is failing because no thread ID is found
for the last line, so the mapconcat call is sticking an extra " or "
onto the end of the search string. And Xapian doesn't like that:

	$ notmuch search tag:inbox or
	A Xapian exception occurred performing query: Syntax: <expression> OR <exp=
ression>
        Query string was: tag:inbox or

Things behave even worse if I make the region be the entire buffer,
(including the last blank line). Then the commands hang. I got nervous
that this was then adding "or or" and trying to add/remove a tag to
every message containing the word "or". But I haven't looked closely.

If we can fix this bug, then I'd like to apply this patch. I'd even like
to fix the "*" binding to be implemented in terms of something like
these functions so that we could get visual updates of the tag state
From=20that command. (But that will take some reworking of the interface
as the current implementation can't support multiple tags added/removed
as * current expects).

I'd appreciate any help fixing up the patch.

=2DCarl

From=20fec5622add1a4e9f305c16e96143439ee22a5c58 Mon Sep 17 00:00:00 2001
From: Carl Worth <cworth@cworth.org>
Date: Wed, 7 Apr 2010 13:15:27 -0700
Subject: [PATCH] emacs: Correct the documentation for notmuch-search-add-ta=
g (and -remove-tag)

These commands act on all messages in the thread, not simply those
that match the search. (There are use case for both behaviors, but the
documentation must match the behavior that's actually implemented).
=2D--
 emacs/notmuch.el |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 64f3e3d..517c53a 100644
=2D-- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -532,12 +532,11 @@ and will also appear in a buffer named \"*Notmuch err=
ors*\"."
 	  (notmuch-search-set-tags (delete tag (notmuch-search-get-tags)))
 	  (forward-line))))))
=20
=2D
 (defun notmuch-search-add-tag (tag)
   "Add a tag to the currently selected thread or region.
=20
=2DThe tag is added to messages in the currently selected thread or
=2Dregion which match the current search terms."
+The tag is added to all messages in the currently selected thread
+or threads in the current region."
   (interactive
    (list (notmuch-select-tag-with-completion "Tag to add: ")))
   (save-excursion
@@ -550,8 +549,8 @@ region which match the current search terms."
 (defun notmuch-search-remove-tag (tag)
   "Remove a tag from the currently selected thread or region.
=20
=2DThe tag is removed from all messages in the currently selected thread
=2Dor region which match the current search terms."
+The tag is removed from all messages in the currently selected
+thread or threads in the current region."
   (interactive
    (list (notmuch-select-tag-with-completion
 	  "Tag to remove: "
=2D-=20
1.7.0


--=-=-=
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iD8DBQFLvPTO6JDdNq8qSWgRAsxHAKCpnJDPWvi6vZhr6L0qctDiAcoryACbBbTJ
CIsitjtUX2yLdude6PH796M=
=25Nm
-----END PGP SIGNATURE-----
--=-=-=--