[Patch V4] Add NOTMUCH_MESSAGE_FLAG_EXCLUDED flag

[Mark Walters]

Here is the latest version of this patch set. I think I have fixed most
of the problems raised in review but there are some remaining issues
detailed below.

Changes and queries:

1) Changed --do-not-exclude option to --no-exclude

2) The api notmuch_query_set_omit_excluded_messages remains: without it I
can't see how a user can pass the notmuch_messages_t object around which
does not contain the excluded messages. See
id:"[hidden email]"

3) I have introduced a new function notmuch_thread_get_flag_messages
(notmuch_thread_t *thread, unsigned int flag_mask, unsigned int flags)

which returns the number of messages with the specified flags on
flag_mask. (Note the current NOTMUCH_MESSAGE_FLAGs were nominally the
bit position of the flag rather than the actual bit of the flag. I
changed that. I am not completely happy with the style for this commit
(patch 7/11): any comments gratefully received!

4) In id:"[hidden email]" Austin suggested that I use a
notmuch_mset_messages_t object rather than an notmuch_doc_id_set_t. I
couldn't see how that would work unless the iterator would generate the
excludes in step with the main query. At the moment the doc_id object
just stores a bitmap listing all relevant excluded messages.

5) 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?

6) In id:"[hidden email]" Austin pointed out that the
sort will be influenced by the excluded messages. I do not think either
of us are sure whether it should be or not so I have left it as is for
the moment.

Best wishes

Mark






_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Mark Walters]

This option turns off the exclusion so all matching messages are
returned. We do not need to add this to notmuch-show as that does not
(yet) exclude.
---
 notmuch-count.c  |   17 +++++++++++------
 notmuch-search.c |   17 +++++++++++------
 2 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/notmuch-count.c b/notmuch-count.c
index 63459fb..5364507 100644
--- a/notmuch-count.c
+++ b/notmuch-count.c
@@ -35,8 +35,7 @@ notmuch_count_command (void *ctx, int argc, char *argv[])
     char *query_str;
     int opt_index;
     int output = OUTPUT_MESSAGES;
-    const char **search_exclude_tags;
-    size_t search_exclude_tags_length;
+    notmuch_bool_t no_exclude = FALSE;
     unsigned int i;
 
     notmuch_opt_desc_t options[] = {
@@ -44,6 +43,7 @@ notmuch_count_command (void *ctx, int argc, char *argv[])
   (notmuch_keyword_t []){ { "threads", OUTPUT_THREADS },
   { "messages", OUTPUT_MESSAGES },
   { 0, 0 } } },
+ { NOTMUCH_OPT_BOOLEAN, &no_exclude, "no-exclude", 'd', 0 },
  { 0, 0, 0, 0, 0 }
     };
 
@@ -78,10 +78,15 @@ notmuch_count_command (void *ctx, int argc, char *argv[])
  return 1;
     }
 
-    search_exclude_tags = notmuch_config_get_search_exclude_tags
- (config, &search_exclude_tags_length);
-    for (i = 0; i < search_exclude_tags_length; i++)
- notmuch_query_add_tag_exclude (query, search_exclude_tags[i]);
+    if (!no_exclude) {
+ const char **search_exclude_tags;
+ size_t search_exclude_tags_length;
+
+ search_exclude_tags = notmuch_config_get_search_exclude_tags
+    (config, &search_exclude_tags_length);
+ for (i = 0; i < search_exclude_tags_length; i++)
+    notmuch_query_add_tag_exclude (query, search_exclude_tags[i]);
+    }
 
     switch (output) {
     case OUTPUT_MESSAGES:
diff --git a/notmuch-search.c b/notmuch-search.c
index d504051..43ec90b 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -423,8 +423,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
     output_t output = OUTPUT_SUMMARY;
     int offset = 0;
     int limit = -1; /* unlimited */
-    const char **search_exclude_tags;
-    size_t search_exclude_tags_length;
+    notmuch_bool_t no_exclude = FALSE;
     unsigned int i;
 
     enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT }
@@ -446,6 +445,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
   { "files", OUTPUT_FILES },
   { "tags", OUTPUT_TAGS },
   { 0, 0 } } },
+        { NOTMUCH_OPT_BOOLEAN, &no_exclude, "no-exclude", 'd', 0 },
  { NOTMUCH_OPT_INT, &offset, "offset", 'O', 0 },
  { NOTMUCH_OPT_INT, &limit, "limit", 'L', 0  },
  { 0, 0, 0, 0, 0 }
@@ -493,10 +493,15 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
 
     notmuch_query_set_sort (query, sort);
 
-    search_exclude_tags = notmuch_config_get_search_exclude_tags
- (config, &search_exclude_tags_length);
-    for (i = 0; i < search_exclude_tags_length; i++)
- notmuch_query_add_tag_exclude (query, search_exclude_tags[i]);
+    if (!no_exclude) {
+ const char **search_exclude_tags;
+ size_t search_exclude_tags_length;
+
+ search_exclude_tags = notmuch_config_get_search_exclude_tags
+    (config, &search_exclude_tags_length);
+ for (i = 0; i < search_exclude_tags_length; i++)
+    notmuch_query_add_tag_exclude (query, search_exclude_tags[i]);
+    }
 
     switch (output) {
     default:
--
1.7.2.3

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Mark Walters]

---
 man/man1/notmuch-count.1  |    7 +++++++
 man/man1/notmuch-search.1 |    7 +++++++
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/man/man1/notmuch-count.1 b/man/man1/notmuch-count.1
index 25fe329..413b405 100644
--- a/man/man1/notmuch-count.1
+++ b/man/man1/notmuch-count.1
@@ -38,6 +38,13 @@ Output the number of matching messages. This is the default.
 Output the number of matching threads.
 .RE
 .RE
+
+.RS 4
+.TP 4
+.BR \-\-no\-exclude
+
+Do not exclude the messages matching search_exclude_tags in the config file.
+.RE
 .RE
 .RE
 
diff --git a/man/man1/notmuch-search.1 b/man/man1/notmuch-search.1
index 0bc3f40..bc54b4d 100644
--- a/man/man1/notmuch-search.1
+++ b/man/man1/notmuch-search.1
@@ -112,6 +112,13 @@ result from the end.
 Limit the number of displayed results to N.
 .RE
 
+.RS 4
+.TP 4
+.BR \-\-no\-exclude
+
+Do not exclude the messages matching search_exclude_tags in the config file.
+.RE
+
 .SH SEE ALSO
 
 \fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
--
1.7.2.3

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Mark Walters]

The tests test the new --no-exclude option to search and count.
There were no existing tests for the exclude behaviour for count so
added these too.
---
 test/count  |   21 +++++++++++++++++++++
 test/search |    5 +++++
 2 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/test/count b/test/count
index 300b171..976fff1 100755
--- a/test/count
+++ b/test/count
@@ -37,4 +37,25 @@ test_expect_equal \
     "0" \
     "`notmuch count --output=threads ${SEARCH}`"
 
+test_begin_subtest "count excluding \"deleted\" messages"
+notmuch config set search.exclude_tags = deleted
+generate_message '[subject]="Not deleted"'
+generate_message '[subject]="Another not deleted"'
+generate_message '[subject]="Deleted"'
+notmuch new > /dev/null
+notmuch tag +deleted id:$gen_msg_id
+test_expect_equal \
+    "2" \
+    "`notmuch count subject:deleted`"
+
+test_begin_subtest "count \"deleted\" messages, exclude overridden"
+test_expect_equal \
+    "1" \
+    "`notmuch count subject:deleted and tag:deleted`"
+
+test_begin_subtest "count \"deleted\" messages, with --no-exclude"
+test_expect_equal \
+    "3" \
+    "`notmuch count --no-exclude subject:deleted`"
+
 test_done
diff --git a/test/search b/test/search
index 414be35..3da5d17 100755
--- a/test/search
+++ b/test/search
@@ -148,6 +148,11 @@ output=$(notmuch search subject:deleted | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox unread)
 thread:XXX   2001-01-05 [1/2] Notmuch Test Suite; Not deleted reply (deleted inbox unread)"
 
+test_begin_subtest "Don't exclude \"deleted\" messages when --no-exclude specified"
+output=$(notmuch search --no-exclude subject:deleted | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox unread)
+thread:XXX   2001-01-05 [2/2] Notmuch Test Suite; Deleted (deleted inbox unread)"
+
 test_begin_subtest "Don't exclude \"deleted\" messages from search if not configured"
 notmuch config set search.exclude_tags
 output=$(notmuch search subject:deleted | notmuch_search_sanitize)
--
1.7.2.3

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Mark Walters]

Slightly refactor the exclude code to give the callers access to the
exclude query itself. There should be no functional change.
---
 lib/query.cc |   29 +++++++++++++++++++----------
 1 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/lib/query.cc b/lib/query.cc
index 0b36602..c25b301 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -122,12 +122,15 @@ _notmuch_messages_destructor (notmuch_mset_messages_t *messages)
     return 0;
 }
 
-/* Return a query that does not match messages with the excluded tags
- * registered with the query.  Any tags that explicitly appear in
- * xquery will not be excluded. */
+/* Return a query that matches messages with the excluded tags
+ * registered with query.  Any tags that explicitly appear in xquery
+ * will not be excluded. The caller of this function has to combine
+ * the returned query appropriately.*/
 static Xapian::Query
 _notmuch_exclude_tags (notmuch_query_t *query, Xapian::Query xquery)
 {
+    Xapian::Query exclude_query = Xapian::Query::MatchNothing;
+
     for (notmuch_string_node_t *term = query->exclude_terms->head; term;
  term = term->next) {
  Xapian::TermIterator it = xquery.get_terms_begin ();
@@ -137,10 +140,10 @@ _notmuch_exclude_tags (notmuch_query_t *query, Xapian::Query xquery)
  break;
  }
  if (it == end)
-    xquery = Xapian::Query (Xapian::Query::OP_AND_NOT,
-    xquery, Xapian::Query (term->string));
+    exclude_query = Xapian::Query (Xapian::Query::OP_OR,
+    exclude_query, Xapian::Query (term->string));
     }
-    return xquery;
+    return exclude_query;
 }
 
 notmuch_messages_t *
@@ -168,7 +171,7 @@ notmuch_query_search_messages (notmuch_query_t *query)
  Xapian::Query mail_query (talloc_asprintf (query, "%s%s",
    _find_prefix ("type"),
    "mail"));
- Xapian::Query string_query, final_query;
+ Xapian::Query string_query, final_query, exclude_query;
  Xapian::MSet mset;
  unsigned int flags = (Xapian::QueryParser::FLAG_BOOLEAN |
       Xapian::QueryParser::FLAG_PHRASE |
@@ -188,7 +191,10 @@ notmuch_query_search_messages (notmuch_query_t *query)
  mail_query, string_query);
  }
 
- final_query = _notmuch_exclude_tags (query, final_query);
+ exclude_query = _notmuch_exclude_tags (query, final_query);
+
+ final_query = Xapian::Query (Xapian::Query::OP_AND_NOT,
+ final_query, exclude_query);
 
  enquire.set_weighting_scheme (Xapian::BoolWeight());
 
@@ -449,7 +455,7 @@ notmuch_query_count_messages (notmuch_query_t *query)
  Xapian::Query mail_query (talloc_asprintf (query, "%s%s",
    _find_prefix ("type"),
    "mail"));
- Xapian::Query string_query, final_query;
+ Xapian::Query string_query, final_query, exclude_query;
  Xapian::MSet mset;
  unsigned int flags = (Xapian::QueryParser::FLAG_BOOLEAN |
       Xapian::QueryParser::FLAG_PHRASE |
@@ -469,7 +475,10 @@ notmuch_query_count_messages (notmuch_query_t *query)
  mail_query, string_query);
  }
 
- final_query = _notmuch_exclude_tags (query, final_query);
+ exclude_query = _notmuch_exclude_tags (query, final_query);
+
+ final_query = Xapian::Query (Xapian::Query::OP_AND_NOT,
+ final_query, exclude_query);
 
  enquire.set_weighting_scheme(Xapian::BoolWeight());
  enquire.set_docid_order(Xapian::Enquire::ASCENDING);
--
1.7.2.3

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Mark Walters]

Add a flag NOTMUCH_MESSAGE_FLAG_EXCLUDED which is set by
notmuch_query_search_messages for excluded messages. Also add an
option omit_excluded_messages to the search that we do not want the
excludes at all.

