[PATCH v2 00/11] Fix search tagging races
Tomi Ollila
tomi.ollila at iki.fi
Thu Nov 7 09:26:32 PST 2013
On Thu, Oct 24 2013, Austin Clements <amdragon at MIT.EDU> wrote:
> This is v2 of id:1381185201-25197-1-git-send-email-amdragon at mit.edu.
> It fixes several comments from Mark and Jani. This has been rebased
> on top of the tag completion changes, so doing * from a large search
> buffer will no longer crash. Hence, this series depends on the
> (currently pending) series in
> id:1382487721-31776-1-git-send-email-amdragon at mit.edu.
>
> This version does not address what happens when you * on a search
> buffer that's still filling. With this series, it will apply to all
> messages that have appeared when the user finishes entering tag
> changes. This isn't ideal, but this seems pretty obscure and I'm not
> sure what the right answer is, so I'm punting it to the future.
>
> Another thing that may be worth changing in a follow-up is what
> messages * applies to. Currently, * applies *only* to matched
> messages, not all threads in the search, which I think was an accident
> of implementation. This series retains that behavior, but opens up
> the possibility of applying to all threads instead. I think that
> would be much more consistent and less surprising behavior.
LGTM. applies cleanly and tests pass
Tomi
>
> An approximate diff from v1 is below. This diff is prior to rebasing,
> since the post-rebase diff is not useful.
>
> diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
> index a4eec14..36937fb 100644
> --- a/emacs/notmuch-tag.el
> +++ b/emacs/notmuch-tag.el
> @@ -242,7 +242,11 @@ from TAGS if present."
> (error "Changed tag must be of the form `+this_tag' or `-that_tag'")))))
> (sort result-tags 'string<)))
>
> -(defconst notmuch-tag-argument-limit 1000)
> +(defconst notmuch-tag-argument-limit 1000
> + "Use batch tagging if the tagging query is longer than this.
> +
> +This limits the length of arguments passed to the notmuch CLI to
> +avoid system argument length limits and performance problems.")
>
> (defun notmuch-tag (query &optional tag-changes)
> "Add/remove tags in TAG-CHANGES to messages matching QUERY.
> @@ -276,7 +280,6 @@ notmuch-after-tag-hook will be run."
> ;; Use batch tag mode to avoid argument length limitations
> (let ((batch-op (concat (mapconcat #'notmuch-hex-encode tag-changes " ")
> " -- " query)))
> - (message "Batch tagging with %s" batch-op)
> (notmuch-call-notmuch-process :stdin-string batch-op "tag" "--batch")))
> (run-hooks 'notmuch-after-tag-hook))
> ;; in all cases we return tag-changes as a list
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 1b14910..8bc1a2a 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -144,7 +144,8 @@ chomp_newline (char *str)
> #define NOTMUCH_FORMAT_MIN 1
> /* The minimum non-deprecated structured output format version.
> * Requests for format versions below this will print a stern warning.
> - * Must be >= NOTMUCH_FORMAT_MIN and < NOTMUCH_FORMAT_CUR.
> + * Must be between NOTMUCH_FORMAT_MIN and NOTMUCH_FORMAT_CUR,
> + * inclusive.
> */
> #define NOTMUCH_FORMAT_MIN_ACTIVE 1
>
> diff --git a/notmuch-search.c b/notmuch-search.c
> index 1d14651..7c973b3 100644
> --- a/notmuch-search.c
> +++ b/notmuch-search.c
> @@ -53,13 +53,13 @@ sanitize_string (const void *ctx, const char *str)
> * NULL. */
> static int
> get_thread_query (notmuch_thread_t *thread,
> - char **matched_out, char **unmached_out)
> + char **matched_out, char **unmatched_out)
> {
> notmuch_messages_t *messages;
> char *escaped = NULL;
> size_t escaped_len = 0;
>
> - *matched_out = *unmached_out = NULL;
> + *matched_out = *unmatched_out = NULL;
>
> for (messages = notmuch_thread_get_messages (thread);
> notmuch_messages_valid (messages);
> @@ -69,17 +69,16 @@ get_thread_query (notmuch_thread_t *thread,
> const char *mid = notmuch_message_get_message_id (message);
> /* Determine which query buffer to extend */
> char **buf = notmuch_message_get_flag (
> - message, NOTMUCH_MESSAGE_FLAG_MATCH) ? matched_out : unmached_out;
> - /* Allocate the query buffer is this is the first message */
> - if (!*buf && (*buf = talloc_strdup (thread, "")) == NULL)
> - return -1;
> + message, NOTMUCH_MESSAGE_FLAG_MATCH) ? matched_out : unmatched_out;
> /* Add this message's id: query. Since "id" is an exclusive
> * prefix, it is implicitly 'or'd together, so we only need to
> * join queries with a space. */
> if (make_boolean_term (thread, "id", mid, &escaped, &escaped_len) < 0)
> return -1;
> - *buf = talloc_asprintf_append_buffer (
> - *buf, "%s%s", **buf ? " " : "", escaped);
> + if (*buf)
> + *buf = talloc_asprintf_append_buffer (*buf, " %s", escaped);
> + else
> + *buf = talloc_strdup (thread, escaped);
> if (!*buf)
> return -1;
> }
>
>
> _______________________________________________
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
More information about the notmuch
mailing list