On Tue, 16 Feb 2010 19:07:40 -0500, Jesse Rosenthal <jrosenthal at jhu.edu> 
wrote:
> This patch adds `-region' versions of the `notmuch-search-' commands to find
> 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 whether
> region is active.
> 
> 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.
> 
> 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 
<expression>
        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

Reply via email to