This exclude flag will be added to notmuch_query_search threads in the
next patch.
---
 lib/notmuch-private.h |    1 +
 lib/notmuch.h         |   10 ++++++++-
 lib/query.cc          |   52 +++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 7bf153e..e791bb0 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -401,6 +401,7 @@ typedef struct _notmuch_message_list {
  */
 struct visible _notmuch_messages {
     notmuch_bool_t is_of_list_type;
+    notmuch_doc_id_set_t *excluded_doc_ids;
     notmuch_message_node_t *iterator;
 };
 
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 7929fe7..f75afae 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -449,6 +449,13 @@ typedef enum {
 const char *
 notmuch_query_get_query_string (notmuch_query_t *query);
 
+/* Specify whether to results should omit the excluded results rather
+ * than just marking them excluded. This is useful for passing a
+ * notmuch_messages_t not containing the excluded messages to other
+ * functions. */
+void
+notmuch_query_set_omit_excluded_messages (notmuch_query_t *query, notmuch_bool_t omit);
+
 /* Specify the sorting desired for this query. */
 void
 notmuch_query_set_sort (notmuch_query_t *query, notmuch_sort_t sort);
@@ -895,7 +902,8 @@ notmuch_message_get_filenames (notmuch_message_t *message);
 
 /* Message flags */
 typedef enum _notmuch_message_flag {
-    NOTMUCH_MESSAGE_FLAG_MATCH
+    NOTMUCH_MESSAGE_FLAG_MATCH,
+    NOTMUCH_MESSAGE_FLAG_EXCLUDED
 } notmuch_message_flag_t;
 
 /* Get a value of a flag for the email corresponding to 'message'. */
diff --git a/lib/query.cc b/lib/query.cc
index c25b301..90a71a1 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -28,6 +28,7 @@ struct _notmuch_query {
     const char *query_string;
     notmuch_sort_t sort;
     notmuch_string_list_t *exclude_terms;
+    notmuch_bool_t omit_excluded_messages;
 };
 
 typedef struct _notmuch_mset_messages {
@@ -57,6 +58,12 @@ struct visible _notmuch_threads {
     notmuch_doc_id_set_t match_set;
 };
 
+/* We need this in the message functions so forward declare. */
+static notmuch_bool_t
+_notmuch_doc_id_set_init (void *ctx,
+  notmuch_doc_id_set_t *doc_ids,
+  GArray *arr);
+
 notmuch_query_t *
 notmuch_query_create (notmuch_database_t *notmuch,
       const char *query_string)
@@ -79,6 +86,8 @@ notmuch_query_create (notmuch_database_t *notmuch,
 
     query->exclude_terms = _notmuch_string_list_create (query);
 
+    query->omit_excluded_messages = FALSE;
+
     return query;
 }
 
@@ -89,6 +98,12 @@ notmuch_query_get_query_string (notmuch_query_t *query)
 }
 
 void
+notmuch_query_set_omit_excluded_messages (notmuch_query_t *query, notmuch_bool_t omit)
+{
+    query->omit_excluded_messages = omit;
+}
+
+void
 notmuch_query_set_sort (notmuch_query_t *query, notmuch_sort_t sort)
 {
     query->sort = sort;
@@ -173,6 +188,7 @@ notmuch_query_search_messages (notmuch_query_t *query)
    "mail"));
  Xapian::Query string_query, final_query, exclude_query;
  Xapian::MSet mset;
+ Xapian::MSetIterator iterator;
  unsigned int flags = (Xapian::QueryParser::FLAG_BOOLEAN |
       Xapian::QueryParser::FLAG_PHRASE |
       Xapian::QueryParser::FLAG_LOVEHATE |
@@ -190,11 +206,35 @@ notmuch_query_search_messages (notmuch_query_t *query)
     final_query = Xapian::Query (Xapian::Query::OP_AND,
  mail_query, string_query);
  }
+ messages->base.excluded_doc_ids = NULL;
+
+ if (query->exclude_terms) {
+    exclude_query = _notmuch_exclude_tags (query, final_query);
+    exclude_query = Xapian::Query (Xapian::Query::OP_AND,
+   exclude_query, final_query);
+
+    if (query->omit_excluded_messages)
+ final_query = Xapian::Query (Xapian::Query::OP_AND_NOT,
+     final_query, exclude_query);
+    else {
+ enquire.set_weighting_scheme (Xapian::BoolWeight());
+ enquire.set_query (exclude_query);
+
+ mset = enquire.get_mset (0, notmuch->xapian_db->get_doccount ());
+
+ GArray *excluded_doc_ids = g_array_new (FALSE, FALSE, sizeof (unsigned int));
+
+ for (iterator = mset.begin (); iterator != mset.end (); iterator++) {
+    unsigned int doc_id = *iterator;
+    g_array_append_val (excluded_doc_ids, doc_id);
+ }
+ messages->base.excluded_doc_ids = talloc (query, _notmuch_doc_id_set);
+ _notmuch_doc_id_set_init (query, messages->base.excluded_doc_ids,
+  excluded_doc_ids);
+ g_array_unref (excluded_doc_ids);
+    }
+ }
 
- exclude_query = _notmuch_exclude_tags (query, final_query);
-
- final_query = Xapian::Query (Xapian::Query::OP_AND_NOT,
- final_query, exclude_query);
 
  enquire.set_weighting_scheme (Xapian::BoolWeight());
 
@@ -283,6 +323,10 @@ _notmuch_mset_messages_get (notmuch_messages_t *messages)
  INTERNAL_ERROR ("a messages iterator contains a non-existent document ID.\n");
     }
 
+    if (messages->excluded_doc_ids &&
+ _notmuch_doc_id_set_contains (messages->excluded_doc_ids, doc_id))
+ notmuch_message_set_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED, TRUE);
+
     return message;
 }
 
--
1.7.2.3

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Mark Walters]

Add the NOTMUCH_MESSAGE_FLAG_EXCLUDED flag to
notmuch_query_search_threads. Implemented by inspecting the tags
directly in _notmuch_thread_create/_thread_add_message rather than as
a Xapian query for speed reasons.
---
 lib/notmuch-private.h |    7 +++++--
 lib/query.cc          |    1 +
 lib/thread.cc         |   18 +++++++++++++++---
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index e791bb0..ea836f7 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -148,6 +148,8 @@ typedef enum _notmuch_private_status {
 
 typedef struct _notmuch_doc_id_set notmuch_doc_id_set_t;
 
+typedef struct _notmuch_string_list notmuch_string_list_t;
+
 /* database.cc */
 
 /* Lookup a prefix value by name.
@@ -216,6 +218,7 @@ _notmuch_thread_create (void *ctx,
  notmuch_database_t *notmuch,
  unsigned int seed_doc_id,
  notmuch_doc_id_set_t *match_set,
+ notmuch_string_list_t *excluded_terms,
  notmuch_sort_t sort);
 
 /* message.cc */
@@ -459,11 +462,11 @@ typedef struct _notmuch_string_node {
     struct _notmuch_string_node *next;
 } notmuch_string_node_t;
 
-typedef struct visible _notmuch_string_list {
+struct visible _notmuch_string_list {
     int length;
     notmuch_string_node_t *head;
     notmuch_string_node_t **tail;
-} notmuch_string_list_t;
+};
 
 notmuch_string_list_t *
 _notmuch_string_list_create (const void *ctx);
diff --git a/lib/query.cc b/lib/query.cc
index 90a71a1..e1c3977 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -472,6 +472,7 @@ notmuch_threads_get (notmuch_threads_t *threads)
    threads->query->notmuch,
    doc_id,
    &threads->match_set,
+   threads->query->exclude_terms,
    threads->query->sort);
 }
 
diff --git a/lib/thread.cc b/lib/thread.cc
index 0435ee6..e976d64 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -214,7 +214,8 @@ _thread_cleanup_author (notmuch_thread_t *thread,
  */
 static void
 _thread_add_message (notmuch_thread_t *thread,
-     notmuch_message_t *message)
+     notmuch_message_t *message,
+     notmuch_string_list_t *exclude_terms)
 {
     notmuch_tags_t *tags;
     const char *tag;
@@ -262,6 +263,15 @@ _thread_add_message (notmuch_thread_t *thread,
  notmuch_tags_move_to_next (tags))
     {
  tag = notmuch_tags_get (tags);
+ /* Mark excluded messages. */
+ for (notmuch_string_node_t *term = exclude_terms->head; term;
+     term = term->next) {
+    /* We ignore initial 'K'. */
+    if (strcmp(tag, (term->string + 1)) == 0) {
+ notmuch_message_set_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED, TRUE);
+ break;
+    }
+ }
  g_hash_table_insert (thread->tags, xstrdup (tag), NULL);
     }
 }
@@ -321,7 +331,8 @@ _thread_add_matched_message (notmuch_thread_t *thread,
     _thread_set_subject_from_message (thread, message);
     }
 
