[PATCH v2 2/3] cli: refactor "notmuch tag" query tagging into a separate function
Tomi Ollila
tomi.ollila at iki.fi
Sun Mar 25 13:45:39 PDT 2012
Jani Nikula <jani at nikula.org> writes:
> Refactor to make tagging code easier to reuse in the future. No
> functional changes.
>
> Signed-off-by: Jani Nikula <jani at nikula.org>
> ---
tag_query() is pretty generic name for a function which (also) does
tagging operations (adds and removes tags based on tag_ops).
If, however, tag_opts != NULL (as is needs to be) but tag_opts[0] == NULL
(now tag_query() would not do any tagging operations, but
optimize_tag_query would mangle query string ( '*' to '()' and
'any_other' to '( any_other ) and ()' )
I.e. IMO the function name should be more spesific & the fact that this
function needs tag changing data in tag_ops array should be documented.
---
Minor thing in patch #1:
+ tag_ops[tag_ops_count].remove = argv[i][0] == '-';
would be more clearer as:
+ tag_ops[tag_ops_count].remove = (argv[i][0] == '-');
Everything else LGTM.
Tomi
> notmuch-tag.c | 101 ++++++++++++++++++++++++++++++++-------------------------
> 1 files changed, 57 insertions(+), 44 deletions(-)
>
> diff --git a/notmuch-tag.c b/notmuch-tag.c
> index c39b235..edbfdec 100644
> --- a/notmuch-tag.c
> +++ b/notmuch-tag.c
> @@ -106,6 +106,60 @@ _optimize_tag_query (void *ctx, const char *orig_query_string,
> return query_string;
> }
>
> +static int
> +tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string,
> + tag_operation_t *tag_ops, notmuch_bool_t synchronize_flags)
> +{
> + notmuch_query_t *query;
> + notmuch_messages_t *messages;
> + notmuch_message_t *message;
> + int i;
> +
> + /* Optimize the query so it excludes messages that already have
> + * the specified set of tags. */
> + query_string = _optimize_tag_query (ctx, query_string, tag_ops);
> + if (query_string == NULL) {
> + fprintf (stderr, "Out of memory.\n");
> + return 1;
> + }
> +
> + query = notmuch_query_create (notmuch, query_string);
> + if (query == NULL) {
> + fprintf (stderr, "Out of memory.\n");
> + return 1;
> + }
> +
> + /* tagging is not interested in any special sort order */
> + notmuch_query_set_sort (query, NOTMUCH_SORT_UNSORTED);
> +
> + for (messages = notmuch_query_search_messages (query);
> + notmuch_messages_valid (messages) && !interrupted;
> + notmuch_messages_move_to_next (messages))
> + {
> + message = notmuch_messages_get (messages);
> +
> + notmuch_message_freeze (message);
> +
> + for (i = 0; tag_ops[i].tag; i++) {
> + if (tag_ops[i].remove)
> + notmuch_message_remove_tag (message, tag_ops[i].tag);
> + else
> + notmuch_message_add_tag (message, tag_ops[i].tag);
> + }
> +
> + notmuch_message_thaw (message);
> +
> + if (synchronize_flags)
> + notmuch_message_tags_to_maildir_flags (message);
> +
> + notmuch_message_destroy (message);
> + }
> +
> + notmuch_query_destroy (query);
> +
> + return interrupted;
> +}
> +
> int
> notmuch_tag_command (void *ctx, int argc, char *argv[])
> {
> @@ -114,12 +168,10 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
> char *query_string;
> notmuch_config_t *config;
> notmuch_database_t *notmuch;
> - notmuch_query_t *query;
> - notmuch_messages_t *messages;
> - notmuch_message_t *message;
> struct sigaction action;
> notmuch_bool_t synchronize_flags;
> int i;
> + int ret;
>
> /* Setup our handler for SIGINT */
> memset (&action, 0, sizeof (struct sigaction));
> @@ -166,14 +218,6 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
> return 1;
> }
>
> - /* Optimize the query so it excludes messages that already have
> - * the specified set of tags. */
> - query_string = _optimize_tag_query (ctx, query_string, tag_ops);
> - if (query_string == NULL) {
> - fprintf (stderr, "Out of memory.\n");
> - return 1;
> - }
> -
> config = notmuch_config_open (ctx, NULL, NULL);
> if (config == NULL)
> return 1;
> @@ -185,40 +229,9 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
>
> synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
>
> - query = notmuch_query_create (notmuch, query_string);
> - if (query == NULL) {
> - fprintf (stderr, "Out of memory.\n");
> - return 1;
> - }
> -
> - /* tagging is not interested in any special sort order */
> - notmuch_query_set_sort (query, NOTMUCH_SORT_UNSORTED);
> + ret = tag_query (ctx, notmuch, query_string, tag_ops, synchronize_flags);
>
> - for (messages = notmuch_query_search_messages (query);
> - notmuch_messages_valid (messages) && !interrupted;
> - notmuch_messages_move_to_next (messages))
> - {
> - message = notmuch_messages_get (messages);
> -
> - notmuch_message_freeze (message);
> -
> - for (i = 0; tag_ops[i].tag; i++) {
> - if (tag_ops[i].remove)
> - notmuch_message_remove_tag (message, tag_ops[i].tag);
> - else
> - notmuch_message_add_tag (message, tag_ops[i].tag);
> - }
> -
> - notmuch_message_thaw (message);
> -
> - if (synchronize_flags)
> - notmuch_message_tags_to_maildir_flags (message);
> -
> - notmuch_message_destroy (message);
> - }
> -
> - notmuch_query_destroy (query);
> notmuch_database_close (notmuch);
>
> - return interrupted;
> + return ret;
> }
> --
> 1.7.5.4
>
> _______________________________________________
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
More information about the notmuch
mailing list