notmuch show should tell us what messages matched the search expression

notmuch show should tell us what messages matched the search expression

[Bart Trojanowski]

The following 3 patches implement this feature.

Internally the message object learns about flags.  Only a single flag is
defined to denote that a message matched the search expression.

That flag is then rendered on the "message{" line in the output of
notmuch show like this:

message{ id:... depth:4 match:0 filename:...
message{ id:... depth:4 match:1 filename:...

This can now be used by UI interfaces to hide or collapse less
interesting messages.

-Bart

[PATCH 1/3] message: add flags to notmuch_message_t

[Bart Trojanowski]

This patch allows for different flags, internal to notmuch, to be set on a
message object.  The patch does not define any such flags, just the
facilities to manage these flags.

Signed-off-by: Bart Trojanowski <bart@jukie.net>
---
 lib/message.cc |   19 +++++++++++++++++++
 lib/notmuch.h  |   14 ++++++++++++++
 2 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/lib/message.cc b/lib/message.cc
index 1e325e2..e0834f1 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -37,6 +37,7 @@ struct _notmuch_message {
     char *filename;
     notmuch_message_file_t *message_file;
     notmuch_message_list_t *replies;
+    unsigned long flags;
 
     Xapian::Document doc;
 };
@@ -108,6 +109,7 @@ _notmuch_message_create (const void *talloc_owner,
     message->doc_id = doc_id;
 
     message->frozen = 0;
+    message->flags = 0;
 
     /* Each of these will be lazily created as needed. */
     message->message_id = NULL;
@@ -445,6 +447,23 @@ notmuch_message_get_filename (notmuch_message_t *message)
     return message->filename;
 }
 
+notmuch_bool_t
+notmuch_message_get_flag (notmuch_message_t *message,
+			  notmuch_message_flag_t flag)
+{
+    return message->flags & (1 << flag);
+}
+
+void
+notmuch_message_set_flag (notmuch_message_t *message,
+			  notmuch_message_flag_t flag, notmuch_bool_t enable)
+{
+    if (enable)
+	message->flags |= (1 << flag);
+    else
+	message->flags &= ~(1 << flag);
+}
+
 time_t
 notmuch_message_get_date (notmuch_message_t *message)
 {
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 8bba442..c232c58 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -684,6 +684,20 @@ notmuch_message_get_replies (notmuch_message_t *message);
 const char *
 notmuch_message_get_filename (notmuch_message_t *message);
 
+/* Message flags */
+typedef enum _notmuch_message_flag {
+} notmuch_message_flag_t;
+
+/* Get a value of a flag for the email corresponding to 'message'. */
+notmuch_bool_t
+notmuch_message_get_flag (notmuch_message_t *message,
+			  notmuch_message_flag_t flag);
+
+/* Set a value of a flag for the email corresponding to 'message'. */
+void
+notmuch_message_set_flag (notmuch_message_t *message,
+			  notmuch_message_flag_t flag, notmuch_bool_t value);
+
 /* Get the date of 'message' as a time_t value.
  *
  * For the original textual representation of the Date header from the
-- 
1.6.4.4.2.gc2f148

[PATCH 2/3] have _notmuch_thread_create mark which messages matched the query

[Bart Trojanowski]

When _notmuch_thread_create() is given a query string, it can return more
messages than just those matching the query.  To distinguish those that
matched the query expression, the MATCHING_SEARCH flag is set
appropriately.

Signed-off-by: Bart Trojanowski <bart@jukie.net>
---
 lib/notmuch.h |    1 +
 lib/thread.cc |    8 ++++++++
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/lib/notmuch.h b/lib/notmuch.h
index c232c58..3974820 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -686,6 +686,7 @@ notmuch_message_get_filename (notmuch_message_t *message);
 
 /* Message flags */
 typedef enum _notmuch_message_flag {
+    NOTMUCH_MSG_FLAG_MATCHING_SEARCH,
 } 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 58d88c2..9e4cb5c 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -132,6 +132,7 @@ _thread_add_matched_message (notmuch_thread_t *thread,
 			     notmuch_message_t *message)
 {
     time_t date;
+    notmuch_message_t *hashed_message;
 
     date = notmuch_message_get_date (message);
 
@@ -142,6 +143,13 @@ _thread_add_matched_message (notmuch_thread_t *thread,
 	thread->newest = date;
 
     thread->matched_messages++;
+
+    if (g_hash_table_lookup_extended (thread->message_hash,
+			    notmuch_message_get_message_id (message), NULL,
+			    (void **) &hashed_message)) {
+	notmuch_message_set_flag (hashed_message,
+			NOTMUCH_MSG_FLAG_MATCHING_SEARCH, 1);
+    }
 }
 
 static void
-- 
1.6.4.4.2.gc2f148

[PATCH 3/3] notmuch-show: identify which messages printed matched the query string

[Bart Trojanowski]

The show command outputs all messages in the threads that match the
search-terms.  This patch introduces a 'match:[01]' entry to the 'message{'
line output by the show command.  Value of 1 indicates that the message is
matching the search expression.

Signed-off-by: Bart Trojanowski <bart@jukie.net>
---
 notmuch-show.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index 8599c6c..e9f0883 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -155,9 +155,10 @@ show_message (void *ctx, notmuch_message_t *message, int indent)
     const char *name, *value;
     unsigned int i;
 
-    printf ("\fmessage{ id:%s depth:%d filename:%s\n",
+    printf ("\fmessage{ id:%s depth:%d match:%d filename:%s\n",
 	    notmuch_message_get_message_id (message),
 	    indent,
+	    notmuch_message_get_flag (message, NOTMUCH_MSG_FLAG_MATCHING_SEARCH),
 	    notmuch_message_get_filename (message));
 
     printf ("\fheader{\n");
-- 
1.6.4.4.2.gc2f148

Re: [PATCH 2/3] have _notmuch_thread_create mark which messages matched the query

[Carl Worth]

On Tue, 24 Nov 2009 23:54:34 -0500, Bart Trojanowski <bart@jukie.net> wrote:
> When _notmuch_thread_create() is given a query string, it can return more
> messages than just those matching the query.  To distinguish those that
> matched the query expression, the MATCHING_SEARCH flag is set
> appropriately.

This is a very useful feature, Bart. Thanks for coding it up. And it's a
nicely-implemented patch series as well.

I've pushed this out now, so anyone reading along should be able to try
it with:

./notmuch show thread:6d5e3e276461188c5778c9f219f63782 and subject:"PATCH 2/3"

I can't wait to have the emacs support for opening only matched messages
by default.

By the way, do you think that this support obviates the
--only-matching-messages option for "notmuch search" or does anyone
still want that?

Or maybe the right fix is to make "notmuch show" display only matching
messages by default, (which will likely be more friendly to a user
manually typing "notmuch show" at the command line). And then make the
user-interfaces pass an "--entire-thread" option (or so) to get the
current results.

If we're going to make the command-line user-interface usable on its
own, then I definitely want to make it be the user interfaces that have
to pass extra-long command-line options to get what they want.

One quick point on naming:

>  /* Message flags */
>  typedef enum _notmuch_message_flag {
> +    NOTMUCH_MSG_FLAG_MATCHING_SEARCH,
>  } notmuch_message_flag_t;

I like my enum values to match their type name without abbreviation. I
also like internals (like this enum value) to match the way they are
exposed in the interface, (which in this case is "match"). So I'd like
the above value to instead be:

	NOTMUCH_MESSAGE_FLAG_MATCH

-Carl

[PATCH] correct message flag enum value so that it matches the type

[Bart Trojanowski]

As per Carl's request, this patch corrects the only value defined under
the notmuch_message_flag_t enum typedef to match the name of the type.

Signed-off-by: Bart Trojanowski <bart@jukie.net>
---
 lib/notmuch.h  |    2 +-
 lib/thread.cc  |    2 +-
 notmuch-show.c |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/notmuch.h b/lib/notmuch.h
index e4f3992..60834fb 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -711,7 +711,7 @@ notmuch_message_get_filename (notmuch_message_t *message);
 
 /* Message flags */
 typedef enum _notmuch_message_flag {
-    NOTMUCH_MSG_FLAG_MATCHING_SEARCH,
+    NOTMUCH_MESSAGE_FLAG_MATCH,
 } 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 9e4cb5c..321937b 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -148,7 +148,7 @@ _thread_add_matched_message (notmuch_thread_t *thread,
 			    notmuch_message_get_message_id (message), NULL,
 			    (void **) &hashed_message)) {
 	notmuch_message_set_flag (hashed_message,
-			NOTMUCH_MSG_FLAG_MATCHING_SEARCH, 1);
+				  NOTMUCH_MESSAGE_FLAG_MATCH, 1);
     }
 }
 
diff --git a/notmuch-show.c b/notmuch-show.c
index f189e94..13c91e4 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -158,7 +158,7 @@ show_message (void *ctx, notmuch_message_t *message, int indent)
     printf ("\fmessage{ id:%s depth:%d match:%d filename:%s\n",
 	    notmuch_message_get_message_id (message),
 	    indent,
-	    notmuch_message_get_flag (message, NOTMUCH_MSG_FLAG_MATCHING_SEARCH),
+	    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH),
 	    notmuch_message_get_filename (message));
 
     printf ("\fheader{\n");
-- 
1.6.4.4.2.gc2f148

Re: [PATCH 2/3] have _notmuch_thread_create mark which messages matched the query

[Bart Trojanowski]

On Fri, 27 Nov 2009 17:15:35 -0800, Carl Worth <cworth@cworth.org> wrote:
> This is a very useful feature, Bart. Thanks for coding it up. And it's a
> nicely-implemented patch series as well.

Thanks.  I've found it very handy.

> By the way, do you think that this support obviates the
> --only-matching-messages option for "notmuch search" or does anyone
> still want that?

I personally don't have much use for it (in the notmuch.vim UI).  However, I
can see it being useful if someone reads the messages on the terminal, maybe
using some less verbose output format.

> Or maybe the right fix is to make "notmuch show" display only matching
> messages by default, (which will likely be more friendly to a user
> manually typing "notmuch show" at the command line). And then make the
> user-interfaces pass an "--entire-thread" option (or so) to get the
> current results.

Sure, I'll fix up the patch and submit it in a bit.

> If we're going to make the command-line user-interface usable on its
> own, then I definitely want to make it be the user interfaces that have
> to pass extra-long command-line options to get what they want.

Agreed.  I still think we could use some templating for the different output
modes.  Maybe it will be important enough for soemone to implement :)