-    thread->matched_messages++;
+    if (!notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED))
+ thread->matched_messages++;
 
     if (g_hash_table_lookup_extended (thread->message_hash,
     notmuch_message_get_message_id (message), NULL,
@@ -392,6 +403,7 @@ _notmuch_thread_create (void *ctx,
  notmuch_database_t *notmuch,
  unsigned int seed_doc_id,
  notmuch_doc_id_set_t *match_set,
+ notmuch_string_list_t *exclude_terms,
  notmuch_sort_t sort)
 {
     notmuch_thread_t *thread;
@@ -467,7 +479,7 @@ _notmuch_thread_create (void *ctx,
  if (doc_id == seed_doc_id)
     message = seed_message;
 
- _thread_add_message (thread, message);
+ _thread_add_message (thread, message, exclude_terms);
 
  if ( _notmuch_doc_id_set_contains (match_set, doc_id)) {
     _notmuch_doc_id_set_remove (match_set, doc_id);
--
1.7.2.3

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Mark Walters]

The function is
notmuch_thread_get_flag_messages
(notmuch_thread_t *thread, unsigned int flag_mask, unsigned int flags)

and returns the number of messages with the specified flags on flag_mask.

This generalises the existing function
notmuch_thread_get_total_messages and
notmuch_thread_get_matched_messages which are retained to maintain
compatibility.
---
 lib/message.cc |    6 +++---
 lib/notmuch.h  |   15 +++++++++++++--
 lib/thread.cc  |   39 ++++++++++++++++++++++++++-------------
 3 files changed, 42 insertions(+), 18 deletions(-)

diff --git a/lib/message.cc b/lib/message.cc
index 0075425..d60da83 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -746,7 +746,7 @@ notmuch_bool_t
 notmuch_message_get_flag (notmuch_message_t *message,
   notmuch_message_flag_t flag)
 {
-    return message->flags & (1 << flag);
+    return message->flags & flag;
 }
 
 void
@@ -754,9 +754,9 @@ notmuch_message_set_flag (notmuch_message_t *message,
   notmuch_message_flag_t flag, notmuch_bool_t enable)
 {
     if (enable)
- message->flags |= (1 << flag);
+ message->flags |= flag;
     else
- message->flags &= ~(1 << flag);
+ message->flags &= ~flag;
 }
 
 time_t
diff --git a/lib/notmuch.h b/lib/notmuch.h
index f75afae..c02e7f4 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -654,6 +654,16 @@ notmuch_thread_get_thread_id (notmuch_thread_t *thread);
 int
 notmuch_thread_get_total_messages (notmuch_thread_t *thread);
 
+/* Get the number of messages in 'thread' which have the specified
+ * flags on flag_mask.
+ *
+ * This is a more general interface than
+ * notmuch_thread_get_total_messages or
+ * notmuch_thread_get_matched_messages
+ */
+int
+notmuch_thread_get_flag_messages (notmuch_thread_t *thread, unsigned int flag_mask, unsigned int flags);
+
 /* Get a notmuch_messages_t iterator for the top-level messages in
  * 'thread'.
  *
@@ -902,8 +912,9 @@ notmuch_message_get_filenames (notmuch_message_t *message);
 
 /* Message flags */
 typedef enum _notmuch_message_flag {
-    NOTMUCH_MESSAGE_FLAG_MATCH,
-    NOTMUCH_MESSAGE_FLAG_EXCLUDED
+    NOTMUCH_MESSAGE_FLAG_MATCH = (1<<0),
+    NOTMUCH_MESSAGE_FLAG_EXCLUDED = (1<<1),
+    NOTMUCH_MESSAGE_FLAG_MAX  = (1<<2)
 } notmuch_message_flag_t;
 
 /* Get a value of a flag for the email corresponding to 'message'. */
diff --git a/lib/thread.cc b/lib/thread.cc
index e976d64..542f7f4 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -37,8 +37,7 @@ struct visible _notmuch_thread {
 
     notmuch_message_list_t *message_list;
     GHashTable *message_hash;
-    int total_messages;
-    int matched_messages;
+    int flag_count_messages[NOTMUCH_MESSAGE_FLAG_MAX];
     time_t oldest;
     time_t newest;
 };
@@ -226,7 +225,6 @@ _thread_add_message (notmuch_thread_t *thread,
 
     _notmuch_message_list_add_message (thread->message_list,
        talloc_steal (thread, message));
-    thread->total_messages++;
 
     g_hash_table_insert (thread->message_hash,
  xstrdup (notmuch_message_get_message_id (message)),
@@ -319,21 +317,18 @@ _thread_add_matched_message (notmuch_thread_t *thread,
 
     date = notmuch_message_get_date (message);
 
-    if (date < thread->oldest || ! thread->matched_messages) {
+    if (date < thread->oldest || ! notmuch_thread_get_matched_messages (thread)) {
  thread->oldest = date;
  if (sort == NOTMUCH_SORT_OLDEST_FIRST)
     _thread_set_subject_from_message (thread, message);
     }
 
-    if (date > thread->newest || ! thread->matched_messages) {
+    if (date > thread->newest || ! notmuch_thread_get_matched_messages (thread)) {
  thread->newest = date;
  if (sort != NOTMUCH_SORT_OLDEST_FIRST)
     _thread_set_subject_from_message (thread, message);
     }
 
-    if (!notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED))
- thread->matched_messages++;
-
     if (g_hash_table_lookup_extended (thread->message_hash,
     notmuch_message_get_message_id (message), NULL,
     (void **) &hashed_message)) {
@@ -411,7 +406,7 @@ _notmuch_thread_create (void *ctx,
     const char *thread_id;
     char *thread_id_query_string;
     notmuch_query_t *thread_id_query;
-
+    int i;
     notmuch_messages_t *messages;
     notmuch_message_t *message;
 
@@ -457,8 +452,8 @@ _notmuch_thread_create (void *ctx,
     thread->message_hash = g_hash_table_new_full (g_str_hash, g_str_equal,
   free, NULL);
 
-    thread->total_messages = 0;
-    thread->matched_messages = 0;
+    for (i = 0; i < NOTMUCH_MESSAGE_FLAG_MAX; i++)
+ thread->flag_count_messages[i] = 0;
     thread->oldest = 0;
     thread->newest = 0;
 
@@ -473,6 +468,7 @@ _notmuch_thread_create (void *ctx,
  notmuch_messages_move_to_next (messages))
     {
  unsigned int doc_id;
+ unsigned int message_flags;
 
  message = notmuch_messages_get (messages);
  doc_id = _notmuch_message_get_doc_id (message);
@@ -485,6 +481,10 @@ _notmuch_thread_create (void *ctx,
     _notmuch_doc_id_set_remove (match_set, doc_id);
     _thread_add_matched_message (thread, message, sort);
  }
+ message_flags =
+    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH) |
+    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED);
+ thread->flag_count_messages[message_flags]++;
 
  _notmuch_message_close (message);
     }
@@ -511,15 +511,28 @@ notmuch_thread_get_thread_id (notmuch_thread_t *thread)
 }
 
 int
+notmuch_thread_get_flag_messages (notmuch_thread_t *thread, unsigned int flag_mask, unsigned int flags)
+{
+    unsigned int i;
+    int count = 0;
+    for (i = 0; i < NOTMUCH_MESSAGE_FLAG_MAX; i++)
+ if ((i & flag_mask) == (flags & flag_mask))
+    count += thread->flag_count_messages[i];
+    return count;
+}
+
+int
 notmuch_thread_get_total_messages (notmuch_thread_t *thread)
 {
-    return thread->total_messages;
+    return notmuch_thread_get_flag_messages (thread, 0, 0);
 }
 
 int
 notmuch_thread_get_matched_messages (notmuch_thread_t *thread)
 {
-    return thread->matched_messages;
+    return notmuch_thread_get_flag_messages (thread,
+     NOTMUCH_MESSAGE_FLAG_MATCH | NOTMUCH_MESSAGE_FLAG_EXCLUDED,
+     NOTMUCH_MESSAGE_FLAG_MATCH);
 }
 
 const char *
--
1.7.2.3

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Mark Walters]

This adds the excludes to notmuch-show.c. We do not exclude when only
a single message (or part) is requested. notmuch-show will output the
exclude information when either text or json format is requested. As
this changes the output from notmuch-show it breaks many tests (in a
trivial and expected fashion).
---
 notmuch-show.c |   24 ++++++++++++++++++++----
 1 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index dec799c..108f13b 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -193,10 +193,11 @@ _get_one_line_summary (const void *ctx, notmuch_message_t *message)
 static void
 format_message_text (unused (const void *ctx), notmuch_message_t *message, int indent)
 {
-    printf ("id:%s depth:%d match:%d filename:%s\n",
+    printf ("id:%s depth:%d match:%d excluded:%d filename:%s\n",
     notmuch_message_get_message_id (message),
     indent,
-    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH),
+    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH) ? 1 : 0,
+    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED) ? 1 : 0,
     notmuch_message_get_filename (message));
 }
 
@@ -212,9 +213,10 @@ format_message_json (const void *ctx, notmuch_message_t *message, unused (int in
     date = notmuch_message_get_date (message);
     relative_date = notmuch_time_relative_date (ctx, date);
 
-    printf ("\"id\": %s, \"match\": %s, \"filename\": %s, \"timestamp\": %ld, \"date_relative\": \"%s\", \"tags\": [",
+    printf ("\"id\": %s, \"match\": %s, \"excluded\": %s, \"filename\": %s, \"timestamp\": %ld, \"date_relative\": \"%s\", \"tags\": [",
     json_quote_str (ctx_quote, notmuch_message_get_message_id (message)),
     notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH) ? "true" : "false",
+    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED) ? "true" : "false",
     json_quote_str (ctx_quote, notmuch_message_get_filename (message)),
     date, relative_date);
 
@@ -1059,9 +1061,13 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
     char *opt;
     const notmuch_show_format_t *format = &format_text;
     notmuch_show_params_t params;
+    const char **search_exclude_tags;
+    size_t search_exclude_tags_length;
     int mbox = 0;
     int format_specified = 0;
     int i;
+    notmuch_bool_t no_exclude = FALSE;
+    unsigned int j;
 
     params.entire_thread = 0;
     params.raw = 0;
@@ -1098,6 +1104,8 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
     params.part = atoi(argv[i] + sizeof ("--part=") - 1);
  } else if (STRNCMP_LITERAL (argv[i], "--entire-thread") == 0) {
     params.entire_thread = 1;
+ } else if (STRNCMP_LITERAL (argv[i], "--no-exclude") == 0) {
+    no_exclude = TRUE;
  } else if ((STRNCMP_LITERAL (argv[i], "--verify") == 0) ||
    (STRNCMP_LITERAL (argv[i], "--decrypt") == 0)) {
     if (params.cryptoctx == NULL) {
@@ -1167,10 +1175,18 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
     if (params.raw && params.part < 0)
  params.part = 0;
 
+    /* If a single message is requested we do not use search_excludes. */
     if (params.part >= 0)
  return do_show_single (ctx, query, format, &params);
-    else
+    else {
+ if (!no_exclude) {
+    search_exclude_tags = notmuch_config_get_search_exclude_tags
+ (config, &search_exclude_tags_length);
+    for (j = 0; j < search_exclude_tags_length; j++)
+ notmuch_query_add_tag_exclude (query, search_exclude_tags[j]);
+ }
  return do_show (ctx, query, format, &params);
+    }
 
     notmuch_query_destroy (query);
     notmuch_database_close (notmuch);
--
1.7.2.3

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Mark Walters]

notmuch show outputs the exclude flag so many tests using notmuch
show failed. This commit adds "excluded:0" or "excluded: false" to
the expected outputs. After this commit there should be no failing
tests.
---
 test/crypto        |    9 ++++++++-
 test/encoding      |    2 +-
 test/json          |    6 +++---
 test/maildir-sync  |    1 +
 test/multipart     |    4 ++--
 test/thread-naming |   16 ++++++++--------
 6 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/test/crypto b/test/crypto
index 446a58b..f29d959 100755
--- a/test/crypto
+++ b/test/crypto
@@ -43,6 +43,7 @@ output=$(notmuch show --format=json --verify subject:"test signed message 001" \
     | sed -e 's|"created": [1234567890]*|"created": 946728000|')
 expected='[[[{"id": "XXXXX",
  "match": true,
+ "excluded": false,
  "filename": "YYYYY",
  "timestamp": 946728000,
  "date_relative": "2000-01-01",
@@ -77,6 +78,7 @@ output=$(notmuch show --format=json --verify subject:"test signed message 001" \
     | sed -e 's|"created": [1234567890]*|"created": 946728000|')
 expected='[[[{"id": "XXXXX",
  "match": true,
+ "excluded": false,
  "filename": "YYYYY",
  "timestamp": 946728000,
  "date_relative": "2000-01-01",
@@ -113,6 +115,7 @@ output=$(notmuch show --format=json --verify subject:"test signed message 001" \
     | sed -e 's|"created": [1234567890]*|"created": 946728000|')
 expected='[[[{"id": "XXXXX",
  "match": true,
+ "excluded": false,
  "filename": "YYYYY",
  "timestamp": 946728000,
  "date_relative": "2000-01-01",
@@ -153,7 +156,7 @@ test_begin_subtest "decryption, --format=text"
 output=$(notmuch show --format=text --decrypt subject:"test encrypted message 001" \
     | notmuch_show_sanitize_all \
     | sed -e 's|"created": [1234567890]*|"created": 946728000|')
-expected='message{ id:XXXXX depth:0 match:1 filename:XXXXX
+expected='message{ id:XXXXX depth:0 match:1 excluded:0 filename:XXXXX
 header{
 Notmuch Test Suite <[hidden email]> (2000-01-01) (encrypted inbox)
 Subject: test encrypted message 001
@@ -187,6 +190,7 @@ output=$(notmuch show --format=json --decrypt subject:"test encrypted message 00
     | sed -e 's|"created": [1234567890]*|"created": 946728000|')
 expected='[[[{"id": "XXXXX",
  "match": true,
+ "excluded": false,
  "filename": "YYYYY",
  "timestamp": 946728000,
  "date_relative": "2000-01-01",
@@ -242,6 +246,7 @@ output=$(notmuch show --format=json --decrypt subject:"test encrypted message 00
     | sed -e 's|"created": [1234567890]*|"created": 946728000|')
 expected='[[[{"id": "XXXXX",
  "match": true,
+ "excluded": false,
  "filename": "YYYYY",
  "timestamp": 946728000,
  "date_relative": "2000-01-01",
@@ -277,6 +282,7 @@ output=$(notmuch show --format=json --decrypt subject:"test encrypted message 00
     | sed -e 's|"created": [1234567890]*|"created": 946728000|')
 expected='[[[{"id": "XXXXX",
  "match": true,
+ "excluded": false,
  "filename": "YYYYY",
  "timestamp": 946728000,
  "date_relative": "2000-01-01",
@@ -332,6 +338,7 @@ output=$(notmuch show --format=json --verify subject:"test signed message 001" \
     | sed -e 's|"created": [1234567890]*|"created": 946728000|')
 expected='[[[{"id": "XXXXX",
  "match": true,
+ "excluded": false,
  "filename": "YYYYY",
  "timestamp": 946728000,
  "date_relative": "2000-01-01",
diff --git a/test/encoding b/test/encoding
index 33259c1..a872345 100755
--- a/test/encoding
+++ b/test/encoding
@@ -6,7 +6,7 @@ test_begin_subtest "Message with text of unknown charset"
 add_message '[content-type]="text/plain; charset=unknown-8bit"' \
     "[body]=irrelevant"
 output=$(notmuch show id:${gen_msg_id} 2>&1 | notmuch_show_sanitize)
-test_expect_equal "$output" "message{ id:msg-001@notmuch-test-suite depth:0 match:1 filename:/XXX/mail/msg-001
+test_expect_equal "$output" "message{ id:msg-001@notmuch-test-suite depth:0 match:1 excluded:0 filename:/XXX/mail/msg-001
 header{
 Notmuch Test Suite <[hidden email]> (2001-01-05) (inbox unread)
 Subject: Test message #1
diff --git a/test/json b/test/json
index 7df4380..f95fcf8 100755
--- a/test/json
+++ b/test/json
@@ -5,7 +5,7 @@ test_description="--format=json output"
 test_begin_subtest "Show message: json"
 add_message "[subject]=\"json-show-subject\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"json-show-message\""
 output=$(notmuch show --format=json "json-show-message")
-test_expect_equal "$output" "[[[{\"id\": \"${gen_msg_id}\", \"match\": true, \"filename\": \"${gen_msg_filename}\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\",\"unread\"], \"headers\": {\"Subject\": \"json-show-subject\", \"From\": \"Notmuch Test Suite <[hidden email]>\", \"To\": \"Notmuch Test Suite <[hidden email]>\", \"Cc\": \"\", \"Bcc\": \"\", \"Date\": \"Sat, 01 Jan 2000 12:00:00 -0000\"}, \"body\": [{\"id\": 1, \"content-type\": \"text/plain\", \"content\": \"json-show-message\n\"}]}, []]]]"
+test_expect_equal "$output" "[[[{\"id\": \"${gen_msg_id}\", \"match\": true, \"excluded\": false, \"filename\": \"${gen_msg_filename}\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\",\"unread\"], \"headers\": {\"Subject\": \"json-show-subject\", \"From\": \"Notmuch Test Suite <[hidden email]>\", \"To\": \"Notmuch Test Suite <[hidden email]>\", \"Cc\": \"\", \"Bcc\": \"\", \"Date\": \"Sat, 01 Jan 2000 12:00:00 -0000\"}, \"body\": [{\"id\": 1, \"content-type\": \"text/plain\", \"content\": \"json-show-message\n\"}]}, []]]]"
 
 test_begin_subtest "Search message: json"
 add_message "[subject]=\"json-search-subject\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"json-search-message\""
@@ -22,7 +22,7 @@ test_expect_equal "$output" "[{\"thread\": \"XXX\",
 test_begin_subtest "Show message: json, utf-8"
 add_message "[subject]=\"json-show-utf8-body-sübjéct\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"jsön-show-méssage\""
 output=$(notmuch show --format=json "jsön-show-méssage")
-test_expect_equal "$output" "[[[{\"id\": \"${gen_msg_id}\", \"match\": true, \"filename\": \"${gen_msg_filename}\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\",\"unread\"], \"headers\": {\"Subject\": \"json-show-utf8-body-sübjéct\", \"From\": \"Notmuch Test Suite <[hidden email]>\", \"To\": \"Notmuch Test Suite <[hidden email]>\", \"Cc\": \"\", \"Bcc\": \"\", \"Date\": \"Sat, 01 Jan 2000 12:00:00 -0000\"}, \"body\": [{\"id\": 1, \"content-type\": \"text/plain\", \"content\": \"jsön-show-méssage\n\"}]}, []]]]"
+test_expect_equal "$output" "[[[{\"id\": \"${gen_msg_id}\", \"match\": true, \"excluded\": false, \"filename\": \"${gen_msg_filename}\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\",\"unread\"], \"headers\": {\"Subject\": \"json-show-utf8-body-sübjéct\", \"From\": \"Notmuch Test Suite <[hidden email]>\", \"To\": \"Notmuch Test Suite <[hidden email]>\", \"Cc\": \"\", \"Bcc\": \"\", \"Date\": \"Sat, 01 Jan 2000 12:00:00 -0000\"}, \"body\": [{\"id\": 1, \"content-type\": \"text/plain\", \"content\": \"jsön-show-méssage\n\"}]}, []]]]"
 
 test_begin_subtest "Show message: json, inline attachment filename"
 subject='json-show-inline-attachment-filename'
@@ -35,7 +35,7 @@ emacs_deliver_message \
      (insert \"Message-ID: <$id>\n\")"
 output=$(notmuch show --format=json "id:$id")
 filename=$(notmuch search --output=files "id:$id")
-test_expect_equal "$output" "[[[{\"id\": \"$id\", \"match\": true, \"filename\": \"$filename\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\"], \"headers\": {\"Subject\": \"$subject\", \"From\": \"Notmuch Test Suite <[hidden email]>\", \"To\": \"[hidden email]\", \"Cc\": \"\", \"Bcc\": \"\", \"Date\": \"01 Jan 2000 12:00:00 -0000\"}, \"body\": [{\"id\": 1, \"content-type\": \"multipart/mixed\", \"content\": [{\"id\": 2, \"content-type\": \"text/plain\", \"content\": \"This is a test message with inline attachment with a filename\"}, {\"id\": 3, \"content-type\": \"application/octet-stream\", \"filename\": \"README\"}]}]}, []]]]"
+test_expect_equal "$output" "[[[{\"id\": \"$id\", \"match\": true, \"excluded\": false, \"filename\": \"$filename\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\"], \"headers\": {\"Subject\": \"$subject\", \"From\": \"Notmuch Test Suite <[hidden email]>\", \"To\": \"[hidden email]\", \"Cc\": \"\", \"Bcc\": \"\", \"Date\": \"01 Jan 2000 12:00:00 -0000\"}, \"body\": [{\"id\": 1, \"content-type\": \"multipart/mixed\", \"content\": [{\"id\": 2, \"content-type\": \"text/plain\", \"content\": \"This is a test message with inline attachment with a filename\"}, {\"id\": 3, \"content-type\": \"application/octet-stream\", \"filename\": \"README\"}]}]}, []]]]"
 
 test_begin_subtest "Search message: json, utf-8"
 add_message "[subject]=\"json-search-utf8-body-sübjéct\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"jsön-search-méssage\""
diff --git a/test/maildir-sync b/test/maildir-sync
index d5872a5..dfa3966 100755
--- a/test/maildir-sync
+++ b/test/maildir-sync
@@ -46,6 +46,7 @@ test_begin_subtest "notmuch show works with renamed file (without notmuch new)"
 output=$(notmuch show --format=json id:${gen_msg_id} | filter_show_json)
 test_expect_equal "$output" '[[[{"id": "adding-replied-tag@notmuch-test-suite",
 "match": true,
+"excluded": false,
 "filename": "MAIL_DIR/cur/adding-replied-tag:2,RS",
 "timestamp": 978709437,
 "date_relative": "2001-01-05",
diff --git a/test/multipart b/test/multipart
index 2dd73f5..3d2dcfc 100755
--- a/test/multipart
+++ b/test/multipart
@@ -108,7 +108,7 @@ notmuch new > /dev/null
 test_begin_subtest "--format=text --part=0, full message"
 notmuch show --format=text --part=0 'id:[hidden email]' >OUTPUT
 cat <<EOF >EXPECTED
-message{ id:[hidden email] depth:0 match:1 filename:${MAIL_DIR}/multipart
+message{ id:[hidden email] depth:0 match:1 excluded:0 filename:${MAIL_DIR}/multipart
 header{
 Carl Worth <[hidden email]> (2001-01-05) (attachment inbox signed unread)
 Subject: Multipart message
@@ -322,7 +322,7 @@ notmuch show --format=json --part=0 'id:[hidden email]' | s
 echo >>OUTPUT # expect *no* newline at end of output
 cat <<EOF >EXPECTED
 
-{"id": "[hidden email]", "match": true, "filename": "${MAIL_DIR}/multipart", "timestamp": 978709437, "date_relative": "2001-01-05", "tags": ["attachment","inbox","signed","unread"], "headers": {"Subject": "Multipart message", "From": "Carl Worth <[hidden email]>", "To": "[hidden email]", "Cc": "", "Bcc": "", "Date": "Fri, 05 Jan 2001 15:43:57 +0000"}, "body": [
+{"id": "[hidden email]", "match": true, "excluded": false, "filename": "${MAIL_DIR}/multipart", "timestamp": 978709437, "date_relative": "2001-01-05", "tags": ["attachment","inbox","signed","unread"], "headers": {"Subject": "Multipart message", "From": "Carl Worth <[hidden email]>", "To": "[hidden email]", "Cc": "", "Bcc": "", "Date": "Fri, 05 Jan 2001 15:43:57 +0000"}, "body": [
 {"id": 1, "content-type": "multipart/signed", "content": [
 {"id": 2, "content-type": "multipart/mixed", "content": [
 {"id": 3, "content-type": "message/rfc822", "content": [{"headers": {"From": "Carl Worth <[hidden email]>", "To": "[hidden email]", "Subject": "html message", "Date": "Fri, 05 Jan 2001 15:42:57 +0000"}, "body": [
diff --git a/test/thread-naming b/test/thread-naming
index 2ce9216..f5f1de3 100755
--- a/test/thread-naming
+++ b/test/thread-naming
@@ -65,7 +65,7 @@ test_expect_equal "$output" "thread:XXX   2001-01-12 [6/8] Notmuch Test Suite; t
 
 test_begin_subtest 'Test order of messages in "notmuch show"'
 output=$(notmuch show thread-naming | notmuch_show_sanitize)
-test_expect_equal "$output" "message{ id:msg-$(printf "%03d" $first)@notmuch-test-suite depth:0 match:1 filename:/XXX/mail/msg-$(printf "%03d" $first)
+test_expect_equal "$output" "message{ id:msg-$(printf "%03d" $first)@notmuch-test-suite depth:0 match:1 excluded:0 filename:/XXX/mail/msg-$(printf "%03d" $first)
 header{
 Notmuch Test Suite <[hidden email]> (2001-01-05) (unread)
 Subject: thread-naming: Initial thread subject
@@ -79,7 +79,7 @@ This is just a test message (#$first)
 part}
 body}
 message}
-message{ id:msg-$(printf "%03d" $((first + 1)))@notmuch-test-suite depth:1 match:1 filename:/XXX/mail/msg-$(printf "%03d" $((first + 1)))
+message{ id:msg-$(printf "%03d" $((first + 1)))@notmuch-test-suite depth:1 match:1 excluded:0 filename:/XXX/mail/msg-$(printf "%03d" $((first + 1)))
 header{
 Notmuch Test Suite <[hidden email]> (2001-01-06) (inbox unread)
 Subject: thread-naming: Older changed subject
@@ -93,7 +93,7 @@ This is just a test message (#$((first + 1)))
 part}
 body}
 message}
-message{ id:msg-$(printf "%03d" $((first + 2)))@notmuch-test-suite depth:1 match:1 filename:/XXX/mail/msg-$(printf "%03d" $((first + 2)))
+message{ id:msg-$(printf "%03d" $((first + 2)))@notmuch-test-suite depth:1 match:1 excluded:0 filename:/XXX/mail/msg-$(printf "%03d" $((first + 2)))
 header{
 Notmuch Test Suite <[hidden email]> (2001-01-07) (inbox unread)
 Subject: thread-naming: Newer changed subject
@@ -107,7 +107,7 @@ This is just a test message (#$((first + 2)))
 part}
 body}
 message}
-message{ id:msg-$(printf "%03d" $((first + 3)))@notmuch-test-suite depth:1 match:1 filename:/XXX/mail/msg-$(printf "%03d" $((first + 3)))
+message{ id:msg-$(printf "%03d" $((first + 3)))@notmuch-test-suite depth:1 match:1 excluded:0 filename:/XXX/mail/msg-$(printf "%03d" $((first + 3)))
 header{
 Notmuch Test Suite <[hidden email]> (2001-01-08) (unread)
 Subject: thread-naming: Final thread subject
@@ -121,7 +121,7 @@ This is just a test message (#$((first + 3)))
 part}
 body}
 message}
-message{ id:msg-$(printf "%03d" $((first + 4)))@notmuch-test-suite depth:1 match:1 filename:/XXX/mail/msg-$(printf "%03d" $((first + 4)))
+message{ id:msg-$(printf "%03d" $((first + 4)))@notmuch-test-suite depth:1 match:1 excluded:0 filename:/XXX/mail/msg-$(printf "%03d" $((first + 4)))
 header{
 Notmuch Test Suite <[hidden email]> (2001-01-09) (inbox unread)
 Subject: Re: thread-naming: Initial thread subject
@@ -135,7 +135,7 @@ This is just a test message (#$((first + 4)))
 part}
 body}
 message}
-message{ id:msg-$(printf "%03d" $((first + 5)))@notmuch-test-suite depth:1 match:1 filename:/XXX/mail/msg-$(printf "%03d" $((first + 5)))
+message{ id:msg-$(printf "%03d" $((first + 5)))@notmuch-test-suite depth:1 match:1 excluded:0 filename:/XXX/mail/msg-$(printf "%03d" $((first + 5)))
 header{
 Notmuch Test Suite <[hidden email]> (2001-01-10) (inbox unread)
 Subject: Aw: thread-naming: Initial thread subject
@@ -149,7 +149,7 @@ This is just a test message (#$((first + 5)))
 part}
 body}
 message}
-message{ id:msg-$(printf "%03d" $((first + 6)))@notmuch-test-suite depth:1 match:1 filename:/XXX/mail/msg-$(printf "%03d" $((first + 6)))
+message{ id:msg-$(printf "%03d" $((first + 6)))@notmuch-test-suite depth:1 match:1 excluded:0 filename:/XXX/mail/msg-$(printf "%03d" $((first + 6)))
 header{
 Notmuch Test Suite <[hidden email]> (2001-01-11) (inbox unread)
 Subject: Vs: thread-naming: Initial thread subject
@@ -163,8 +163,7 @@ This is just a test message (#$((first + 6)))
 part}
 body}
 message}
-message{ id:msg-$(printf "%03d" $((first + 7)))@notmuch-test-suite depth:1 match:1 filename:/XXX/mail/msg-$(printf "%03d" $((first + 7)))
+message{ id:msg-$(printf "%03d" $((first + 7)))@notmuch-test-suite depth:1 match:1 excluded:0 filename:/XXX/mail/msg-$(printf "%03d" $((first + 7)))
 header{
 Notmuch Test Suite <[hidden email]> (2001-01-12) (inbox unread)
 Subject: Sv: thread-naming: Initial thread subject
--
1.7.2.3

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Mark Walters]

In all cases of notmuch count/search/show where the results returned
cannot reflect the exclude flag return just the matched not-excluded
results. If the caller wishes to have all the matched results (i.e.,
including the excluded ones) they should call with the
--no-exclude option.

The relevant cases are
    count: both threads and messages
    search: all cases except the summary view
    show: mbox format
---
 notmuch-count.c  |    2 ++
 notmuch-search.c |    9 +++++++++
 notmuch-show.c   |    5 +++++
 3 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/notmuch-count.c b/notmuch-count.c
index 5364507..46b76ae 100644
--- a/notmuch-count.c
+++ b/notmuch-count.c
@@ -88,6 +88,8 @@ notmuch_count_command (void *ctx, int argc, char *argv[])
     notmuch_query_add_tag_exclude (query, search_exclude_tags[i]);
     }
 
+    notmuch_query_set_omit_excluded_messages (query, TRUE);
+
     switch (output) {
     case OUTPUT_MESSAGES:
  printf ("%u\n", notmuch_query_count_messages (query));
diff --git a/notmuch-search.c b/notmuch-search.c
index 43ec90b..d2b2488 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -207,6 +207,9 @@ do_search_threads (const search_format_t *format,
     int first_thread = 1;
     int i;
 
+    if (output == OUTPUT_THREADS)
+ notmuch_query_set_omit_excluded_messages (query, TRUE);
+
     if (offset < 0) {
  offset += notmuch_query_count_threads (query);
  if (offset < 0)
@@ -297,6 +300,8 @@ do_search_messages (const search_format_t *format,
     int first_message = 1;
     int i;
 
+    notmuch_query_set_omit_excluded_messages (query, TRUE);
+
     if (offset < 0) {
  offset += notmuch_query_count_messages (query);
  if (offset < 0)
@@ -368,6 +373,10 @@ do_search_tags (notmuch_database_t *notmuch,
     const char *tag;
     int first_tag = 1;
 
+    notmuch_query_set_omit_excluded_messages (query, TRUE);
+    /* should the following only special case if no excluded terms
+     * specified? */
+
     /* Special-case query of "*" for better performance. */
     if (strcmp (notmuch_query_get_query_string (query), "*") == 0) {
  tags = notmuch_database_get_all_tags (notmuch);
diff --git a/notmuch-show.c b/notmuch-show.c
index 108f13b..924e7ea 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1166,6 +1166,11 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
  return 1;
     }
 
+    /* if format=mbox then we can not output excluded messages as
+     * there is no way to make the exclude flag available */
+    if (mbox)
+ notmuch_query_set_omit_excluded_messages (query, TRUE);
+
     /* if part was requested and format was not specified, use format=raw */
     if (params.part >= 0 && !format_specified)
  format = &format_raw;
--
1.7.2.3

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Mark Walters]

Show mode will recognize the exclude flag by not opening excluding
messages by default, and will start at the first matching non-excluded
message. If there are no matching non-excluded messages it will go to
the first matching (necessarily excluded) message.
---
 emacs/notmuch-show.el |   18 +++++++++++++++++-
 1 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 84ac624..8efadf6 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -905,7 +905,8 @@ current buffer, if possible."
 
     ;; Message visibility depends on whether it matched the search
     ;; criteria.
-    (notmuch-show-message-visible msg (plist-get msg :match))))
+    (notmuch-show-message-visible msg (and (plist-get msg :match)
+   (not (plist-get msg :excluded))))))
 
 (defun notmuch-show-insert-tree (tree depth)
   "Insert the message tree TREE at depth DEPTH in the current thread."
@@ -1015,6 +1016,9 @@ buffer."
     ;; Move straight to the first open message
     (unless (notmuch-show-message-visible-p)
       (notmuch-show-next-open-message))
+    (when (eq (point) (point-max))
+      (goto-char (point-min))
+      (notmuch-show-next-matching-message))
 
     ;; Set the header line to the subject of the first open message.
     (setq header-line-format (notmuch-show-strip-re (notmuch-show-get-subject)))
@@ -1417,6 +1421,18 @@ any effects from previous calls to
   (notmuch-show-message-adjust))
       (goto-char (point-max)))))
 
+(defun notmuch-show-next-matching-message ()
+  "Show the next matching message."
+  (interactive)
+  (let (r)
+    (while (and (setq r (notmuch-show-goto-message-next))
+ (not (notmuch-show-get-prop :match))))
+    (if r
+ (progn
+  (notmuch-show-mark-read)
+  (notmuch-show-message-adjust))
+      (goto-char (point-max)))))
+
 (defun notmuch-show-previous-open-message ()
   "Show the previous message."
   (interactive)
--
1.7.2.3

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Jani Nikula]

Hi Mark -

This is my first look at any version of the series; apologies if I'm
clueless about some details... Please find some comments below.

BR,
Jani.


On Thu,  2 Feb 2012 17:43:35 +0000, Mark Walters <[hidden email]> wrote:
> The function is
> notmuch_thread_get_flag_messages
> (notmuch_thread_t *thread, unsigned int flag_mask, unsigned int flags)
>
> and returns the number of messages with the specified flags on flag_mask.

Is the purpose of this function to get the count of messages that have
certain flags set, certain flags not set, and certain flags don't-care?

At the very least, I think the documentation of the function should be
greatly improved.

I think the name of the function should be notmuch_thread_count_messages
which is like notmuch_query_count_messages, but for messages in threads
(and with some extra restrictions).

>
> This generalises the existing function
> notmuch_thread_get_total_messages and
> notmuch_thread_get_matched_messages which are retained to maintain
> compatibility.
> ---
>  lib/message.cc |    6 +++---
>  lib/notmuch.h  |   15 +++++++++++++--
>  lib/thread.cc  |   39 ++++++++++++++++++++++++++-------------
>  3 files changed, 42 insertions(+), 18 deletions(-)
>
> diff --git a/lib/message.cc b/lib/message.cc
> index 0075425..d60da83 100644
> --- a/lib/message.cc
> +++ b/lib/message.cc
> @@ -746,7 +746,7 @@ notmuch_bool_t
>  notmuch_message_get_flag (notmuch_message_t *message,
>    notmuch_message_flag_t flag)
>  {
> -    return message->flags & (1 << flag);
> +    return message->flags & flag;
>  }
>  
>  void
> @@ -754,9 +754,9 @@ notmuch_message_set_flag (notmuch_message_t *message,
>    notmuch_message_flag_t flag, notmuch_bool_t enable)
>  {
>      if (enable)
> - message->flags |= (1 << flag);
> + message->flags |= flag;
>      else
> - message->flags &= ~(1 << flag);
> + message->flags &= ~flag;
>  }
>  
>  time_t
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index f75afae..c02e7f4 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -654,6 +654,16 @@ notmuch_thread_get_thread_id (notmuch_thread_t *thread);
>  int
>  notmuch_thread_get_total_messages (notmuch_thread_t *thread);
>  
> +/* Get the number of messages in 'thread' which have the specified
> + * flags on flag_mask.
> + *
> + * This is a more general interface than
> + * notmuch_thread_get_total_messages or
> + * notmuch_thread_get_matched_messages
> + */
> +int
> +notmuch_thread_get_flag_messages (notmuch_thread_t *thread, unsigned int flag_mask, unsigned int flags);
> +
>  /* Get a notmuch_messages_t iterator for the top-level messages in
>   * 'thread'.
>   *
> @@ -902,8 +912,9 @@ notmuch_message_get_filenames (notmuch_message_t *message);
>  
>  /* Message flags */
>  typedef enum _notmuch_message_flag {
> -    NOTMUCH_MESSAGE_FLAG_MATCH,
> -    NOTMUCH_MESSAGE_FLAG_EXCLUDED
> +    NOTMUCH_MESSAGE_FLAG_MATCH = (1<<0),
> +    NOTMUCH_MESSAGE_FLAG_EXCLUDED = (1<<1),
> +    NOTMUCH_MESSAGE_FLAG_MAX  = (1<<2)

How are these used by the current lib users at the moment? How will they
break with this change?

Please align the assignments.

>  } notmuch_message_flag_t;
>  
>  /* Get a value of a flag for the email corresponding to 'message'. */
> diff --git a/lib/thread.cc b/lib/thread.cc
> index e976d64..542f7f4 100644
> --- a/lib/thread.cc
> +++ b/lib/thread.cc
> @@ -37,8 +37,7 @@ struct visible _notmuch_thread {
>  
>      notmuch_message_list_t *message_list;
>      GHashTable *message_hash;
> -    int total_messages;
> -    int matched_messages;
> +    int flag_count_messages[NOTMUCH_MESSAGE_FLAG_MAX];
>      time_t oldest;
>      time_t newest;
>  };
> @@ -226,7 +225,6 @@ _thread_add_message (notmuch_thread_t *thread,
>  
>      _notmuch_message_list_add_message (thread->message_list,
>         talloc_steal (thread, message));
> -    thread->total_messages++;
>  
>      g_hash_table_insert (thread->message_hash,
>   xstrdup (notmuch_message_get_message_id (message)),
> @@ -319,21 +317,18 @@ _thread_add_matched_message (notmuch_thread_t *thread,
>  
>      date = notmuch_message_get_date (message);
>  
> -    if (date < thread->oldest || ! thread->matched_messages) {
> +    if (date < thread->oldest || ! notmuch_thread_get_matched_messages (thread)) {
>   thread->oldest = date;
>   if (sort == NOTMUCH_SORT_OLDEST_FIRST)
>      _thread_set_subject_from_message (thread, message);
>      }
>  
> -    if (date > thread->newest || ! thread->matched_messages) {
> +    if (date > thread->newest || ! notmuch_thread_get_matched_messages (thread)) {
>   thread->newest = date;
>   if (sort != NOTMUCH_SORT_OLDEST_FIRST)
>      _thread_set_subject_from_message (thread, message);
>      }
>  
> -    if (!notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED))
> - thread->matched_messages++;
> -
>      if (g_hash_table_lookup_extended (thread->message_hash,
>      notmuch_message_get_message_id (message), NULL,
>      (void **) &hashed_message)) {
> @@ -411,7 +406,7 @@ _notmuch_thread_create (void *ctx,
>      const char *thread_id;
>      char *thread_id_query_string;
>      notmuch_query_t *thread_id_query;
> -
> +    int i;
>      notmuch_messages_t *messages;
>      notmuch_message_t *message;
>  
> @@ -457,8 +452,8 @@ _notmuch_thread_create (void *ctx,
>      thread->message_hash = g_hash_table_new_full (g_str_hash, g_str_equal,
>    free, NULL);
>  
> -    thread->total_messages = 0;
> -    thread->matched_messages = 0;
> +    for (i = 0; i < NOTMUCH_MESSAGE_FLAG_MAX; i++)
> + thread->flag_count_messages[i] = 0;

memset (thread->flag_count_messages, 0, sizeof(thread->flag_count_messages));

>      thread->oldest = 0;
>      thread->newest = 0;
>  
> @@ -473,6 +468,7 @@ _notmuch_thread_create (void *ctx,
>   notmuch_messages_move_to_next (messages))
>      {
>   unsigned int doc_id;
> + unsigned int message_flags;
>  
>   message = notmuch_messages_get (messages);
>   doc_id = _notmuch_message_get_doc_id (message);
> @@ -485,6 +481,10 @@ _notmuch_thread_create (void *ctx,
>      _notmuch_doc_id_set_remove (match_set, doc_id);
>      _thread_add_matched_message (thread, message, sort);
>   }
> + message_flags =
> +    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH) |
> +    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED);
> + thread->flag_count_messages[message_flags]++;

The first impression of using a set of flags as index is that there's a
bug. But this is to keep count of messages with certain flag sets rather
than total for each flag, right? I think this needs more comments, more
documentation. Already naming the field flag_set_message_counts or
similar would help greatly.

>  
>   _notmuch_message_close (message);
>      }
> @@ -511,15 +511,28 @@ notmuch_thread_get_thread_id (notmuch_thread_t *thread)
>  }
>  
>  int
> +notmuch_thread_get_flag_messages (notmuch_thread_t *thread, unsigned int flag_mask, unsigned int flags)
> +{
> +    unsigned int i;
> +    int count = 0;
> +    for (i = 0; i < NOTMUCH_MESSAGE_FLAG_MAX; i++)

ARRAY_SIZE (thread->flag_count_messages)

> + if ((i & flag_mask) == (flags & flag_mask))
> +    count += thread->flag_count_messages[i];
> +    return count;
> +}

I wonder if the same could be accomplished by using two flag mask
parameters, include_flag_mask and exclude_flag_mask. I'm thinking of the
usage, would it be easier to use:

notmuch_query_count_messages (thread, NOTMUCH_MESSAGE_FLAG_MATCH, NOTMUCH_MESSAGE_FLAG_EXCLUDED);

to get number of messages that have MATCH but not EXCLUDED? 0 as
include_flag_mask could still be special for "all", and you could use:

notmuch_query_count_messages (thread, 0, NOTMUCH_MESSAGE_FLAG_EXCLUDED);

Note the name change according to my earlier suggestion. It might be
wise to not export the function before the API is chrystal clear if
there is no pressing need to do so.

> +
> +int
>  notmuch_thread_get_total_messages (notmuch_thread_t *thread)
>  {
> -    return thread->total_messages;
> +    return notmuch_thread_get_flag_messages (thread, 0, 0);
>  }
>  
>  int
>  notmuch_thread_get_matched_messages (notmuch_thread_t *thread)
>  {
> -    return thread->matched_messages;
> +    return notmuch_thread_get_flag_messages (thread,
> +     NOTMUCH_MESSAGE_FLAG_MATCH | NOTMUCH_MESSAGE_FLAG_EXCLUDED,
> +     NOTMUCH_MESSAGE_FLAG_MATCH);
>  }
>  
>  const char *
> --
> 1.7.2.3
>
> _______________________________________________
> notmuch mailing list
> [hidden email]
> http://notmuchmail.org/mailman/listinfo/notmuch

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Jani Nikula]

On Thu,  2 Feb 2012 17:43:36 +0000, Mark Walters <[hidden email]> wrote:

> This adds the excludes to notmuch-show.c. We do not exclude when only
> a single message (or part) is requested. notmuch-show will output the
> exclude information when either text or json format is requested. As
> this changes the output from notmuch-show it breaks many tests (in a
> trivial and expected fashion).
> ---
>  notmuch-show.c |   24 ++++++++++++++++++++----
>  1 files changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/notmuch-show.c b/notmuch-show.c
> index dec799c..108f13b 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -193,10 +193,11 @@ _get_one_line_summary (const void *ctx, notmuch_message_t *message)
>  static void
>  format_message_text (unused (const void *ctx), notmuch_message_t *message, int indent)
>  {
> -    printf ("id:%s depth:%d match:%d filename:%s\n",
> +    printf ("id:%s depth:%d match:%d excluded:%d filename:%s\n",
>      notmuch_message_get_message_id (message),
>      indent,
> -    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH),
> +    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH) ? 1 : 0,
> +    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED) ? 1 : 0,
>      notmuch_message_get_filename (message));
>  }
>  
> @@ -212,9 +213,10 @@ format_message_json (const void *ctx, notmuch_message_t *message, unused (int in
>      date = notmuch_message_get_date (message);
>      relative_date = notmuch_time_relative_date (ctx, date);
>  
> -    printf ("\"id\": %s, \"match\": %s, \"filename\": %s, \"timestamp\": %ld, \"date_relative\": \"%s\", \"tags\": [",
> +    printf ("\"id\": %s, \"match\": %s, \"excluded\": %s, \"filename\": %s, \"timestamp\": %ld, \"date_relative\": \"%s\", \"tags\": [",
>      json_quote_str (ctx_quote, notmuch_message_get_message_id (message)),
>      notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH) ? "true" : "false",
> +    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED) ? "true" : "false",
>      json_quote_str (ctx_quote, notmuch_message_get_filename (message)),
>      date, relative_date);
>  
> @@ -1059,9 +1061,13 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
>      char *opt;
>      const notmuch_show_format_t *format = &format_text;
>      notmuch_show_params_t params;
> +    const char **search_exclude_tags;
> +    size_t search_exclude_tags_length;

Please move these within the if (!no_exclude) block.

>      int mbox = 0;
>      int format_specified = 0;
>      int i;
> +    notmuch_bool_t no_exclude = FALSE;
> +    unsigned int j;

Same. Or better yet, reuse i.

>  
>      params.entire_thread = 0;
>      params.raw = 0;
> @@ -1098,6 +1104,8 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
>      params.part = atoi(argv[i] + sizeof ("--part=") - 1);
>   } else if (STRNCMP_LITERAL (argv[i], "--entire-thread") == 0) {
>      params.entire_thread = 1;
> + } else if (STRNCMP_LITERAL (argv[i], "--no-exclude") == 0) {
> +    no_exclude = TRUE;
>   } else if ((STRNCMP_LITERAL (argv[i], "--verify") == 0) ||
>     (STRNCMP_LITERAL (argv[i], "--decrypt") == 0)) {
>      if (params.cryptoctx == NULL) {
> @@ -1167,10 +1175,18 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
>      if (params.raw && params.part < 0)
>   params.part = 0;
>  
> +    /* If a single message is requested we do not use search_excludes. */
>      if (params.part >= 0)
>   return do_show_single (ctx, query, format, &params);
> -    else
> +    else {

Nitpick: There's no rule about this, but I do like the style of using
braces for both branches if either branch needs them.

> + if (!no_exclude) {
> +    search_exclude_tags = notmuch_config_get_search_exclude_tags
> + (config, &search_exclude_tags_length);
> +    for (j = 0; j < search_exclude_tags_length; j++)
> + notmuch_query_add_tag_exclude (query, search_exclude_tags[j]);
> + }
>   return do_show (ctx, query, format, &params);
> +    }

Hmm, unreachable code below. Why doesn't the compiler complain?

>  
>      notmuch_query_destroy (query);
>      notmuch_database_close (notmuch);
> --
> 1.7.2.3
>
> _______________________________________________
> notmuch mailing list
> [hidden email]
> http://notmuchmail.org/mailman/listinfo/notmuch

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Mark Walters]

On Thu, 02 Feb 2012 23:55:33 +0200, Jani Nikula <[hidden email]> wrote:

>
> Hi Mark -
>
> This is my first look at any version of the series; apologies if I'm
> clueless about some details... Please find some comments below.
>
> BR,
> Jani.
>
>
> On Thu,  2 Feb 2012 17:43:35 +0000, Mark Walters <[hidden email]> wrote:
> > The function is
> > notmuch_thread_get_flag_messages
> > (notmuch_thread_t *thread, unsigned int flag_mask, unsigned int flags)
> >
> > and returns the number of messages with the specified flags on flag_mask.
>
> Is the purpose of this function to get the count of messages that have
> certain flags set, certain flags not set, and certain flags don't-care?

Yes: I was trying to follow Austin's suggestion from
id:"[hidden email]" (although stupidly I didn't
follow his suggestion of a function name).

> At the very least, I think the documentation of the function should be
> greatly improved.
>
> I think the name of the function should be notmuch_thread_count_messages
> which is like notmuch_query_count_messages, but for messages in threads
> (and with some extra restrictions).

Yes I like your name; before I change it do you (and others) prefer it
to Austin's suggestion of notmuch_thread_count_flags. Or we could even
be more verbose with something like
notmuch_thread_count_messages_with_flags

> >  /* Message flags */
> >  typedef enum _notmuch_message_flag {
> > -    NOTMUCH_MESSAGE_FLAG_MATCH,
> > -    NOTMUCH_MESSAGE_FLAG_EXCLUDED
> > +    NOTMUCH_MESSAGE_FLAG_MATCH = (1<<0),
> > +    NOTMUCH_MESSAGE_FLAG_EXCLUDED = (1<<1),
> > +    NOTMUCH_MESSAGE_FLAG_MAX  = (1<<2)
>
> How are these used by the current lib users at the moment? How will they
> break with this change?

The only existing flag is NOTMUCH_MESSAGE_FLAG_MATCH: that is currently
zero but in the current code that is the bit offset of the flag; in my
version it is the actual bit for the flag (otherwise I think flag masks
end up very ugly). I believe all callers use notmuch_message_set_flag
and notmuch_message_get_flag so they should not notice the difference.

> Please align the assignments.

Will do.

> > @@ -457,8 +452,8 @@ _notmuch_thread_create (void *ctx,
> >      thread->message_hash = g_hash_table_new_full (g_str_hash, g_str_equal,
> >    free, NULL);
> >  
> > -    thread->total_messages = 0;
> > -    thread->matched_messages = 0;
> > +    for (i = 0; i < NOTMUCH_MESSAGE_FLAG_MAX; i++)
> > + thread->flag_count_messages[i] = 0;
>
> memset (thread->flag_count_messages, 0, sizeof(thread->flag_count_messages));

Will do

> >      thread->oldest = 0;
> >      thread->newest = 0;
> >  
> > @@ -473,6 +468,7 @@ _notmuch_thread_create (void *ctx,
> >   notmuch_messages_move_to_next (messages))
> >      {
> >   unsigned int doc_id;
> > + unsigned int message_flags;
> >  
> >   message = notmuch_messages_get (messages);
> >   doc_id = _notmuch_message_get_doc_id (message);
> > @@ -485,6 +481,10 @@ _notmuch_thread_create (void *ctx,
> >      _notmuch_doc_id_set_remove (match_set, doc_id);
> >      _thread_add_matched_message (thread, message, sort);
> >   }
> > + message_flags =
> > +    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH) |
> > +    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED);
> > + thread->flag_count_messages[message_flags]++;
>
> The first impression of using a set of flags as index is that there's a
> bug. But this is to keep count of messages with certain flag sets rather
> than total for each flag, right? I think this needs more comments, more
> documentation. Already naming the field flag_set_message_counts or
> similar would help greatly.

I will try and document it better: on first reading I parsed your name
as flag set (as verb) message counts whereas I assume you mean "flag
set" as a noun! I will see if I can come up with something though.

> >   _notmuch_message_close (message);
> >      }
> > @@ -511,15 +511,28 @@ notmuch_thread_get_thread_id (notmuch_thread_t *thread)
> >  }
> >  
> >  int
> > +notmuch_thread_get_flag_messages (notmuch_thread_t *thread, unsigned int flag_mask, unsigned int flags)
> > +{
> > +    unsigned int i;
> > +    int count = 0;
> > +    for (i = 0; i < NOTMUCH_MESSAGE_FLAG_MAX; i++)
>
> ARRAY_SIZE (thread->flag_count_messages)

ok

>
> > + if ((i & flag_mask) == (flags & flag_mask))
> > +    count += thread->flag_count_messages[i];
> > +    return count;
> > +}
>
> I wonder if the same could be accomplished by using two flag mask
> parameters, include_flag_mask and exclude_flag_mask. I'm thinking of the
> usage, would it be easier to use:
>
> notmuch_query_count_messages (thread, NOTMUCH_MESSAGE_FLAG_MATCH, NOTMUCH_MESSAGE_FLAG_EXCLUDED);
>
> to get number of messages that have MATCH but not EXCLUDED? 0 as
> include_flag_mask could still be special for "all", and you could use:
>
> notmuch_query_count_messages (thread, 0, NOTMUCH_MESSAGE_FLAG_EXCLUDED);
>
> Note the name change according to my earlier suggestion. It might be
> wise to not export the function before the API is chrystal clear if
> there is no pressing need to do so.

(I assume you mean notmuch_thread_count_messages.) Can I just check this
would return the number of messages which have all the flags  in
include_flag_mask and none of the flags in exclude_flag_mask?

I completely agree about leaving it until we have the API well worked
out. I wrote it in response to Austin's suggestion and then it looked
like it would useful in my attempts to remove the
notmuch_query_set_omit_excluded_messages API. However, those attempts
failed so it doesn't have any users yet.

Best wishes

Mark
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Mark Walters]

On Fri, 03 Feb 2012 00:08:32 +0200, Jani Nikula <[hidden email]> wrote:

> On Thu,  2 Feb 2012 17:43:36 +0000, Mark Walters <[hidden email]> wrote:
> > This adds the excludes to notmuch-show.c. We do not exclude when only
> > a single message (or part) is requested. notmuch-show will output the
> > exclude information when either text or json format is requested. As
> > this changes the output from notmuch-show it breaks many tests (in a
> > trivial and expected fashion).
> > ---
> >  notmuch-show.c |   24 ++++++++++++++++++++----
> >  1 files changed, 20 insertions(+), 4 deletions(-)
> >
> > diff --git a/notmuch-show.c b/notmuch-show.c
> > index dec799c..108f13b 100644
> > --- a/notmuch-show.c
> > +++ b/notmuch-show.c
> > @@ -193,10 +193,11 @@ _get_one_line_summary (const void *ctx, notmuch_message_t *message)
> >  static void
> >  format_message_text (unused (const void *ctx), notmuch_message_t *message, int indent)
> >  {
> > -    printf ("id:%s depth:%d match:%d filename:%s\n",
> > +    printf ("id:%s depth:%d match:%d excluded:%d filename:%s\n",
> >      notmuch_message_get_message_id (message),
> >      indent,
> > -    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH),
> > +    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH) ? 1 : 0,
> > +    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED) ? 1 : 0,
> >      notmuch_message_get_filename (message));
> >  }
> >  
> > @@ -212,9 +213,10 @@ format_message_json (const void *ctx, notmuch_message_t *message, unused (int in
> >      date = notmuch_message_get_date (message);
> >      relative_date = notmuch_time_relative_date (ctx, date);
> >  
> > -    printf ("\"id\": %s, \"match\": %s, \"filename\": %s, \"timestamp\": %ld, \"date_relative\": \"%s\", \"tags\": [",
> > +    printf ("\"id\": %s, \"match\": %s, \"excluded\": %s, \"filename\": %s, \"timestamp\": %ld, \"date_relative\": \"%s\", \"tags\": [",
> >      json_quote_str (ctx_quote, notmuch_message_get_message_id (message)),
> >      notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH) ? "true" : "false",
> > +    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED) ? "true" : "false",
> >      json_quote_str (ctx_quote, notmuch_message_get_filename (message)),
> >      date, relative_date);
> >  
> > @@ -1059,9 +1061,13 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
> >      char *opt;
> >      const notmuch_show_format_t *format = &format_text;
> >      notmuch_show_params_t params;
> > +    const char **search_exclude_tags;
> > +    size_t search_exclude_tags_length;
>
> Please move these within the if (!no_exclude) block.

Will do. (I forgot to move them in notmuch-show when doing notmuch-count
and notmuch-search)

> >      int mbox = 0;
> >      int format_specified = 0;
> >      int i;
> > +    notmuch_bool_t no_exclude = FALSE;
> > +    unsigned int j;
>
> Same. Or better yet, reuse i.

Will do.

> >      params.entire_thread = 0;
> >      params.raw = 0;
> > @@ -1098,6 +1104,8 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
> >      params.part = atoi(argv[i] + sizeof ("--part=") - 1);
> >   } else if (STRNCMP_LITERAL (argv[i], "--entire-thread") == 0) {
> >      params.entire_thread = 1;
> > + } else if (STRNCMP_LITERAL (argv[i], "--no-exclude") == 0) {
> > +    no_exclude = TRUE;
> >   } else if ((STRNCMP_LITERAL (argv[i], "--verify") == 0) ||
> >     (STRNCMP_LITERAL (argv[i], "--decrypt") == 0)) {
> >      if (params.cryptoctx == NULL) {
> > @@ -1167,10 +1175,18 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
> >      if (params.raw && params.part < 0)
> >   params.part = 0;
> >  
> > +    /* If a single message is requested we do not use search_excludes. */
> >      if (params.part >= 0)
> >   return do_show_single (ctx, query, format, &params);
> > -    else
> > +    else {
>
> Nitpick: There's no rule about this, but I do like the style of using
> braces for both branches if either branch needs them.

Will fix.

> > + if (!no_exclude) {
> > +    search_exclude_tags = notmuch_config_get_search_exclude_tags
> > + (config, &search_exclude_tags_length);
> > +    for (j = 0; j < search_exclude_tags_length; j++)
> > + notmuch_query_add_tag_exclude (query, search_exclude_tags[j]);
> > + }
> >   return do_show (ctx, query, format, &params);
> > +    }
>
> Hmm, unreachable code below. Why doesn't the compiler complain?

Yes I wondered about that (id:"[hidden email]"): but didn't think I should do anything about
that in this series.

Thanks

Mark

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Jani Nikula]

On Thu, 02 Feb 2012 22:27:36 +0000, Mark Walters <[hidden email]> wrote:

> On Thu, 02 Feb 2012 23:55:33 +0200, Jani Nikula <[hidden email]> wrote:
> >
> > Hi Mark -
> >
> > This is my first look at any version of the series; apologies if I'm
> > clueless about some details... Please find some comments below.
> >
> > BR,
> > Jani.
> >
> >
> > On Thu,  2 Feb 2012 17:43:35 +0000, Mark Walters <[hidden email]> wrote:
> > > The function is
> > > notmuch_thread_get_flag_messages
> > > (notmuch_thread_t *thread, unsigned int flag_mask, unsigned int flags)
> > >
> > > and returns the number of messages with the specified flags on flag_mask.
> >
> > Is the purpose of this function to get the count of messages that have
> > certain flags set, certain flags not set, and certain flags don't-care?
>
> Yes: I was trying to follow Austin's suggestion from
> id:"[hidden email]" (although stupidly I didn't
> follow his suggestion of a function name).
>
> > At the very least, I think the documentation of the function should be
> > greatly improved.
> >
> > I think the name of the function should be notmuch_thread_count_messages
> > which is like notmuch_query_count_messages, but for messages in threads
> > (and with some extra restrictions).
>
> Yes I like your name; before I change it do you (and others) prefer it
> to Austin's suggestion of notmuch_thread_count_flags. Or we could even
> be more verbose with something like
> notmuch_thread_count_messages_with_flags

I'd like to make it clear that it's about message count. Not about
getting flags, not about flag counts. _with_flags is a matter of taste,
no strong opinions there.

>
> > >  /* Message flags */
> > >  typedef enum _notmuch_message_flag {
> > > -    NOTMUCH_MESSAGE_FLAG_MATCH,
> > > -    NOTMUCH_MESSAGE_FLAG_EXCLUDED
> > > +    NOTMUCH_MESSAGE_FLAG_MATCH = (1<<0),
> > > +    NOTMUCH_MESSAGE_FLAG_EXCLUDED = (1<<1),
> > > +    NOTMUCH_MESSAGE_FLAG_MAX  = (1<<2)
> >
> > How are these used by the current lib users at the moment? How will they
> > break with this change?
>
> The only existing flag is NOTMUCH_MESSAGE_FLAG_MATCH: that is currently
> zero but in the current code that is the bit offset of the flag; in my
> version it is the actual bit for the flag (otherwise I think flag masks
> end up very ugly). I believe all callers use notmuch_message_set_flag
> and notmuch_message_get_flag so they should not notice the difference.
>
> > Please align the assignments.
>
> Will do.
>
> > > @@ -457,8 +452,8 @@ _notmuch_thread_create (void *ctx,
> > >      thread->message_hash = g_hash_table_new_full (g_str_hash, g_str_equal,
> > >    free, NULL);
> > >  
> > > -    thread->total_messages = 0;
> > > -    thread->matched_messages = 0;
> > > +    for (i = 0; i < NOTMUCH_MESSAGE_FLAG_MAX; i++)
> > > + thread->flag_count_messages[i] = 0;
> >
> > memset (thread->flag_count_messages, 0, sizeof(thread->flag_count_messages));
>
>
> Will do
>
> > >      thread->oldest = 0;
> > >      thread->newest = 0;
> > >  
> > > @@ -473,6 +468,7 @@ _notmuch_thread_create (void *ctx,
> > >   notmuch_messages_move_to_next (messages))
> > >      {
> > >   unsigned int doc_id;
> > > + unsigned int message_flags;
> > >  
> > >   message = notmuch_messages_get (messages);
> > >   doc_id = _notmuch_message_get_doc_id (message);
> > > @@ -485,6 +481,10 @@ _notmuch_thread_create (void *ctx,
> > >      _notmuch_doc_id_set_remove (match_set, doc_id);
> > >      _thread_add_matched_message (thread, message, sort);
> > >   }
> > > + message_flags =
> > > +    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH) |
> > > +    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED);
> > > + thread->flag_count_messages[message_flags]++;
> >
> > The first impression of using a set of flags as index is that there's a
> > bug. But this is to keep count of messages with certain flag sets rather
> > than total for each flag, right? I think this needs more comments, more
> > documentation. Already naming the field flag_set_message_counts or
> > similar would help greatly.
>
> I will try and document it better: on first reading I parsed your name
> as flag set (as verb) message counts whereas I assume you mean "flag
> set" as a noun! I will see if I can come up with something though.

Yes, as a noun! :)

>
> > >   _notmuch_message_close (message);
> > >      }
> > > @@ -511,15 +511,28 @@ notmuch_thread_get_thread_id (notmuch_thread_t *thread)
> > >  }
> > >  
> > >  int
> > > +notmuch_thread_get_flag_messages (notmuch_thread_t *thread, unsigned int flag_mask, unsigned int flags)
> > > +{
> > > +    unsigned int i;
> > > +    int count = 0;
> > > +    for (i = 0; i < NOTMUCH_MESSAGE_FLAG_MAX; i++)
> >
> > ARRAY_SIZE (thread->flag_count_messages)
>
> ok
>
> >
> > > + if ((i & flag_mask) == (flags & flag_mask))
> > > +    count += thread->flag_count_messages[i];
> > > +    return count;
> > > +}
> >
> > I wonder if the same could be accomplished by using two flag mask
> > parameters, include_flag_mask and exclude_flag_mask. I'm thinking of the
> > usage, would it be easier to use:
> >
> > notmuch_query_count_messages (thread, NOTMUCH_MESSAGE_FLAG_MATCH, NOTMUCH_MESSAGE_FLAG_EXCLUDED);
> >
> > to get number of messages that have MATCH but not EXCLUDED? 0 as
> > include_flag_mask could still be special for "all", and you could use:
> >
> > notmuch_query_count_messages (thread, 0, NOTMUCH_MESSAGE_FLAG_EXCLUDED);
> >
> > Note the name change according to my earlier suggestion. It might be
> > wise to not export the function before the API is chrystal clear if
> > there is no pressing need to do so.
>
> (I assume you mean notmuch_thread_count_messages.)

Doh! Yes.

> Can I just check this
> would return the number of messages which have all the flags  in
> include_flag_mask and none of the flags in exclude_flag_mask?

Yes, but only if it makes sense to you! :)

>
> I completely agree about leaving it until we have the API well worked
> out. I wrote it in response to Austin's suggestion and then it looked
> like it would useful in my attempts to remove the
> notmuch_query_set_omit_excluded_messages API. However, those attempts
> failed so it doesn't have any users yet.
>
> Best wishes
>
> Mark

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Jani Nikula]

On Thu, 02 Feb 2012 22:35:10 +0000, Mark Walters <[hidden email]> wrote:

>
> On Fri, 03 Feb 2012 00:08:32 +0200, Jani Nikula <[hidden email]> wrote:
> > On Thu,  2 Feb 2012 17:43:36 +0000, Mark Walters <[hidden email]> wrote:
> > > This adds the excludes to notmuch-show.c. We do not exclude when only
> > > a single message (or part) is requested. notmuch-show will output the
> > > exclude information when either text or json format is requested. As
> > > this changes the output from notmuch-show it breaks many tests (in a
> > > trivial and expected fashion).
> > > ---
> > >  notmuch-show.c |   24 ++++++++++++++++++++----
> > >  1 files changed, 20 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/notmuch-show.c b/notmuch-show.c
> > > index dec799c..108f13b 100644
> > > --- a/notmuch-show.c
> > > +++ b/notmuch-show.c
> > > @@ -193,10 +193,11 @@ _get_one_line_summary (const void *ctx, notmuch_message_t *message)
> > >  static void
> > >  format_message_text (unused (const void *ctx), notmuch_message_t *message, int indent)
> > >  {
> > > -    printf ("id:%s depth:%d match:%d filename:%s\n",
> > > +    printf ("id:%s depth:%d match:%d excluded:%d filename:%s\n",
> > >      notmuch_message_get_message_id (message),
> > >      indent,
> > > -    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH),
> > > +    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH) ? 1 : 0,
> > > +    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED) ? 1 : 0,
> > >      notmuch_message_get_filename (message));
> > >  }
> > >  
> > > @@ -212,9 +213,10 @@ format_message_json (const void *ctx, notmuch_message_t *message, unused (int in
> > >      date = notmuch_message_get_date (message);
> > >      relative_date = notmuch_time_relative_date (ctx, date);
> > >  
> > > -    printf ("\"id\": %s, \"match\": %s, \"filename\": %s, \"timestamp\": %ld, \"date_relative\": \"%s\", \"tags\": [",
> > > +    printf ("\"id\": %s, \"match\": %s, \"excluded\": %s, \"filename\": %s, \"timestamp\": %ld, \"date_relative\": \"%s\", \"tags\": [",
> > >      json_quote_str (ctx_quote, notmuch_message_get_message_id (message)),
> > >      notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH) ? "true" : "false",
> > > +    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED) ? "true" : "false",
> > >      json_quote_str (ctx_quote, notmuch_message_get_filename (message)),
> > >      date, relative_date);
> > >  
> > > @@ -1059,9 +1061,13 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
> > >      char *opt;
> > >      const notmuch_show_format_t *format = &format_text;
> > >      notmuch_show_params_t params;
> > > +    const char **search_exclude_tags;
> > > +    size_t search_exclude_tags_length;
> >
> > Please move these within the if (!no_exclude) block.
>
> Will do. (I forgot to move them in notmuch-show when doing notmuch-count
> and notmuch-search)
>
> > >      int mbox = 0;
> > >      int format_specified = 0;
> > >      int i;
> > > +    notmuch_bool_t no_exclude = FALSE;
> > > +    unsigned int j;
> >
> > Same. Or better yet, reuse i.
>
> Will do.
>
> > >      params.entire_thread = 0;
> > >      params.raw = 0;
> > > @@ -1098,6 +1104,8 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
> > >      params.part = atoi(argv[i] + sizeof ("--part=") - 1);
> > >   } else if (STRNCMP_LITERAL (argv[i], "--entire-thread") == 0) {
> > >      params.entire_thread = 1;
> > > + } else if (STRNCMP_LITERAL (argv[i], "--no-exclude") == 0) {
> > > +    no_exclude = TRUE;
> > >   } else if ((STRNCMP_LITERAL (argv[i], "--verify") == 0) ||
> > >     (STRNCMP_LITERAL (argv[i], "--decrypt") == 0)) {
> > >      if (params.cryptoctx == NULL) {
> > > @@ -1167,10 +1175,18 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
> > >      if (params.raw && params.part < 0)
> > >   params.part = 0;
> > >  
> > > +    /* If a single message is requested we do not use search_excludes. */
> > >      if (params.part >= 0)
> > >   return do_show_single (ctx, query, format, &params);
> > > -    else
> > > +    else {
> >
> > Nitpick: There's no rule about this, but I do like the style of using
> > braces for both branches if either branch needs them.
>
> Will fix.
>
> > > + if (!no_exclude) {
> > > +    search_exclude_tags = notmuch_config_get_search_exclude_tags
> > > + (config, &search_exclude_tags_length);
> > > +    for (j = 0; j < search_exclude_tags_length; j++)
> > > + notmuch_query_add_tag_exclude (query, search_exclude_tags[j]);
> > > + }
> > >   return do_show (ctx, query, format, &params);
> > > +    }
> >
> > Hmm, unreachable code below. Why doesn't the compiler complain?
>
> Yes I wondered about that (id:"[hidden email]"): but didn't think I should do anything about
> that in this series.

I'll send patches to fix that along with converting notmuch-show to the
new style argument parsing in a day or two. I didn't like you adding
options to old style parsing, but also didn't think it reasonable to ask
you to fix it in this series. I might have to ask you to rebase if my
patches get in first, though. ;)

>
> Thanks
>
> Mark
>
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Mark Walters]

On Fri, 03 Feb 2012 01:13:31 +0200, Jani Nikula <[hidden email]> wrote:

> On Thu, 02 Feb 2012 22:35:10 +0000, Mark Walters <[hidden email]> wrote:
> >
> > On Fri, 03 Feb 2012 00:08:32 +0200, Jani Nikula <[hidden email]> wrote:
> > > On Thu,  2 Feb 2012 17:43:36 +0000, Mark Walters <[hidden email]> wrote:
> > > > This adds the excludes to notmuch-show.c. We do not exclude when only
> > > > a single message (or part) is requested. notmuch-show will output the
> > > > exclude information when either text or json format is requested. As
> > > > this changes the output from notmuch-show it breaks many tests (in a
> > > > trivial and expected fashion).
> > > > ---
> > > >  notmuch-show.c |   24 ++++++++++++++++++++----
> > > >  1 files changed, 20 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/notmuch-show.c b/notmuch-show.c
> > > > index dec799c..108f13b 100644
> > > > --- a/notmuch-show.c
> > > > +++ b/notmuch-show.c
> > > > @@ -193,10 +193,11 @@ _get_one_line_summary (const void *ctx, notmuch_message_t *message)
> > > >  static void
> > > >  format_message_text (unused (const void *ctx), notmuch_message_t *message, int indent)
> > > >  {
> > > > -    printf ("id:%s depth:%d match:%d filename:%s\n",
> > > > +    printf ("id:%s depth:%d match:%d excluded:%d filename:%s\n",
> > > >      notmuch_message_get_message_id (message),
> > > >      indent,
> > > > -    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH),
> > > > +    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH) ? 1 : 0,
> > > > +    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED) ? 1 : 0,
> > > >      notmuch_message_get_filename (message));
> > > >  }
> > > >  
> > > > @@ -212,9 +213,10 @@ format_message_json (const void *ctx, notmuch_message_t *message, unused (int in
> > > >      date = notmuch_message_get_date (message);
> > > >      relative_date = notmuch_time_relative_date (ctx, date);
> > > >  
> > > > -    printf ("\"id\": %s, \"match\": %s, \"filename\": %s, \"timestamp\": %ld, \"date_relative\": \"%s\", \"tags\": [",
> > > > +    printf ("\"id\": %s, \"match\": %s, \"excluded\": %s, \"filename\": %s, \"timestamp\": %ld, \"date_relative\": \"%s\", \"tags\": [",
> > > >      json_quote_str (ctx_quote, notmuch_message_get_message_id (message)),
> > > >      notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH) ? "true" : "false",
> > > > +    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED) ? "true" : "false",
> > > >      json_quote_str (ctx_quote, notmuch_message_get_filename (message)),
> > > >      date, relative_date);
> > > >  
> > > > @@ -1059,9 +1061,13 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
> > > >      char *opt;
> > > >      const notmuch_show_format_t *format = &format_text;
> > > >      notmuch_show_params_t params;
> > > > +    const char **search_exclude_tags;
> > > > +    size_t search_exclude_tags_length;
> > >
> > > Please move these within the if (!no_exclude) block.
> >
> > Will do. (I forgot to move them in notmuch-show when doing notmuch-count
> > and notmuch-search)
> >
> > > >      int mbox = 0;
> > > >      int format_specified = 0;
> > > >      int i;
> > > > +    notmuch_bool_t no_exclude = FALSE;
> > > > +    unsigned int j;
> > >
> > > Same. Or better yet, reuse i.
> >
> > Will do.
> >
> > > >      params.entire_thread = 0;
> > > >      params.raw = 0;
> > > > @@ -1098,6 +1104,8 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
> > > >      params.part = atoi(argv[i] + sizeof ("--part=") - 1);
> > > >   } else if (STRNCMP_LITERAL (argv[i], "--entire-thread") == 0) {
> > > >      params.entire_thread = 1;
> > > > + } else if (STRNCMP_LITERAL (argv[i], "--no-exclude") == 0) {
> > > > +    no_exclude = TRUE;
> > > >   } else if ((STRNCMP_LITERAL (argv[i], "--verify") == 0) ||
> > > >     (STRNCMP_LITERAL (argv[i], "--decrypt") == 0)) {
> > > >      if (params.cryptoctx == NULL) {
> > > > @@ -1167,10 +1175,18 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
> > > >      if (params.raw && params.part < 0)
> > > >   params.part = 0;
> > > >  
> > > > +    /* If a single message is requested we do not use search_excludes. */
> > > >      if (params.part >= 0)
> > > >   return do_show_single (ctx, query, format, &params);
> > > > -    else
> > > > +    else {
> > >
> > > Nitpick: There's no rule about this, but I do like the style of using
> > > braces for both branches if either branch needs them.
> >
> > Will fix.
> >
> > > > + if (!no_exclude) {
> > > > +    search_exclude_tags = notmuch_config_get_search_exclude_tags
> > > > + (config, &search_exclude_tags_length);
> > > > +    for (j = 0; j < search_exclude_tags_length; j++)
> > > > + notmuch_query_add_tag_exclude (query, search_exclude_tags[j]);
> > > > + }
> > > >   return do_show (ctx, query, format, &params);
> > > > +    }
> > >
> > > Hmm, unreachable code below. Why doesn't the compiler complain?
> >
> > Yes I wondered about that (id:"[hidden email]"): but didn't think I should do anything about
> > that in this series.
>
> I'll send patches to fix that along with converting notmuch-show to the
> new style argument parsing in a day or two. I didn't like you adding
> options to old style parsing, but also didn't think it reasonable to ask
> you to fix it in this series. I might have to ask you to rebase if my
> patches get in first, though. ;)

That's great: I looked at the impressive spaghetti argument parsing and
couldn't face trying to convert it. I am very happy to rebase on top of
your patch!

Best wishes

Mark

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Mark Walters]

On Fri, 03 Feb 2012 01:07:59 +0200, Jani Nikula <[hidden email]> wrote:

> On Thu, 02 Feb 2012 22:27:36 +0000, Mark Walters <[hidden email]> wrote:
> > On Thu, 02 Feb 2012 23:55:33 +0200, Jani Nikula <[hidden email]> wrote:
> > >
> > > Hi Mark -
> > >
> > > This is my first look at any version of the series; apologies if I'm
> > > clueless about some details... Please find some comments below.
> > >
> > > BR,
> > > Jani.
> > >
> > >
> > > On Thu,  2 Feb 2012 17:43:35 +0000, Mark Walters <[hidden email]> wrote:
> > > > The function is
> > > > notmuch_thread_get_flag_messages
> > > > (notmuch_thread_t *thread, unsigned int flag_mask, unsigned int flags)
> > > >
> > > > and returns the number of messages with the specified flags on flag_mask.
> > >
> > > Is the purpose of this function to get the count of messages that have
> > > certain flags set, certain flags not set, and certain flags don't-care?
> >
> > Yes: I was trying to follow Austin's suggestion from
> > id:"[hidden email]" (although stupidly I didn't
> > follow his suggestion of a function name).
> >
> > > At the very least, I think the documentation of the function should be
> > > greatly improved.
> > >
> > > I think the name of the function should be notmuch_thread_count_messages
> > > which is like notmuch_query_count_messages, but for messages in threads
> > > (and with some extra restrictions).
> >
> > Yes I like your name; before I change it do you (and others) prefer it
> > to Austin's suggestion of notmuch_thread_count_flags. Or we could even
> > be more verbose with something like
> > notmuch_thread_count_messages_with_flags
>
> I'd like to make it clear that it's about message count. Not about
> getting flags, not about flag counts. _with_flags is a matter of taste,
> no strong opinions there.

I think I will go with notmuch_thread_count_messages as you suggest.

> > > >  /* Message flags */
> > > >  typedef enum _notmuch_message_flag {
> > > > -    NOTMUCH_MESSAGE_FLAG_MATCH,
> > > > -    NOTMUCH_MESSAGE_FLAG_EXCLUDED
> > > > +    NOTMUCH_MESSAGE_FLAG_MATCH = (1<<0),
> > > > +    NOTMUCH_MESSAGE_FLAG_EXCLUDED = (1<<1),
> > > > +    NOTMUCH_MESSAGE_FLAG_MAX  = (1<<2)
> > >
> > > How are these used by the current lib users at the moment? How will they
> > > break with this change?

I will just comment on this: the *only* reason I put in
NOTMUCH_MESSAGE_FLAG_MAX was as a way of keeping track of the size of
the bitfield. If there is a better way do say!

> > The only existing flag is NOTMUCH_MESSAGE_FLAG_MATCH: that is currently
> > zero but in the current code that is the bit offset of the flag; in my
> > version it is the actual bit for the flag (otherwise I think flag masks
> > end up very ugly). I believe all callers use notmuch_message_set_flag
> > and notmuch_message_get_flag so they should not notice the difference.
> >
> > > Please align the assignments.
> >
> > Will do.
> >
> > > > @@ -457,8 +452,8 @@ _notmuch_thread_create (void *ctx,
> > > >      thread->message_hash = g_hash_table_new_full (g_str_hash, g_str_equal,
> > > >    free, NULL);
> > > >  
> > > > -    thread->total_messages = 0;
> > > > -    thread->matched_messages = 0;
> > > > +    for (i = 0; i < NOTMUCH_MESSAGE_FLAG_MAX; i++)
> > > > + thread->flag_count_messages[i] = 0;
> > >
> > > memset (thread->flag_count_messages, 0, sizeof(thread->flag_count_messages));
> >
> >
> > Will do
> >
> > > >      thread->oldest = 0;
> > > >      thread->newest = 0;
> > > >  
> > > > @@ -473,6 +468,7 @@ _notmuch_thread_create (void *ctx,
> > > >   notmuch_messages_move_to_next (messages))
> > > >      {
> > > >   unsigned int doc_id;
> > > > + unsigned int message_flags;
> > > >  
> > > >   message = notmuch_messages_get (messages);
> > > >   doc_id = _notmuch_message_get_doc_id (message);
> > > > @@ -485,6 +481,10 @@ _notmuch_thread_create (void *ctx,
> > > >      _notmuch_doc_id_set_remove (match_set, doc_id);
> > > >      _thread_add_matched_message (thread, message, sort);
> > > >   }
> > > > + message_flags =
> > > > +    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH) |
> > > > +    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED);
> > > > + thread->flag_count_messages[message_flags]++;
> > >
> > > The first impression of using a set of flags as index is that there's a
> > > bug. But this is to keep count of messages with certain flag sets rather
> > > than total for each flag, right? I think this needs more comments, more
> > > documentation. Already naming the field flag_set_message_counts or
> > > similar would help greatly.
> >
> > I will try and document it better: on first reading I parsed your name
> > as flag set (as verb) message counts whereas I assume you mean "flag
> > set" as a noun! I will see if I can come up with something though.
>
> Yes, as a noun! :)

I haven't come up with a good name: the best I have come up with is
flagset_message_count so if you have any suggestions...

> > > >   _notmuch_message_close (message);
> > > >      }
> > > > @@ -511,15 +511,28 @@ notmuch_thread_get_thread_id (notmuch_thread_t *thread)
> > > >  }
> > > >  
> > > >  int
> > > > +notmuch_thread_get_flag_messages (notmuch_thread_t *thread, unsigned int flag_mask, unsigned int flags)
> > > > +{
> > > > +    unsigned int i;
> > > > +    int count = 0;
> > > > +    for (i = 0; i < NOTMUCH_MESSAGE_FLAG_MAX; i++)
> > >
> > > ARRAY_SIZE (thread->flag_count_messages)
> >
> > ok
> >
> > >
> > > > + if ((i & flag_mask) == (flags & flag_mask))
> > > > +    count += thread->flag_count_messages[i];
> > > > +    return count;
> > > > +}
> > >
> > > I wonder if the same could be accomplished by using two flag mask
> > > parameters, include_flag_mask and exclude_flag_mask. I'm thinking of the
> > > usage, would it be easier to use:
> > >
> > > notmuch_query_count_messages (thread, NOTMUCH_MESSAGE_FLAG_MATCH, NOTMUCH_MESSAGE_FLAG_EXCLUDED);
> > >
> > > to get number of messages that have MATCH but not EXCLUDED? 0 as
> > > include_flag_mask could still be special for "all", and you could use:
> > >
> > > notmuch_query_count_messages (thread, 0, NOTMUCH_MESSAGE_FLAG_EXCLUDED);
> > >
> > > Note the name change according to my earlier suggestion. It might be
> > > wise to not export the function before the API is chrystal clear if
> > > there is no pressing need to do so.
> >
> > (I assume you mean notmuch_thread_count_messages.)
>
> Doh! Yes.
>
> > Can I just check this
> > would return the number of messages which have all the flags  in
> > include_flag_mask and none of the flags in exclude_flag_mask?

Yes I think this works better: these are the flags I want, these are the
ones I don't want seems natural (versus here are the ones I care about
and here are the ones of those I want). But I will wait to see if anyone
else has an opinion.

> Yes, but only if it makes sense to you! :)
>
> >
> > I completely agree about leaving it until we have the API well worked
> > out. I wrote it in response to Austin's suggestion and then it looked
> > like it would useful in my attempts to remove the
> > notmuch_query_set_omit_excluded_messages API. However, those attempts
> > failed so it doesn't have any users yet.
> >
> > Best wishes
> >
> > Mark

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch