Date: Tue, 5 Jul 2011 17:42:34 -0400
From: Austin Clements <amdragon@MIT.EDU>
To: Pieter Praet <pieter@praet.org>
Subject: Re: [PROTO] possible solution for "Race condition for '*' command"
Message-ID: <20110705214234.GA15360@mit.edu>
References: <20110703171743.GL15901@mit.edu>
	<1309762318-4530-1-git-send-email-pieter@praet.org>
	<e8c5fbf4-4dfa-461a-8f5c-6c696291a270@email.android.com>
	<87sjqldgr7.fsf@praet.org> <87iprg7dm0.fsf@praet.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87iprg7dm0.fsf@praet.org>
User-Agent: Mutt/1.5.20 (2009-06-14)
Cc: Notmuch Mail <notmuch@notmuchmail.org>
Precedence: list

Quoth Pieter Praet on Jul 05 at  9:04 pm:
> On Mon, 04 Jul 2011 20:48:12 +0200, Pieter Praet <pieter@praet.org> wrote:
> > On Mon, 04 Jul 2011 13:56:26 -0400, Austin Clements <amdragon@MIT.EDU> wrote:
> > > I should probably emit two lists per thread: one of matched IDs and
> > > one of unmatched IDs. Tagging a region can then operate on the
> > > concatenation of these, while * can operate only on the matched
> > > lists. This should be easy to do. I'll send an updated patch when I'm
> > > back at a computer.
> > 
> > The matched MsgIds will be sufficient, as we'll want to operate on
> > either the matched messages or the entire thread (for which the
> > `thread-id' property is already present).
> > 
> > Can't think of a use case for non-matched messages right now,
> > but if required, we'll just use `set-exclusive-or'.
> 
> Wasn't thinking clearly:
> 
> You're right, we *will* be needing both a list of matched as well as one
> of unmatched Message-Id's per result. Otherwise there would still be a
> potential race condition when tagging with +/-.

Yes, exactly.  (I had originally thought this race was a strict
superset of the '*' race; I now realize that's not the case, but
they're related enough that they might as well be addressed together.)

Below is an updated patch that separates the matched and unmatched
ID's (it's ugly, but no point in cleaning it up since it's a
prototype).  Now the tag list on each search line is followed by
something that looks like

  (id:x or id:y) or id:z

or just

  (id:x or id:y)

where the parenthesized part of the query is for the matched messages
and the (possibly empty) unparenthesized part is for the non-matched
messages.  This is designed to be easy for emacs to parse: grab just
the parenthesized part for the queries used by '*' and the whole thing
for region tagging queries.  This should also eliminate corner cases
for pasting together multiple queries with "or".

diff --git a/notmuch-search.c b/notmuch-search.c
index faccaf7..2288eb7 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -190,6 +190,28 @@ format_thread_json (const void *ctx,
     talloc_free (ctx_quote);
 }
 
+static void
+show_message_ids (notmuch_messages_t *messages, const char **first,
+		  notmuch_bool_t match)
+{
+    notmuch_message_t *message;
+
+    for (;
+	 notmuch_messages_valid (messages);
+	 notmuch_messages_move_to_next (messages)) {
+	message = notmuch_messages_get (messages);
+	if (notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH) == match) {
+	    if (*first)
+		fputs (*first, stdout);
+	    else
+		fputs (" or ", stdout);
+	    *first = NULL;
+	    printf ("id:\"%s\"", notmuch_message_get_message_id (message));
+	}
+	show_message_ids (notmuch_message_get_replies (message), first, match);
+    }
+}
+
 static int
 do_search_threads (const search_format_t *format,
 		   notmuch_query_t *query,
@@ -252,6 +274,22 @@ do_search_threads (const search_format_t *format,
 
 	    fputs (format->tag_end, stdout);
 
+	    if (format == &format_text) {
+		notmuch_messages_t *toplevel;
+		const char *first;
+
+		toplevel = notmuch_thread_get_toplevel_messages (thread);
+		first = " (";
+		show_message_ids (toplevel, &first, TRUE);
+		if (first)
+		    INTERNAL_ERROR ("No matched messages");
+		fputs (")", stdout);
+
+		toplevel = notmuch_thread_get_toplevel_messages (thread);
+		first = " or ";
+		show_message_ids (toplevel, &first, FALSE);
+	    }
+
 	    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");