> One quick point on naming:
> 
> >  /* Message flags */
> >  typedef enum _notmuch_message_flag {
> > +    NOTMUCH_MSG_FLAG_MATCHING_SEARCH,
> >  } notmuch_message_flag_t;
> 
> I like my enum values to match their type name without abbreviation. I
> also like internals (like this enum value) to match the way they are
> exposed in the interface, (which in this case is "match"). So I'd like
> the above value to instead be:
> 
> 	NOTMUCH_MESSAGE_FLAG_MATCH

OK.  Patch is in flight.

Cheers,
-Bart

-- 
email sent from notmuch.vim plugin

[PATCH 1/2] notmuch-show: limit display to only matching messages

[Bart Trojanowski]

This patch changes the default behaviour of notmuch show to display only
messages that match the search expression.  However, --entire-thread
option is provided to display all messages in threads that matched the
search expression.

It is deemed that will be more useful for human users on the command line.
Scripts can be modified to include the --entire-thread option so that they
can display all messages once more.

Example:

$ notmuch search subject:git AND thread:23d99d0f364f93e90e15df8b42eddb5b
thread:23d99d0f364f93e90e15df8b42eddb5b      July 31 [4/12] Johan Herland; [RFCv2 00/12] Foreign VCS helper program for CVS repositories (inbox unread)

