From: Austin Clements <amdragon@MIT.EDU>
To: Mark Walters <markwalters1009@gmail.com>
Cc: notmuch@notmuchmail.org
Subject: Re: [RFC PATCH v5 00/11] Add NOTMUCH_MESSAGE_FLAG_EXCLUDED flag
Date: Fri, 24 Feb 2012 18:58:47 -0500 [thread overview]
Message-ID: <20120224235847.GJ30513@mit.edu> (raw)
In-Reply-To: <1329296619-7463-1-git-send-email-markwalters1009@gmail.com>
Quoth Mark Walters on Feb 15 at 9:03 am:
> This is v5 of the exclude flag series. (v4 was at
> id:"874nv9rv79.fsf@qmul.ac.uk")
>
> This email has 4 sections, an overview of what the patch set is
> trying to achieve, a summary of the changes from v4, some comments on
> the status of the patches in the series and some remaining queries.
>
> OVERVIEW
>
> The current implementation of exclude-tags does not use excludes in
> notmuch-show.c (and thus not in notmuch-show.el). Thus when selecting
> a thread in the search view claiming one matched message you may get
> several matches in show all but one of which are tagged excluded.
>
> The trivial change of adding excludes to show does not work as easily
> as one would like. For example if you try notmuch-show
> id:<deleted message> you get no results (see
> id:"871uqvgrnm.fsf@qmul.ac.uk" for more discussion).
>
> This set moves in a different direction. It returns all the results
> but marks excluded messages with a new flag
> (NOTMUCH_MESSAGE_FLAG_EXCLUDED) and lets the consumer decide what to
> do with them. For example it could start with the message closed in
> emacs show view, it could colour the headerline differently etc.
>
> CHANGES:
> This has been rebased on top of Jani's notmuch-show command line
> parsing patch.
>
> The function notmuch_thread_get_flag_messages function added in v4
> has been removed. Unfortunately (as Austin pointed out) that patch
> broke binary compatibility so should be deferred until we wish to
> bump the library version.
>
> I also fixed a minor bug and a style comment in the emacs part of
> the patch.
>
> STATUS:
> The first 3 patches in the series just add a --no-exclude option
> to notmuch-search.c and notmuch-count.c to "turn off" the excluding.
> (The 3 patches are one for the C code, one for the man pages and
> one for the tests.) I think this change is desirable independently
> of the rest of the series (and indeed Jameson had a use for it
> id:"878vk943ci.fsf@servo.finestructure.net").
>
> QUERIES
>
> 1) As with v4 the api notmuch_query_set_omit_excluded_messages
> remains: without it I can't see how a user can pass a
> notmuch_messages_t object around which does not contain the
> excluded messages. See id:"87fweusabh.fsf@qmul.ac.uk".
This does seem like a useful simplification. Another possibility
(which may not work in practice) would be to have a utility function
in the CLI that iterated a notmuch_messages_t to the next non-excluded
message. This would push the knowledge of whether or not a format can
represent excluded messages into that format, but I can't see how to
do things like "count" this way.
> 2) If we have a query which overrides the excludes such as "blah and
> tag:deleted" should the tag:deleted messages still be marked excluded?
> The current implementation does mark them excluded but my preference
> would be not to. What do people think? At the moment 2 tests fail but
> the correct output depends on the above so I will leave them until we
> have a decision on this.
I could go either way on this, but I think it should still be marked
excluded since it is, after all, excluded.
> There are some other queries mentioned in v4 but the two above are
> the significant ones.
>
> Best wishes
>
> Mark
>
>
>
> Mark Walters (11):
> cli: add --no-exclude option to count and search.
> cli: Add --no-exclude to the man pages for search and count
> test: add tests for new cli --no-exclude option
> lib: Rearrange the exclude code in query.cc
> lib: Make notmuch_query_search_messages set the exclude flag
> lib: Add the exclude flag to notmuch_query_search_threads
> cli: Make notmuch-show respect excludes.
> man: update manpage for notmuch-show --no-exclude option
> test: update tests to reflect the exclude flag
> cli: omit excluded messages in results where appropriate.
> emacs: show: recognize the exclude flag.
>
> emacs/notmuch-show.el | 19 +++++++++++-
> lib/notmuch-private.h | 8 ++++-
> lib/notmuch.h | 16 ++++++++--
> lib/query.cc | 74 ++++++++++++++++++++++++++++++++++++++------
> lib/thread.cc | 18 +++++++++--
> man/man1/notmuch-count.1 | 7 ++++
> man/man1/notmuch-search.1 | 7 ++++
> man/man1/notmuch-show.1 | 7 ++++
> notmuch-count.c | 19 ++++++++----
> notmuch-search.c | 26 ++++++++++++----
> notmuch-show.c | 31 ++++++++++++++++--
> test/count | 21 +++++++++++++
> test/crypto | 9 +++++-
> test/encoding | 2 +-
> test/json | 6 ++--
> test/maildir-sync | 1 +
> test/multipart | 4 +-
> test/search | 5 +++
> test/thread-naming | 16 +++++-----
> 19 files changed, 246 insertions(+), 50 deletions(-)
prev parent reply other threads:[~2012-02-24 23:58 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-02-15 9:03 [RFC PATCH v5 00/11] Add NOTMUCH_MESSAGE_FLAG_EXCLUDED flag Mark Walters
2012-02-15 9:03 ` [RFC PATCH v5 01/11] cli: add --no-exclude option to count and search Mark Walters
2012-02-24 23:03 ` Austin Clements
2012-02-15 9:03 ` [RFC PATCH v5 02/11] cli: Add --no-exclude to the man pages for search and count Mark Walters
2012-02-24 23:05 ` Austin Clements
2012-02-15 9:03 ` [RFC PATCH v5 03/11] test: add tests for new cli --no-exclude option Mark Walters
2012-02-15 9:03 ` [RFC PATCH v5 04/11] lib: Rearrange the exclude code in query.cc Mark Walters
2012-02-15 9:03 ` [RFC PATCH v5 05/11] lib: Make notmuch_query_search_messages set the exclude flag Mark Walters
2012-02-24 23:39 ` Austin Clements
2012-02-15 9:03 ` [RFC PATCH v5 06/11] lib: Add the exclude flag to notmuch_query_search_threads Mark Walters
2012-02-15 9:03 ` [RFC PATCH v5 07/11] cli: Make notmuch-show respect excludes Mark Walters
2012-02-15 9:03 ` [RFC PATCH v5 08/11] man: update manpage for notmuch-show --no-exclude option Mark Walters
2012-02-24 23:44 ` Austin Clements
2012-02-15 9:03 ` [RFC PATCH v5 09/11] test: update tests to reflect the exclude flag Mark Walters
2012-02-24 23:47 ` Austin Clements
2012-02-15 9:03 ` [RFC PATCH v5 10/11] cli: omit excluded messages in results where appropriate Mark Walters
2012-02-15 9:03 ` [RFC PATCH v5 11/11] emacs: show: recognize the exclude flag Mark Walters
2012-02-15 17:46 ` [RFC PATCH v5 00/11] Add NOTMUCH_MESSAGE_FLAG_EXCLUDED flag Jameson Graef Rollins
2012-02-15 18:10 ` Jameson Graef Rollins
2012-02-15 21:11 ` Mark Walters
2012-02-15 22:16 ` Jameson Graef Rollins
2012-02-15 23:59 ` Austin Clements
2012-02-16 0:30 ` Mark Walters
2012-02-24 23:58 ` Austin Clements [this message]
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=20120224235847.GJ30513@mit.edu \
--to=amdragon@mit.edu \
--cc=markwalters1009@gmail.com \
--cc=notmuch@notmuchmail.org \
/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).