unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
From: Mark Walters <markwalters1009@gmail.com>
To: Vladimir Panteleev <notmuch@thecybershadow.net>, notmuch@notmuchmail.org
Cc: Vladimir Panteleev <git@thecybershadow.net>
Subject: Re: [PATCH v2] emacs: Add notmuch-update-search-tags
Date: Sun, 27 Aug 2017 08:46:53 +0100	[thread overview]
Message-ID: <8760d9qwpe.fsf@qmul.ac.uk> (raw)
In-Reply-To: <be94da13-7dd7-be62-2005-80ae9a12297c@gmail.com>


Hi

OK I have now actually tested it, and I have read the patch more
carefully, but I am afraid I still have concerns.

The key problem is the patch assumes that the display of a thread in a
search buffer depends only on the thread. And this is not true as the
number of matching messages is displayed eg [10/19], and the author list
is split into matching authors | unmatching authors.

However, when the tags in a thread are changed the patch makes all
search buffers containing that thread update to look the same. Indeed,
if you change a tag on a single message I think the count will always
update to be [1/??] as there is only one message matching the tag-change
query.

I think you could get round this by modifying your code only slightly --
rather than calling notmuch-search-update-result in
notmuch-search-update-results, *just* update the tags, using something
like notmuch-search-set-tags. (I have just tried this and it seems to work.)

This is not perfect, as this will show tags of newly arrived messages in
the thread, but that might well be acceptable.

And this still keeps it to one call to the database, which avoids your
(completely correct) performance concerns.


> It may be possible to optimize this down to one batch search query per 
> notmuch-search buffer - however, this results in a large search query. 
> Not only would one take a while to execute, but the resulting query can 
> become too large to be passed as a command-line parameter, and "notmuch 
> search" does not seem to have a --batch switch to read queries from 
> standard input.

This points to my next concern -- this is already a problem in the
current patch. If you go into a moderately large search buffer, and do
something like * +foo (to tag all the messages with foo), the tag step
works because is uses the --batch switch to tag, but your search query
doesn't.

The options here are to:  add --batch to search handling, or  just
decide not to do display the tag updates for large queries.

Incidentally there also seems to a bug in the current that the first
thread in a search buffer doesn't seem to get updated. I think you are
using too low level functions -- things like
notmuch-search-foreach-result might do exactly what you need.

Finally, in the longer term, do you wanting to do this for tree and show
buffers too?

[An alternative approach, which I don't think I like but I mention in
case others do -- we could only propagate tag changes to parent
buffers. So updating a tag in a show buffer only updates the single
search buffer that called it. This might avoid the large query problem
as show buffers probably don't have large queries.]

Best wishes

Mark

  reply	other threads:[~2017-08-27  7:47 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-14  5:54 [PATCH 0/2] Update search results when tags change Vladimir Panteleev
2017-08-14  5:54 ` [PATCH 1/2] notmuch-tag.el: Fix minor grammar error Vladimir Panteleev
2017-08-23 11:11   ` David Bremner
2017-08-14  5:54 ` [PATCH 2/2] emacs: Add notmuch-update-search-tags Vladimir Panteleev
2017-08-25 11:12   ` David Bremner
2017-08-26  1:50     ` Vladimir Panteleev
2017-08-26  1:55       ` [PATCH v2] " Vladimir Panteleev
2017-08-26 10:23         ` Mark Walters
2017-08-26 17:15           ` Vladimir Panteleev
2017-08-27  7:46             ` Mark Walters [this message]
2017-08-29  1:18         ` David Bremner
2017-09-04 22:05           ` Vladimir Panteleev
2017-09-06 13:11             ` David Bremner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://notmuchmail.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8760d9qwpe.fsf@qmul.ac.uk \
    --to=markwalters1009@gmail.com \
    --cc=git@thecybershadow.net \
    --cc=notmuch@notmuchmail.org \
    --cc=notmuch@thecybershadow.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).