Note that in this thread 4 out of 12 messages matched.  The default show
behaviour is to show only those messages that match:

$ notmuch show subject:git AND thread:23d99d0f364f93e90e15df8b42eddb5b | grep 'message{' | wc -l
4

With the --only-matching-messages option the output will contain all dozen
messages:

$ notmuch show --entire-thread subject:git AND thread:23d99d0f364f93e90e15df8b42eddb5b | grep 'message{' | wc -l
12

Signed-off-by: Bart Trojanowski <bart@jukie.net>
---
 notmuch-show.c |   48 +++++++++++++++++++++++++++++++++++++-----------
 notmuch.c      |    8 ++++++++
 2 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index 13c91e4..60339d0 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -212,6 +212,24 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
     notmuch_thread_t *thread;
     notmuch_messages_t *messages;
     char *query_string;
+    int entire_thread = 0;
+    int i;
+
+    for (i = 0; i < argc && argv[i][0] == '-'; i++) {
+	if (strcmp (argv[i], "--") == 0) {
+	    i++;
+	    break;
+	}
+        if (strcmp(argv[i], "--entire-thread") == 0) {
+	    entire_thread = 1;
+	} else {
+	    fprintf (stderr, "Unrecognized option: %s\n", argv[i]);
+	    return 1;
+	}
+    }
+
+    argc -= i;
+    argv += i;
 
     config = notmuch_config_open (ctx, NULL, NULL);
     if (config == NULL)
