Date: Sun, 3 Jul 2011 13:17:43 -0400
From: Austin Clements <amdragon@MIT.EDU>
To: Pieter Praet <pieter@praet.org>
Subject: Re: [PATCH 2/2] [RFC] possible solution for "Race condition for
	'*' command"
Message-ID: <20110703171743.GL15901@mit.edu>
References:
 <CAH-f9WticM4EN8F1_ik_-mcBcBtrXwSpO+Drbtp7=UN7McECrg@mail.gmail.com>
	<87zkkwydag.fsf@praet.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87zkkwydag.fsf@praet.org>
User-Agent: Mutt/1.5.20 (2009-06-14)
Cc: notmuch@notmuchmail.org
Precedence: list

Quoth Pieter Praet on Jul 02 at  4:20 pm:
> On Fri, 1 Jul 2011 12:37:11 -0400, Austin Clements <amdragon@mit.edu> wrote:
> Non-text part: multipart/alternative
> > On Jul 1, 2011 10:55 AM, "Austin Clements" <amdragon@mit.edu> wrote:
> > >
> > > On Thu, Jun 30, 2011 at 3:38 PM, Pieter Praet <pieter@praet.org> wrote:
> > > > Ok, even though my very first reply [1] may have created the impression
> > > > that I understood the issue, I clearly didn't...
> > > >
> > > > The test [2] needs a more applicable commit message, and the subsequent
> > > > patch [3] points more or less in the right direction, but the Message-Id
> > > > list should be local to the *search buffer* rather than to the
> > > > `notmuch-search-operate-all' function.
> > > >
> > > > `notmuch-search' could:
> > > >  - run "notmuch-command search" with the "--output=messages" option
> > > >    instead of a plain search,
> > > >  - maintain a buffer-local var with a list of returned Message-Id's,
> > > >  - ...and populate the buffer based on that list.
> > > >
> > > > As such we'd have -for each individual search buffer- a canonical list
> > > > of Message-Id's (i.e. messages which actually *match* the query AND are
> > > > currently visible in the search buffer), to be used by
> > > > `notmuch-search-operate-all' et al.
> > > >
> > > >
> > > > Peace
> > > >
> > > >
> > > > [1] id:"87fwmuxxgd.fsf@praet.org"
> > > > [2] id:"1309450108-2793-2-git-send-email-pieter@praet.org"
> > > > [3] id:"1309450108-2793-1-git-send-email-pieter@praet.org"
> > >
> > > Ideally, this wouldn't be per-buffer, but per *line*.  This race
> > > equally affects adding and removing tags from individual results,
> > > since that's done using a thread: query, whose results could have
> > > changed since the original search.
> > >
> > > This almost certainly requires support from the notmuch core.  The
> > > good news is that the library already provides this information, so
> > > there will be virtually no performance hit for outputting it.
> > 
> > Actually, with a smidgeon of library support, you could even use document
> > IDs for this, rather than message IDs, which would make the tagging
> > operations (even '*') no more expensive than they are now.  (Of course, it
> > would be good to know just how much overhead going through message IDs
> > actually introduces.)
> Non-text part: text/html
> 
> 
> That would be awesome!
> 
> Though I'd rather leave the plumbing to someone sufficiently capable,
> so, if leaving libnotmuch hacking out of the equation, how would one go
> about doing this?
> 
> I (ignorantly) assume we'd get `notmuch-search-process-filter'
> to append each line with an invisible field containing a list
> of matching Message-Id's, presumably obtained via
> `notmuch_thread_get_matched_messages' (@ lib/thread.cc) ?

Here's a super-hacky patch that adds an "id:x or id:y or ..." query
string to the end of each search line, right after the list of tags.
This is just the C side; care to prototype the elisp side?

I also added a --stdin argument to notmuch tag that'll read the query
string from stdin, since these queries could get very long (though
ARG_MAX appears to be 2 megs these days, so maybe this wasn't
necessary for prototyping).

diff --git a/notmuch-search.c b/notmuch-search.c
index faccaf7..65fe438 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -190,6 +190,23 @@ format_thread_json (const void *ctx,
     talloc_free (ctx_quote);
 }
 
+static void
+show_message_ids (notmuch_messages_t *messages, notmuch_bool_t first)
+{
+    notmuch_message_t *message;
+
+    for (;
+	 notmuch_messages_valid (messages);
+	 notmuch_messages_move_to_next (messages)) {
+	if (!first)
+	    fputs (" or ", stdout);
+	first = FALSE;
+	message = notmuch_messages_get (messages);
+	printf ("id:\"%s\"", notmuch_message_get_message_id (message));
+	show_message_ids (notmuch_message_get_replies (message), FALSE);
+    }
+}
+
 static int
 do_search_threads (const search_format_t *format,
 		   notmuch_query_t *query,
@@ -252,6 +269,12 @@ do_search_threads (const search_format_t *format,
 
 	    fputs (format->tag_end, stdout);
 
+	    if (format == &format_text) {
+		notmuch_messages_t *toplevel = notmuch_thread_get_toplevel_messages (thread);
+		fputs (" ", stdout);
+		show_message_ids (toplevel, TRUE);
+	    }
+
 	    fputs (format->item_end, stdout);
 	}
 
diff --git a/notmuch-tag.c b/notmuch-tag.c
index 6204ae3..5609d02 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -30,6 +30,36 @@ handle_sigint (unused (int sig))
     interrupted = 1;
 }
 
+static char*
+query_string_from_stdin (void *ctx)
+{
+    char *query_string = talloc_strdup (ctx, "");
+    char buf[4096];
+
+    if (query_string == NULL) {
+	fprintf (stderr, "Out of memory.\n");
+	return NULL;
+    }
+
+    while (1) {
+	ssize_t r = read(0, buf, sizeof (buf));
+	if (r < 0) {
+	    fprintf (stderr, "Error reading from stdin: %s\n",
+		     strerror (errno));
+	    return NULL;
+	} else if (r == 0) {
+	    break;
+	}
+	query_string = talloc_strndup_append (query_string, buf, r);
+	if (!query_string) {
+	    fprintf (stderr, "Out of memory.\n");
+	    return NULL;
+	}
+    }
+
+    return query_string;
+}
+
 int
 notmuch_tag_command (void *ctx, unused (int argc), unused (char *argv[]))
 {
@@ -44,6 +74,7 @@ notmuch_tag_command (void *ctx, unused (int argc), unused (char *argv[]))
     notmuch_message_t *message;
     struct sigaction action;
     notmuch_bool_t synchronize_flags;
+    notmuch_bool_t use_stdin = FALSE;
     int i;
 
     /* Setup our handler for SIGINT */
@@ -70,7 +101,9 @@ notmuch_tag_command (void *ctx, unused (int argc), unused (char *argv[]))
 	    i++;
 	    break;
 	}
-	if (argv[i][0] == '+') {
+	if (STRNCMP_LITERAL (argv[i], "--stdin") == 0) {
+	    use_stdin = TRUE;
+	} else if (argv[i][0] == '+') {
 	    add_tags[add_tags_count++] = i;
 	} else if (argv[i][0] == '-') {
 	    remove_tags[remove_tags_count++] = i;
@@ -84,7 +117,13 @@ notmuch_tag_command (void *ctx, unused (int argc), unused (char *argv[]))
 	return 1;
     }
 
-    query_string = query_string_from_args (ctx, argc - i, &argv[i]);
+    if (use_stdin)
+	query_string = query_string_from_stdin (ctx);
+    else
+	query_string = query_string_from_args (ctx, argc - i, &argv[i]);
+
+    if (!query_string)
+	return 1;
 
     if (*query_string == '\0') {
 	fprintf (stderr, "Error: notmuch tag requires at least one search term.\n");