[PATCH] vim: fix regex after "notmuch show" output change

2012-03-25 Thread Tomi Ollila
Jakob  writes:

> On Sat, 24 Mar 2012 10:58:59 +0200, Tomi Ollila  wrote:
>> Is this regexp part below good ?
>> 
>> > +... match:\([0-9]*\) excluded:\([[0-9]*\) filename:\(.*\)$', ...
>> 
>> ( --> excluded:\([[0-9]*\) <-- )
>
> Yeah, that was the core of the change.  With this new field in the
> output, the old regex didn't match at all.  You can see the fix working by
> trying to use the vim plugin as-is and trying to reply to a message or
> view a message's ID or any number of other things that fail right now.
> After updating this regex, everything starts working again.

I meant about the syntax: \([[0-9]*\) i.e. 2 opening [[:s

>
> Thanks,
> Jakob

Tomi


[PATCH v2 3/3] cli: refactor "notmuch restore" message tagging into a separate function

2012-03-25 Thread Jani Nikula
Refactor to make tagging code easier to reuse in the future. No
functional changes.

Signed-off-by: Jani Nikula 
---
 notmuch-restore.c |  148 -
 1 files changed, 78 insertions(+), 70 deletions(-)

diff --git a/notmuch-restore.c b/notmuch-restore.c
index 87d9772..2e965af 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -21,6 +21,81 @@
 #include "notmuch-client.h"

 int
+tag_message (void *ctx, notmuch_database_t *notmuch, const char *message_id,
+char *file_tags, notmuch_bool_t remove_all,
+notmuch_bool_t synchronize_flags)
+{
+notmuch_status_t status;
+notmuch_tags_t *db_tags;
+char *db_tags_str;
+notmuch_message_t *message = NULL;
+const char *tag;
+char *next;
+int ret = 0;
+
+status = notmuch_database_find_message (notmuch, message_id, );
+if (status || message == NULL) {
+   fprintf (stderr, "Warning: Cannot apply tags to %smessage: %s\n",
+message ? "" : "missing ", message_id);
+   if (status)
+   fprintf (stderr, "%s\n", notmuch_status_to_string(status));
+   return 1;
+}
+
+/* In order to detect missing messages, this check/optimization is
+ * intentionally done *after* first finding the message. */
+if (!remove_all && (file_tags == NULL || *file_tags == '\0'))
+   goto DONE;
+
+db_tags_str = NULL;
+for (db_tags = notmuch_message_get_tags (message);
+notmuch_tags_valid (db_tags);
+notmuch_tags_move_to_next (db_tags)) {
+   tag = notmuch_tags_get (db_tags);
+
+   if (db_tags_str)
+   db_tags_str = talloc_asprintf_append (db_tags_str, " %s", tag);
+   else
+   db_tags_str = talloc_strdup (message, tag);
+}
+
+if (((file_tags == NULL || *file_tags == '\0') &&
+(db_tags_str == NULL || *db_tags_str == '\0')) ||
+   (file_tags && db_tags_str && strcmp (file_tags, db_tags_str) == 0))
+   goto DONE;
+
+notmuch_message_freeze (message);
+
+if (remove_all)
+   notmuch_message_remove_all_tags (message);
+
+next = file_tags;
+while (next) {
+   tag = strsep (, " ");
+   if (*tag == '\0')
+   continue;
+   status = notmuch_message_add_tag (message, tag);
+   if (status) {
+   fprintf (stderr, "Error applying tag %s to message %s:\n",
+tag, message_id);
+   fprintf (stderr, "%s\n", notmuch_status_to_string (status));
+   ret = 1;
+   }
+}
+
+notmuch_message_thaw (message);
+
+if (synchronize_flags)
+   notmuch_message_tags_to_maildir_flags (message);
+
+DONE:
+if (message)
+   notmuch_message_destroy (message);
+
+return ret;
+}
+
+int
 notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
 {
 notmuch_config_t *config;
@@ -88,11 +163,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char 
*argv[])

 while ((line_len = getline (, _size, input)) != -1) {
regmatch_t match[3];
-   char *message_id, *file_tags, *tag, *next;
-   notmuch_message_t *message = NULL;
-   notmuch_status_t status;
-   notmuch_tags_t *db_tags;
-   char *db_tags_str;
+   char *message_id, *file_tags;

chomp_newline (line);

@@ -109,72 +180,9 @@ notmuch_restore_command (unused (void *ctx), int argc, 
char *argv[])
file_tags = xstrndup (line + match[2].rm_so,
  match[2].rm_eo - match[2].rm_so);

-   status = notmuch_database_find_message (notmuch, message_id, );
-   if (status || message == NULL) {
-   fprintf (stderr, "Warning: Cannot apply tags to %smessage: %s\n",
-message ? "" : "missing ", message_id);
-   if (status)
-   fprintf (stderr, "%s\n",
-notmuch_status_to_string(status));
-   goto NEXT_LINE;
-   }
-
-   /* In order to detect missing messages, this check/optimization is
-* intentionally done *after* first finding the message.  */
-   if (accumulate && (file_tags == NULL || *file_tags == '\0'))
-   {
-   goto NEXT_LINE;
-   }
-
-   db_tags_str = NULL;
-   for (db_tags = notmuch_message_get_tags (message);
-notmuch_tags_valid (db_tags);
-notmuch_tags_move_to_next (db_tags))
-   {
-   const char *tag = notmuch_tags_get (db_tags);
-
-   if (db_tags_str)
-   db_tags_str = talloc_asprintf_append (db_tags_str, " %s", tag);
-   else
-   db_tags_str = talloc_strdup (message, tag);
-   }
-
-   if (((file_tags == NULL || *file_tags == '\0') &&
-(db_tags_str == NULL || *db_tags_str == '\0')) ||
-   (file_tags && db_tags_str && strcmp (file_tags, db_tags_str) == 0))
-   {
-   goto NEXT_LINE;
-   }
-
-   notmuch_message_freeze (message);
-
-   if (!accumulate)
-   notmuch_message_remove_all_tags (message);

[PATCH v2 2/3] cli: refactor "notmuch tag" query tagging into a separate function

2012-03-25 Thread Jani Nikula
Refactor to make tagging code easier to reuse in the future. No
functional changes.

Signed-off-by: Jani Nikula 
---
 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 (, 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



[PATCH v2 1/3] cli: refactor "notmuch tag" data structures for tagging operations

2012-03-25 Thread Jani Nikula
To simplify code, keep all tagging operations in a single array
instead of separate add and remove arrays. Apply tag changes in the
order specified on the command line, instead of first removing and
then adding the tags.

This results in a minor functional change: If a tag is both added and
removed, the last specified operation is now used. Previously the tag
was always added.

Signed-off-by: Jani Nikula 
---
 notmuch-tag.c |   79 +---
 1 files changed, 35 insertions(+), 44 deletions(-)

diff --git a/notmuch-tag.c b/notmuch-tag.c
index 36b9b09..c39b235 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -53,10 +53,14 @@ _escape_tag (char *buf, const char *tag)
 return buf;
 }

+typedef struct {
+const char *tag;
+notmuch_bool_t remove;
+} tag_operation_t;
+
 static char *
-_optimize_tag_query (void *ctx, const char *orig_query_string, char *argv[],
-int *add_tags, int add_tags_count,
-int *remove_tags, int remove_tags_count)
+_optimize_tag_query (void *ctx, const char *orig_query_string,
+const tag_operation_t *tag_ops)
 {
 /* This is subtler than it looks.  Xapian ignores the '-' operator
  * at the beginning both queries and parenthesized groups and,
@@ -74,12 +78,9 @@ _optimize_tag_query (void *ctx, const char 
*orig_query_string, char *argv[],
 /* Allocate a buffer for escaping tags.  This is large enough to
  * hold a fully escaped tag with every character doubled plus
  * enclosing quotes and a NUL. */
-for (i = 0; i < add_tags_count; i++)
-   if (strlen (argv[add_tags[i]] + 1) > max_tag_len)
-   max_tag_len = strlen (argv[add_tags[i]] + 1);
-for (i = 0; i < remove_tags_count; i++)
-   if (strlen (argv[remove_tags[i]] + 1) > max_tag_len)
-   max_tag_len = strlen (argv[remove_tags[i]] + 1);
+for (i = 0; tag_ops[i].tag; i++)
+   if (strlen (tag_ops[i].tag) > max_tag_len)
+   max_tag_len = strlen (tag_ops[i].tag);
 escaped = talloc_array(ctx, char, max_tag_len * 2 + 3);
 if (!escaped)
return NULL;
@@ -90,16 +91,11 @@ _optimize_tag_query (void *ctx, const char 
*orig_query_string, char *argv[],
 else
query_string = talloc_asprintf (ctx, "( %s ) and (", orig_query_string);

-for (i = 0; i < add_tags_count && query_string; i++) {
-   query_string = talloc_asprintf_append_buffer (
-   query_string, "%snot tag:%s", join,
-   _escape_tag (escaped, argv[add_tags[i]] + 1));
-   join = " or ";
-}
-for (i = 0; i < remove_tags_count && query_string; i++) {
+for (i = 0; tag_ops[i].tag && query_string; i++) {
query_string = talloc_asprintf_append_buffer (
-   query_string, "%stag:%s", join,
-   _escape_tag (escaped, argv[remove_tags[i]] + 1));
+   query_string, "%s%stag:%s", join,
+   tag_ops[i].remove ? "" : "not ",
+   _escape_tag (escaped, tag_ops[i].tag));
join = " or ";
 }

@@ -113,9 +109,8 @@ _optimize_tag_query (void *ctx, const char 
*orig_query_string, char *argv[],
 int
 notmuch_tag_command (void *ctx, int argc, char *argv[])
 {
-int *add_tags, *remove_tags;
-int add_tags_count = 0;
-int remove_tags_count = 0;
+tag_operation_t *tag_ops;
+int tag_ops_count = 0;
 char *query_string;
 notmuch_config_t *config;
 notmuch_database_t *notmuch;
@@ -133,35 +128,33 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
 action.sa_flags = SA_RESTART;
 sigaction (SIGINT, , NULL);

-add_tags = talloc_size (ctx, argc * sizeof (int));
-if (add_tags == NULL) {
-   fprintf (stderr, "Out of memory.\n");
-   return 1;
-}
+argc--; argv++; /* skip subcommand argument */

-remove_tags = talloc_size (ctx, argc * sizeof (int));
-if (remove_tags == NULL) {
+/* Array of tagging operations (add or remove), terminated with an
+ * empty element. */
+tag_ops = talloc_array (ctx, tag_operation_t, argc + 1);
+if (tag_ops == NULL) {
fprintf (stderr, "Out of memory.\n");
return 1;
 }

-argc--; argv++; /* skip subcommand argument */
-
 for (i = 0; i < argc; i++) {
if (strcmp (argv[i], "--") == 0) {
i++;
break;
}
-   if (argv[i][0] == '+') {
-   add_tags[add_tags_count++] = i;
-   } else if (argv[i][0] == '-') {
-   remove_tags[remove_tags_count++] = i;
+   if (argv[i][0] == '+' || argv[i][0] == '-') {
+   tag_ops[tag_ops_count].tag = argv[i] + 1;
+   tag_ops[tag_ops_count].remove = argv[i][0] == '-';
+   tag_ops_count++;
} else {
break;
}
 }

-if (add_tags_count == 0 && remove_tags_count == 0) {
+tag_ops[tag_ops_count].tag = NULL;
+
+if (tag_ops_count == 0) {
fprintf (stderr, "Error: 'notmuch tag' requires at least one tag to add 
or remove.\n");
return 1;
 

[PATCH v2 0/3] cli: notmuch tag/restore refactoring

2012-03-25 Thread Jani Nikula
v2 of id:"cover.1332604895.git.jani at nikula.org" with the following
non-functional changes, addressing David's concerns in mail and IRC:

 - do not use C99 style struct assignment in patch 1
 - for now, keep tag_message() local to notmuch-restore.c in patch 3

BR,
Jani.

Jani Nikula (3):
  cli: refactor "notmuch tag" data structures for tagging operations
  cli: refactor "notmuch tag" query tagging into a separate function
  cli: refactor "notmuch restore" message tagging into a separate
function

 notmuch-restore.c |  148 +--
 notmuch-tag.c |  166 +++--
 2 files changed, 163 insertions(+), 151 deletions(-)

-- 
1.7.5.4



[BUG/PATCH 2/2] emacs: Fix replying from alternate addresses

2012-03-25 Thread Mark Walters
Adam Wolfe Gordon  writes:
> The bug was that notmuch-mua-mail used `mail-header` to check whether
> it was passed a "From" header. The implementation of `mail-header`
> must try to compare symbols instead of strings when looking for
> headers, as it was returning nil when a From header was present. This
> is probably because the mail functions construct headers as alists
> with symbols for the header names, while our code uses strings for the
> header names.
>
> Since we don't use `mail-header` anywhere else, and `message-mail` is
> perfectly happy to accept string header names, the fix is just to use
> `assoc` to look for the From header, so that the strings get compared
> properly.

I can confirm that everything works as expected with this fix. 

Best wishes

Mark
> ---
>  emacs/notmuch-mua.el |4 ++--
>  test/emacs   |1 -
>  2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
> index 6aae3a0..9805d79 100644
> --- a/emacs/notmuch-mua.el
> +++ b/emacs/notmuch-mua.el
> @@ -187,7 +187,7 @@ OTHER-ARGS are passed through to `message-mail'."
>(when (not (string= "" user-agent))
>   (push (cons "User-Agent" user-agent) other-headers
>  
> -  (unless (mail-header 'From other-headers)
> +  (unless (assoc "From" other-headers)
>  (push (cons "From" (concat
>   (notmuch-user-name) " <" (notmuch-user-primary-email) 
> ">")) other-headers))
>  
> @@ -250,7 +250,7 @@ the From: address first."
>(interactive "P")
>(let ((other-headers
>(when (or prompt-for-sender notmuch-always-prompt-for-sender)
> -(list (cons 'From (notmuch-mua-prompt-for-sender))
> +(list (cons "From" (notmuch-mua-prompt-for-sender))
>  (notmuch-mua-mail nil nil other-headers)))
>  
>  (defun notmuch-mua-new-forward-message ( prompt-for-sender)
> diff --git a/test/emacs b/test/emacs
> index fa5d706..08db1ee 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -275,7 +275,6 @@ EOF
>  test_expect_equal_file OUTPUT EXPECTED
>  
>  test_begin_subtest "Reply from alternate address within emacs"
> -test_subtest_known_broken
>  add_message '[from]="Sender "' \
>[to]=test_suite_other at notmuchmail.org \
>[subject]=notmuch-reply-test \
> -- 
> 1.7.5.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] vim: fix regex after "notmuch show" output change

2012-03-25 Thread Jakob
On Sat, 24 Mar 2012 10:58:59 +0200, Tomi Ollila  wrote:
> Is this regexp part below good ?
> 
> > +... match:\([0-9]*\) excluded:\([[0-9]*\) filename:\(.*\)$', ...
> 
> ( --> excluded:\([[0-9]*\) <-- )

Yeah, that was the core of the change.  With this new field in the
output, the old regex didn't match at all.  You can see the fix working by
trying to use the vim plugin as-is and trying to reply to a message or
view a message's ID or any number of other things that fail right now.
After updating this regex, everything starts working again.

Thanks,
Jakob


[PATCH 3/3] cli: refactor "notmuch restore" message tagging into a separate function

2012-03-25 Thread David Bremner
On Sat, 24 Mar 2012 18:14:37 +0200, Jani Nikula  wrote:
> Refactor to make tagging code easier to reuse in the future. No
> functional changes.

The second two look OK as well.  It isn't obvious to me that in the long
term we want an API using "file_tags" to pass in a string. It reminds me
of the term "stringly typed". Of course, I see why it is this way in an
intial refactor, but maybe something to think about before we start
propagating such an API.

d


mutt-notmuch in notmuch contrib

2012-03-25 Thread David Bremner

Hi Zack

In the mean time, we at least have a contrib directory, and I think
mutt-notmuch would be welcome there. Could you (or somebody) make a
reasonable size patch series that adds it to contrib/mutt-notmuch
against current master. The patch series should be sent to
the upstream mailing list notmuch at notmuchmail.org for review.

On the debian side your patch series could also include the necessary
changes to make a new binary package.

Notmuch itself is already GPL3+ so no hassles there (for once).


d



[PATCH 1/3] cli: refactor "notmuch tag" data structures for tagging operations

2012-03-25 Thread David Bremner
On Sat, 24 Mar 2012 18:14:35 +0200, Jani Nikula  wrote:
> To simplify code, keep all tagging operations in a single array
> instead of separate add and remove arrays. Apply tag changes in the
> order specified on the command line, instead of first removing and
> then adding the tags.

Like I said on IRC, my only minor reservation with this patch is that
the use of C99 struct assignment is maybe not a good tradeoff here,
since it makes the code a bit more mysterious (for the great unwashed
masses that don't know C99 as well as C89). Still, if you think extra
initialization features are worth it, I can live with it. 

d


[PATCH] emacs: content-type comparison should be case insensitive.

2012-03-25 Thread Mark Walters
The function notmuch-match-content-type was comparing content types
case sensitively. Fix it so it tests case insensitively.

This fixes a bug where emacs would not include any body when replying
to a message with content-type TEXT/PLAIN.
---
 emacs/notmuch-lib.el |5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index c146748..a754de7 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -185,8 +185,9 @@ the user hasn't set this variable with the old or new 
value."
(st2 (notmuch-split-content-type t2)))
 (if (or (string= (cadr st1) "*")
(string= (cadr st2) "*"))
-   (string= (car st1) (car st2))
-  (string= t1 t2
+   ;; Comparison of content types should be case insensitive.
+   (string= (downcase (car st1)) (downcase (car st2)))
+  (string= (downcase t1) (downcase t2)

 (defvar notmuch-multipart/alternative-discouraged
   '(
-- 
1.7.9.1



Re: [BUG/PATCH 2/2] emacs: Fix replying from alternate addresses

2012-03-25 Thread Mark Walters
Adam Wolfe Gordon awg+notm...@xvx.ca writes:
 The bug was that notmuch-mua-mail used `mail-header` to check whether
 it was passed a From header. The implementation of `mail-header`
 must try to compare symbols instead of strings when looking for
 headers, as it was returning nil when a From header was present. This
 is probably because the mail functions construct headers as alists
 with symbols for the header names, while our code uses strings for the
 header names.

 Since we don't use `mail-header` anywhere else, and `message-mail` is
 perfectly happy to accept string header names, the fix is just to use
 `assoc` to look for the From header, so that the strings get compared
 properly.

I can confirm that everything works as expected with this fix. 

Best wishes

Mark
 ---
  emacs/notmuch-mua.el |4 ++--
  test/emacs   |1 -
  2 files changed, 2 insertions(+), 3 deletions(-)

 diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
 index 6aae3a0..9805d79 100644
 --- a/emacs/notmuch-mua.el
 +++ b/emacs/notmuch-mua.el
 @@ -187,7 +187,7 @@ OTHER-ARGS are passed through to `message-mail'.
(when (not (string=  user-agent))
   (push (cons User-Agent user-agent) other-headers
  
 -  (unless (mail-header 'From other-headers)
 +  (unless (assoc From other-headers)
  (push (cons From (concat
   (notmuch-user-name)   (notmuch-user-primary-email) 
 )) other-headers))
  
 @@ -250,7 +250,7 @@ the From: address first.
(interactive P)
(let ((other-headers
(when (or prompt-for-sender notmuch-always-prompt-for-sender)
 -(list (cons 'From (notmuch-mua-prompt-for-sender))
 +(list (cons From (notmuch-mua-prompt-for-sender))
  (notmuch-mua-mail nil nil other-headers)))
  
  (defun notmuch-mua-new-forward-message (optional prompt-for-sender)
 diff --git a/test/emacs b/test/emacs
 index fa5d706..08db1ee 100755
 --- a/test/emacs
 +++ b/test/emacs
 @@ -275,7 +275,6 @@ EOF
  test_expect_equal_file OUTPUT EXPECTED
  
  test_begin_subtest Reply from alternate address within emacs
 -test_subtest_known_broken
  add_message '[from]=Sender sen...@example.com' \
[to]=test_suite_ot...@notmuchmail.org \
[subject]=notmuch-reply-test \
 -- 
 1.7.5.4

 ___
 notmuch mailing list
 notmuch@notmuchmail.org
 http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


mutt-notmuch in notmuch contrib

2012-03-25 Thread David Bremner

Hi Zack

In the mean time, we at least have a contrib directory, and I think
mutt-notmuch would be welcome there. Could you (or somebody) make a
reasonable size patch series that adds it to contrib/mutt-notmuch
against current master. The patch series should be sent to
the upstream mailing list notmuch@notmuchmail.org for review.

On the debian side your patch series could also include the necessary
changes to make a new binary package.

Notmuch itself is already GPL3+ so no hassles there (for once).


d

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] vim: fix regex after notmuch show output change

2012-03-25 Thread Jakob
On Sat, 24 Mar 2012 10:58:59 +0200, Tomi Ollila tomi.oll...@iki.fi wrote:
 Is this regexp part below good ?
 
  +... match:\([0-9]*\) excluded:\([[0-9]*\) filename:\(.*\)$', ...
 
 ( -- excluded:\([[0-9]*\) -- )

Yeah, that was the core of the change.  With this new field in the
output, the old regex didn't match at all.  You can see the fix working by
trying to use the vim plugin as-is and trying to reply to a message or
view a message's ID or any number of other things that fail right now.
After updating this regex, everything starts working again.

Thanks,
Jakob
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 0/3] cli: notmuch tag/restore refactoring

2012-03-25 Thread Jani Nikula
v2 of id:cover.1332604895.git.j...@nikula.org with the following
non-functional changes, addressing David's concerns in mail and IRC:

 - do not use C99 style struct assignment in patch 1
 - for now, keep tag_message() local to notmuch-restore.c in patch 3

BR,
Jani.

Jani Nikula (3):
  cli: refactor notmuch tag data structures for tagging operations
  cli: refactor notmuch tag query tagging into a separate function
  cli: refactor notmuch restore message tagging into a separate
function

 notmuch-restore.c |  148 +--
 notmuch-tag.c |  166 +++--
 2 files changed, 163 insertions(+), 151 deletions(-)

-- 
1.7.5.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 1/3] cli: refactor notmuch tag data structures for tagging operations

2012-03-25 Thread Jani Nikula
To simplify code, keep all tagging operations in a single array
instead of separate add and remove arrays. Apply tag changes in the
order specified on the command line, instead of first removing and
then adding the tags.

This results in a minor functional change: If a tag is both added and
removed, the last specified operation is now used. Previously the tag
was always added.

Signed-off-by: Jani Nikula j...@nikula.org
---
 notmuch-tag.c |   79 +---
 1 files changed, 35 insertions(+), 44 deletions(-)

diff --git a/notmuch-tag.c b/notmuch-tag.c
index 36b9b09..c39b235 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -53,10 +53,14 @@ _escape_tag (char *buf, const char *tag)
 return buf;
 }
 
+typedef struct {
+const char *tag;
+notmuch_bool_t remove;
+} tag_operation_t;
+
 static char *
-_optimize_tag_query (void *ctx, const char *orig_query_string, char *argv[],
-int *add_tags, int add_tags_count,
-int *remove_tags, int remove_tags_count)
+_optimize_tag_query (void *ctx, const char *orig_query_string,
+const tag_operation_t *tag_ops)
 {
 /* This is subtler than it looks.  Xapian ignores the '-' operator
  * at the beginning both queries and parenthesized groups and,
@@ -74,12 +78,9 @@ _optimize_tag_query (void *ctx, const char 
*orig_query_string, char *argv[],
 /* Allocate a buffer for escaping tags.  This is large enough to
  * hold a fully escaped tag with every character doubled plus
  * enclosing quotes and a NUL. */
-for (i = 0; i  add_tags_count; i++)
-   if (strlen (argv[add_tags[i]] + 1)  max_tag_len)
-   max_tag_len = strlen (argv[add_tags[i]] + 1);
-for (i = 0; i  remove_tags_count; i++)
-   if (strlen (argv[remove_tags[i]] + 1)  max_tag_len)
-   max_tag_len = strlen (argv[remove_tags[i]] + 1);
+for (i = 0; tag_ops[i].tag; i++)
+   if (strlen (tag_ops[i].tag)  max_tag_len)
+   max_tag_len = strlen (tag_ops[i].tag);
 escaped = talloc_array(ctx, char, max_tag_len * 2 + 3);
 if (!escaped)
return NULL;
@@ -90,16 +91,11 @@ _optimize_tag_query (void *ctx, const char 
*orig_query_string, char *argv[],
 else
query_string = talloc_asprintf (ctx, ( %s ) and (, orig_query_string);
 
-for (i = 0; i  add_tags_count  query_string; i++) {
-   query_string = talloc_asprintf_append_buffer (
-   query_string, %snot tag:%s, join,
-   _escape_tag (escaped, argv[add_tags[i]] + 1));
-   join =  or ;
-}
-for (i = 0; i  remove_tags_count  query_string; i++) {
+for (i = 0; tag_ops[i].tag  query_string; i++) {
query_string = talloc_asprintf_append_buffer (
-   query_string, %stag:%s, join,
-   _escape_tag (escaped, argv[remove_tags[i]] + 1));
+   query_string, %s%stag:%s, join,
+   tag_ops[i].remove ?  : not ,
+   _escape_tag (escaped, tag_ops[i].tag));
join =  or ;
 }
 
@@ -113,9 +109,8 @@ _optimize_tag_query (void *ctx, const char 
*orig_query_string, char *argv[],
 int
 notmuch_tag_command (void *ctx, int argc, char *argv[])
 {
-int *add_tags, *remove_tags;
-int add_tags_count = 0;
-int remove_tags_count = 0;
+tag_operation_t *tag_ops;
+int tag_ops_count = 0;
 char *query_string;
 notmuch_config_t *config;
 notmuch_database_t *notmuch;
@@ -133,35 +128,33 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
 action.sa_flags = SA_RESTART;
 sigaction (SIGINT, action, NULL);
 
-add_tags = talloc_size (ctx, argc * sizeof (int));
-if (add_tags == NULL) {
-   fprintf (stderr, Out of memory.\n);
-   return 1;
-}
+argc--; argv++; /* skip subcommand argument */
 
-remove_tags = talloc_size (ctx, argc * sizeof (int));
-if (remove_tags == NULL) {
+/* Array of tagging operations (add or remove), terminated with an
+ * empty element. */
+tag_ops = talloc_array (ctx, tag_operation_t, argc + 1);
+if (tag_ops == NULL) {
fprintf (stderr, Out of memory.\n);
return 1;
 }
 
-argc--; argv++; /* skip subcommand argument */
-
 for (i = 0; i  argc; i++) {
if (strcmp (argv[i], --) == 0) {
i++;
break;
}
-   if (argv[i][0] == '+') {
-   add_tags[add_tags_count++] = i;
-   } else if (argv[i][0] == '-') {
-   remove_tags[remove_tags_count++] = i;
+   if (argv[i][0] == '+' || argv[i][0] == '-') {
+   tag_ops[tag_ops_count].tag = argv[i] + 1;
+   tag_ops[tag_ops_count].remove = argv[i][0] == '-';
+   tag_ops_count++;
} else {
break;
}
 }
 
-if (add_tags_count == 0  remove_tags_count == 0) {
+tag_ops[tag_ops_count].tag = NULL;
+
+if (tag_ops_count == 0) {
fprintf (stderr, Error: 'notmuch tag' requires at least one tag to add 
or remove.\n);
return 1;
 }
@@ -175,9 

[PATCH v2 2/3] cli: refactor notmuch tag query tagging into a separate function

2012-03-25 Thread Jani Nikula
Refactor to make tagging code easier to reuse in the future. No
functional changes.

Signed-off-by: Jani Nikula j...@nikula.org
---
 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@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 3/3] cli: refactor notmuch restore message tagging into a separate function

2012-03-25 Thread Jani Nikula
Refactor to make tagging code easier to reuse in the future. No
functional changes.

Signed-off-by: Jani Nikula j...@nikula.org
---
 notmuch-restore.c |  148 -
 1 files changed, 78 insertions(+), 70 deletions(-)

diff --git a/notmuch-restore.c b/notmuch-restore.c
index 87d9772..2e965af 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -21,6 +21,81 @@
 #include notmuch-client.h
 
 int
+tag_message (void *ctx, notmuch_database_t *notmuch, const char *message_id,
+char *file_tags, notmuch_bool_t remove_all,
+notmuch_bool_t synchronize_flags)
+{
+notmuch_status_t status;
+notmuch_tags_t *db_tags;
+char *db_tags_str;
+notmuch_message_t *message = NULL;
+const char *tag;
+char *next;
+int ret = 0;
+
+status = notmuch_database_find_message (notmuch, message_id, message);
+if (status || message == NULL) {
+   fprintf (stderr, Warning: Cannot apply tags to %smessage: %s\n,
+message ?  : missing , message_id);
+   if (status)
+   fprintf (stderr, %s\n, notmuch_status_to_string(status));
+   return 1;
+}
+
+/* In order to detect missing messages, this check/optimization is
+ * intentionally done *after* first finding the message. */
+if (!remove_all  (file_tags == NULL || *file_tags == '\0'))
+   goto DONE;
+
+db_tags_str = NULL;
+for (db_tags = notmuch_message_get_tags (message);
+notmuch_tags_valid (db_tags);
+notmuch_tags_move_to_next (db_tags)) {
+   tag = notmuch_tags_get (db_tags);
+
+   if (db_tags_str)
+   db_tags_str = talloc_asprintf_append (db_tags_str,  %s, tag);
+   else
+   db_tags_str = talloc_strdup (message, tag);
+}
+
+if (((file_tags == NULL || *file_tags == '\0') 
+(db_tags_str == NULL || *db_tags_str == '\0')) ||
+   (file_tags  db_tags_str  strcmp (file_tags, db_tags_str) == 0))
+   goto DONE;
+
+notmuch_message_freeze (message);
+
+if (remove_all)
+   notmuch_message_remove_all_tags (message);
+
+next = file_tags;
+while (next) {
+   tag = strsep (next,  );
+   if (*tag == '\0')
+   continue;
+   status = notmuch_message_add_tag (message, tag);
+   if (status) {
+   fprintf (stderr, Error applying tag %s to message %s:\n,
+tag, message_id);
+   fprintf (stderr, %s\n, notmuch_status_to_string (status));
+   ret = 1;
+   }
+}
+
+notmuch_message_thaw (message);
+
+if (synchronize_flags)
+   notmuch_message_tags_to_maildir_flags (message);
+
+DONE:
+if (message)
+   notmuch_message_destroy (message);
+
+return ret;
+}
+
+int
 notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
 {
 notmuch_config_t *config;
@@ -88,11 +163,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char 
*argv[])
 
 while ((line_len = getline (line, line_size, input)) != -1) {
regmatch_t match[3];
-   char *message_id, *file_tags, *tag, *next;
-   notmuch_message_t *message = NULL;
-   notmuch_status_t status;
-   notmuch_tags_t *db_tags;
-   char *db_tags_str;
+   char *message_id, *file_tags;
 
chomp_newline (line);
 
@@ -109,72 +180,9 @@ notmuch_restore_command (unused (void *ctx), int argc, 
char *argv[])
file_tags = xstrndup (line + match[2].rm_so,
  match[2].rm_eo - match[2].rm_so);
 
-   status = notmuch_database_find_message (notmuch, message_id, message);
-   if (status || message == NULL) {
-   fprintf (stderr, Warning: Cannot apply tags to %smessage: %s\n,
-message ?  : missing , message_id);
-   if (status)
-   fprintf (stderr, %s\n,
-notmuch_status_to_string(status));
-   goto NEXT_LINE;
-   }
-
-   /* In order to detect missing messages, this check/optimization is
-* intentionally done *after* first finding the message.  */
-   if (accumulate  (file_tags == NULL || *file_tags == '\0'))
-   {
-   goto NEXT_LINE;
-   }
-
-   db_tags_str = NULL;
-   for (db_tags = notmuch_message_get_tags (message);
-notmuch_tags_valid (db_tags);
-notmuch_tags_move_to_next (db_tags))
-   {
-   const char *tag = notmuch_tags_get (db_tags);
-
-   if (db_tags_str)
-   db_tags_str = talloc_asprintf_append (db_tags_str,  %s, tag);
-   else
-   db_tags_str = talloc_strdup (message, tag);
-   }
-
-   if (((file_tags == NULL || *file_tags == '\0') 
-(db_tags_str == NULL || *db_tags_str == '\0')) ||
-   (file_tags  db_tags_str  strcmp (file_tags, db_tags_str) == 0))
-   {
-   goto NEXT_LINE;
-   }
-
-   notmuch_message_freeze (message);
-
-   if (!accumulate)
-   notmuch_message_remove_all_tags 

Re: [PATCH] vim: fix regex after notmuch show output change

2012-03-25 Thread Tomi Ollila
Jakob ja...@pipefour.org writes:

 On Sat, 24 Mar 2012 10:58:59 +0200, Tomi Ollila tomi.oll...@iki.fi wrote:
 Is this regexp part below good ?
 
  +... match:\([0-9]*\) excluded:\([[0-9]*\) filename:\(.*\)$', ...
 
 ( -- excluded:\([[0-9]*\) -- )

 Yeah, that was the core of the change.  With this new field in the
 output, the old regex didn't match at all.  You can see the fix working by
 trying to use the vim plugin as-is and trying to reply to a message or
 view a message's ID or any number of other things that fail right now.
 After updating this regex, everything starts working again.

I meant about the syntax: \([[0-9]*\) i.e. 2 opening [[:s


 Thanks,
 Jakob

Tomi
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v2 2/3] cli: refactor notmuch tag query tagging into a separate function

2012-03-25 Thread Tomi Ollila
Jani Nikula j...@nikula.org writes:

 Refactor to make tagging code easier to reuse in the future. No
 functional changes.

 Signed-off-by: Jani Nikula j...@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);
 - }
 -
 - 

Re: [PATCH v2 2/3] cli: refactor notmuch tag query tagging into a separate function

2012-03-25 Thread Jani Nikula
On Sun, 25 Mar 2012 23:45:39 +0300, Tomi Ollila tomi.oll...@iki.fi wrote:
 Jani Nikula j...@nikula.org writes:
 
  Refactor to make tagging code easier to reuse in the future. No
  functional changes.
 
  Signed-off-by: Jani Nikula j...@nikula.org
  ---
 
 tag_query() is pretty generic name for a function which (also) does
 tagging operations (adds and removes tags based on tag_ops).

Mmmh, if you think of tag as a verb, it fairly accurately describes
what the function does: tag (messages matching this) query (according
given arguments). I don't mind changing, but can't come up with anything
better right now either. notmuch_{search,query}_change_tags()? Meh?

 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'm not sure I follow you. AFAICT the behaviour does not change from
what it was before, apart from the tag addition and removal being mixed
together.

 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.

Documentation good. :)

 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] == '-');

I usually don't add braces where they aren't needed, but can do here.

 Everything else LGTM.

Thanks for the review,
Jani.


 
 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 

Re: [PATCH v2 2/3] cli: refactor notmuch tag query tagging into a separate function

2012-03-25 Thread Tomi Ollila
Jani Nikula j...@nikula.org writes:

 On Sun, 25 Mar 2012 23:45:39 +0300, Tomi Ollila tomi.oll...@iki.fi wrote:
 Jani Nikula j...@nikula.org writes:
 
  Refactor to make tagging code easier to reuse in the future. No
  functional changes.
 
  Signed-off-by: Jani Nikula j...@nikula.org
  ---
 
 tag_query() is pretty generic name for a function which (also) does
 tagging operations (adds and removes tags based on tag_ops).

 Mmmh, if you think of tag as a verb, it fairly accurately describes
 what the function does: tag (messages matching this) query (according
 given arguments). I don't mind changing, but can't come up with anything
 better right now either. notmuch_{search,query}_change_tags()? Meh?

Hmmm, yes... -- tag...query... ok, I can live with that if no-one
else get same impression I got first...


 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'm not sure I follow you. AFAICT the behaviour does not change from
 what it was before, apart from the tag addition and removal being mixed
 together.

The thing is that notmuch_tag_command () check that there is at least
one tagging operation to be done, but tag_query () doesn't do such
checking...

 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.

 Documentation good. :)

... therefore the requirement that there needs to tagging information
in tag_ops is in place.


 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] == '-');

 I usually don't add braces where they aren't needed, but can do here.

Assigment is much lower in precedence than equality comparison -- but
as this is so seldom-used construct, humans read (with understanding)
the code faster with this added clarity.

 Everything else LGTM.

 Thanks for the review,
 Jani.

 
 Tomi

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,