@@ -239,21 +257,29 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 	return 1;
     }
 
-    for (threads = notmuch_query_search_threads (query);
-	 notmuch_threads_has_more (threads);
-	 notmuch_threads_advance (threads))
-    {
-	thread = notmuch_threads_get (threads);
+    if (!entire_thread) {
+	messages = notmuch_query_search_messages (query);
+	if (messages == NULL)
+	    INTERNAL_ERROR ("No messages.\n");
+	show_messages (ctx, messages, 0);
 
-	messages = notmuch_thread_get_toplevel_messages (thread);
+    } else {
+	for (threads = notmuch_query_search_threads (query);
+		notmuch_threads_has_more (threads);
+		notmuch_threads_advance (threads))
+	{
+	    thread = notmuch_threads_get (threads);
 
-	if (messages == NULL)
-	    INTERNAL_ERROR ("Thread %s has no toplevel messages.\n",
-			    notmuch_thread_get_thread_id (thread));
+	    messages = notmuch_thread_get_toplevel_messages (thread);
 
-	show_messages (ctx, messages, 0);
+	    if (messages == NULL)
+		INTERNAL_ERROR ("Thread %s has no toplevel messages.\n",
+			notmuch_thread_get_thread_id (thread));
+
+	    show_messages (ctx, messages, 0);
 
-	notmuch_thread_destroy (thread);
+	    notmuch_thread_destroy (thread);
+	}
     }
 
     notmuch_query_destroy (query);
diff --git a/notmuch.c b/notmuch.c
index 5b0284c..ea67b4f 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -177,6 +177,14 @@ command_t commands[] = {
       "\t\t(all replies to a particular message appear immediately\n"
       "\t\tafter that message in date order).\n"
       "\n"
+      "\t\tSupported options for show include:\n"
+      "\n"
+      "\t\t--entire-thread\n"
+      "\n"
+      "\t\t\tBy default only messages strictly matching the\n"
+      "\t\t\tsearch expression will be displayed.  With this\n"
+      "\t\t\toption all messages in matching threads are shown.\n"
+      "\n"
       "\t\tThe output format is plain-text, with all text-content\n"
       "\t\tMIME parts decoded. Various components in the output,\n"
       "\t\t('message', 'header', 'body', 'attachment', and MIME 'part')\n"
-- 
1.6.4.4.2.gc2f148

[PATCH 2/2] vim: use notmuch show --entire-thread

[Bart Trojanowski]

---
 vim/plugin/notmuch.vim |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/vim/plugin/notmuch.vim b/vim/plugin/notmuch.vim
index 34d70d9..a226f20 100644
--- a/vim/plugin/notmuch.vim
+++ b/vim/plugin/notmuch.vim
@@ -409,7 +409,7 @@ endfunction
 
 function! s:NM_cmd_show(words)
         let prev_bufnr = bufnr('%')
-        let data = s:NM_run(['show'] + a:words)
+        let data = s:NM_run(['show', '--entire-thread'] + a:words)
         let lines = split(data, "\n")
 
         let info = s:NM_cmd_show_parse(lines)
-- 
1.6.4.4.2.gc2f148

Re: [PATCH 1/2] notmuch-show: limit display to only matching messages

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 5105 bytes --]

On Fri, 27 Nov 2009 21:49:39 -0500, Bart Trojanowski <bart@jukie.net> wrote:
> This patch changes the default behaviour of notmuch show to display only
> messages that match the search expression.  However, --entire-thread
> option is provided to display all messages in threads that matched the
> search expression.
> 
> It is deemed that will be more useful for human users on the command line.
> Scripts can be modified to include the --entire-thread option so that they
> can display all messages once more.

Thanks Bart.

I've now applied this as well as your following patch to make vim call
"notmuch show" with the --entire-thread option. I also added support to
the emacs client to do the same, and I updated the notmuch man page to
document the option.

Finally, I noticed that not passing the --entire-thread option caused
"notmuch show" to lose all nesting and thread-ordering characteristics,
(even if display the entire thread incidentally). This came from calling
notmuch_query_search_messages (which doesn't do any threading) rather
than notmuch_query_search_threads. So I committed the patch below which
causes it to once again call search_threads and simply not display any
messages that don't match the search when called without
--entire-thread.

I'll be glad if you have any comments to share.

-Carl

commit e1cb6126cc7f66cf71df8880c798f4d0f9d2e411
Author: Carl Worth <cworth@cworth.org>
Date:   Wed Dec 2 16:05:23 2009 -0800

    notmuch show: Preserve thread-ordering and nesting without --entire-thread
    
    When "notmuch show" was recently modified to not show an entire thread
    by default, it also lost all capability to properly order the messages
    in a thread and to print their proper depth. For example, the command:
    
    	notmuch show thread:6d5e3e276461188c5778c9f219f63782
    
    had dramatically different output than:
    
    	notmuch show --entire-thread thread:6d5e3e276461188c5778c9f219f63782
    
    even though both commands were selecting and displaying the same set
    of messages. The first command would diplay them "flat", (all with
    depth:0), and in strict date order; while the second command would
    display them "nested" (with depth based on threading), and in thread
    order.
    
    We now fix "notmuch show" without the --entire-thread option to also
    display nested and thread-ordered messages.
    
    If some messages in the thread are not included in the displayed
    results, then they are not counted when computing depth values.

diff --git a/notmuch-show.c b/notmuch-show.c
index 60339d0..376aacd 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -184,9 +184,12 @@ show_message (void *ctx, notmuch_message_t *message, int indent)
 
 
 static void
-show_messages (void *ctx, notmuch_messages_t *messages, int indent)
+show_messages (void *ctx, notmuch_messages_t *messages, int indent,
+	       notmuch_bool_t entire_thread)
 {
     notmuch_message_t *message;
+    notmuch_bool_t match;
+    int next_indent;
 
     for (;
 	 notmuch_messages_has_more (messages);
@@ -194,9 +197,17 @@ show_messages (void *ctx, notmuch_messages_t *messages, int indent)
     {
 	message = notmuch_messages_get (messages);
 
-	show_message (ctx, message, indent);
+	match = notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH);
 
-	show_messages (ctx, notmuch_message_get_replies (message), indent + 1);
+	next_indent = indent;
+
+	if (match || entire_thread) {
+	    show_message (ctx, message, indent);
+	    next_indent = indent + 1;
+	}
+
+	show_messages (ctx, notmuch_message_get_replies (message),
+		       next_indent, entire_thread);
 
 	notmuch_message_destroy (message);
     }
@@ -257,29 +268,21 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 	return 1;
     }
 
-    if (!entire_thread) {
-	messages = notmuch_query_search_messages (query);
-	if (messages == NULL)
-	    INTERNAL_ERROR ("No messages.\n");
-	show_messages (ctx, messages, 0);
-
-    } else {
-	for (threads = notmuch_query_search_threads (query);
-		notmuch_threads_has_more (threads);
-		notmuch_threads_advance (threads))
-	{
-	    thread = notmuch_threads_get (threads);
+    for (threads = notmuch_query_search_threads (query);
+	 notmuch_threads_has_more (threads);
+	 notmuch_threads_advance (threads))
+    {
+	thread = notmuch_threads_get (threads);
 
-	    messages = notmuch_thread_get_toplevel_messages (thread);
+	messages = notmuch_thread_get_toplevel_messages (thread);
 
-	    if (messages == NULL)
-		INTERNAL_ERROR ("Thread %s has no toplevel messages.\n",
-			notmuch_thread_get_thread_id (thread));
+	if (messages == NULL)
+	    INTERNAL_ERROR ("Thread %s has no toplevel messages.\n",
+			    notmuch_thread_get_thread_id (thread));
 
-	    show_messages (ctx, messages, 0);
+	show_messages (ctx, messages, 0, entire_thread);
 
-	    notmuch_thread_destroy (thread);
-	}
+	notmuch_thread_destroy (thread);
     }
 
     notmuch_query_destroy (query);

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]