Re: [PATCH 0/2] Escape message ID queries in Emacs

2012-03-26 Thread Tomi Ollila
On Tue, Mar 27 2012, Austin Clements wrote:

> Currently, Emacs does not escape message ID queries and is
> inconsistent about quoting them.  This patch centralizes this in one
> function that always produces a properly quoted and escaped message ID
> query.
>
> With this, Emacs no longer gets confused by Tomi's crazy message,
>   id:"id:""1332281811-24710-2b-git-send-email-tomi.oll...@iki.fi"""

LGTM. Things work!


One observation, though:

In search bar the following queries return one match:

id:id:"1332281811-24710-2b-git-send-email-tomi.oll...@iki.fi"
id:"id:""1332281811-24710-2b-git-send-email-tomi.oll...@iki.fi"""

but

id:"id:"1332281811-24710-2b-git-send-email-tomi.oll...@iki.fi""

return 0 matches.

It looks like the (above) search strings goes verbatim to command line

i.e. 'notmuch' 'search' '--sort=oldest-first' 'id:...'

can be used to compare... so that would be CLI issue if there
is ever need to do do anything with it.

+1

Thanks for fixing this.

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


Re: [PATCH] emacs: Fix mis-named argument to notmuch-get-bodypart-internal

2012-03-26 Thread Tomi Ollila
On Tue, Mar 27 2012, Austin Clements  wrote:

> Previously, this function took an argument called "message-id", even
> though it was a general query, rather than a message ID.  This changes
> it to "query".
> ---

+1 

Tomi


>  emacs/notmuch-lib.el |4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index 2e367b5..4b17ecc 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -219,13 +219,13 @@ the given type."
>  
>  ;; Helper for parts which are generally not included in the default
>  ;; JSON output.
> -(defun notmuch-get-bodypart-internal (message-id part-number process-crypto)
> +(defun notmuch-get-bodypart-internal (query part-number process-crypto)
>(let ((args '("show" "--format=raw"))
>   (part-arg (format "--part=%s" part-number)))
>  (setq args (append args (list part-arg)))
>  (if process-crypto
>   (setq args (append args '("--decrypt"
> -(setq args (append args (list message-id)))
> +(setq args (append args (list query)))
>  (with-temp-buffer
>(let ((coding-system-for-read 'no-conversion))
>   (progn
> -- 
> 1.7.7.2
>
> ___
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Bug#628018: [PATCH] notmuch-mutt utility for notmuch/contrib/

2012-03-26 Thread Stefano Zacchiroli
On Mon, Mar 26, 2012 at 01:03:32PM -0600, Scott Barker wrote:
> FYI, I use the following in my .muttrc, which includes an expansion of $HOME
> in , and it works fine:
> 
>   macro index / "mutt-notmuch --prompt 
> search$HOME/.cache/mutt_results" 
> "notmuch search"

Right, but I need support for variable expansions with defaults, because
there's no guarantee that $XDG_CACHE_HOME is defined in user
environment. I.e., I need the equivalent of shell ${name:-default}
idiom.

AFAICT that is not supported by Mutt natively. Hence the need of
resorting to shell escaping.

Hope this explains,
-- 
Stefano Zacchiroli zack@{upsilon.cc,pps.jussieu.fr,debian.org} . o .
Ma?tre de conf?rences   ..   http://upsilon.cc/zack   ..   . . o
Debian Project Leader...   @zack on identi.ca   ...o o o
? the first rule of tautology club is the first rule of tautology club ?


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

2012-03-26 Thread Tomi Ollila
On Sun, Mar 25 2012, Mark Walters  wrote:

> 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.
> ---

+1

Tomi

>  emacs/notmuch-lib.el |5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
>


[PATCH] emacs: Fix mis-named argument to notmuch-get-bodypart-internal

2012-03-26 Thread Austin Clements
Previously, this function took an argument called "message-id", even
though it was a general query, rather than a message ID.  This changes
it to "query".
---
 emacs/notmuch-lib.el |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 2e367b5..4b17ecc 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -219,13 +219,13 @@ the given type."

 ;; Helper for parts which are generally not included in the default
 ;; JSON output.
-(defun notmuch-get-bodypart-internal (message-id part-number process-crypto)
+(defun notmuch-get-bodypart-internal (query part-number process-crypto)
   (let ((args '("show" "--format=raw"))
(part-arg (format "--part=%s" part-number)))
 (setq args (append args (list part-arg)))
 (if process-crypto
(setq args (append args '("--decrypt"
-(setq args (append args (list message-id)))
+(setq args (append args (list query)))
 (with-temp-buffer
   (let ((coding-system-for-read 'no-conversion))
(progn
-- 
1.7.7.2



[PATCH 2/2] emacs: Escape all message ID queries

2012-03-26 Thread Austin Clements
This adds a lib function to turn a message ID into a properly escaped
message ID query and uses this function wherever we previously
hand-constructed ID queries.  Wherever this new function is used,
documentation has been clarified to refer to "id: queries" instead of
"message IDs".

This fixes the broken test introduced by the previous patch.
---
 emacs/notmuch-lib.el |6 +-
 emacs/notmuch-message.el |2 +-
 emacs/notmuch-show.el|   14 +++---
 test/emacs   |1 -
 4 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index c146748..2e367b5 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -144,6 +144,10 @@ the user hasn't set this variable with the old or new 
value."
"[No Subject]"
   subject)))

+(defun notmuch-id-to-query (id)
+  "Return a query that matches the message with id ID."
+  (concat "id:\"" (replace-regexp-in-string "\"" "\"\"" id t t) "\""))
+
 ;;

 (defun notmuch-common-do-stash (text)
@@ -230,7 +234,7 @@ the given type."

 (defun notmuch-get-bodypart-content (msg part nth process-crypto)
   (or (plist-get part :content)
-  (notmuch-get-bodypart-internal (concat "id:" (plist-get msg :id)) nth 
process-crypto)))
+  (notmuch-get-bodypart-internal (notmuch-id-to-query (plist-get msg :id)) 
nth process-crypto)))

 (defun notmuch-plist-to-alist (plist)
   (loop for (key value . rest) on plist by #'cddr
diff --git a/emacs/notmuch-message.el b/emacs/notmuch-message.el
index 264a5b9..3010281 100644
--- a/emacs/notmuch-message.el
+++ b/emacs/notmuch-message.el
@@ -44,7 +44,7 @@ the \"inbox\" and \"todo\", you would set
(concat "+" str)
  str))
  notmuch-message-replied-tags)))
-   (apply 'notmuch-tag (concat "id:" (car (car rep))) tags)
+   (apply 'notmuch-tag (notmuch-id-to-query (car (car rep))) tags)

 (add-hook 'message-send-hook 'notmuch-message-mark-replied)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 0cd7d82..6d3fe62 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -613,7 +613,7 @@ current buffer, if possible."
  ;; times (hundreds!), which results in many calls to
  ;; `notmuch part'.
  (unless content
-   (setq content (notmuch-get-bodypart-internal (concat "id:" 
message-id)
+   (setq content (notmuch-get-bodypart-internal (notmuch-id-to-query 
message-id)
  part-number 
notmuch-show-process-crypto))
(with-current-buffer w3m-current-buffer
  (notmuch-show-w3m-cid-store-internal url
@@ -1325,16 +1325,16 @@ Some useful entries are:
 (plist-get props prop)))

 (defun notmuch-show-get-message-id (&optional bare)
-  "Return the Message-Id of the current message.
+  "Return an id: query for the Message-Id of the current message.

 If optional argument BARE is non-nil, return
-the Message-Id without prefix and quotes."
+the Message-Id without id: prefix and escaping."
   (if bare
   (notmuch-show-get-prop :id)
-(concat "id:\"" (notmuch-show-get-prop :id) "\"")))
+(notmuch-id-to-query (notmuch-show-get-prop :id

 (defun notmuch-show-get-messages-ids ()
-  "Return all message ids of messages in the current thread."
+  "Return all id: queries of messages in the current thread."
   (let ((message-ids))
 (notmuch-show-mapc
  (lambda () (push (notmuch-show-get-message-id) message-ids)))
@@ -1401,7 +1401,7 @@ current thread."
 ;; thread.

 (defun notmuch-show-get-message-ids-for-open-messages ()
-  "Return a list of all message IDs for open messages in the current thread."
+  "Return a list of all id: queries for open messages in the current thread."
   (save-excursion
 (let (message-ids done)
   (goto-char (point-min))
@@ -1805,7 +1805,7 @@ thread from search."
   (notmuch-common-do-stash (notmuch-show-get-from)))

 (defun notmuch-show-stash-message-id ()
-  "Copy message ID of current message to kill-ring."
+  "Copy id: query matching the current message to kill-ring."
   (interactive)
   (notmuch-common-do-stash (notmuch-show-get-message-id)))

diff --git a/test/emacs b/test/emacs
index 62eaedb..8b92d0a 100755
--- a/test/emacs
+++ b/test/emacs
@@ -140,7 +140,6 @@ output=$(notmuch search 'id:"123..456 at example"' | 
notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; 
Message with .. in Message-Id (inbox search-add show-add)"

 test_begin_subtest "Message with quote in Message-Id:"
-test_subtest_known_broken
 add_message '[id]="\"quote\"@example"' '[subject]="Message with quote in 
Message-Id"'
 test_emacs '(notmuch-search "subject:\"Message with quote\"")
(notmuch-test-wait)
-- 
1.7.7.2



[PATCH 1/2] test: Add Emacs test for messages with quotes in their message ID

2012-03-26 Thread Austin Clements
Currently this is broken because Emacs doesn't properly escape double
quotes in message IDs.
---
 test/emacs |   12 
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/test/emacs b/test/emacs
index 8a28705..62eaedb 100755
--- a/test/emacs
+++ b/test/emacs
@@ -139,6 +139,18 @@ test_emacs '(notmuch-search "id:\"123..456 at example\"")
 output=$(notmuch search 'id:"123..456 at example"' | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; 
Message with .. in Message-Id (inbox search-add show-add)"

+test_begin_subtest "Message with quote in Message-Id:"
+test_subtest_known_broken
+add_message '[id]="\"quote\"@example"' '[subject]="Message with quote in 
Message-Id"'
+test_emacs '(notmuch-search "subject:\"Message with quote\"")
+   (notmuch-test-wait)
+   (execute-kbd-macro "+search-add")
+(notmuch-search-show-thread)
+   (notmuch-test-wait)
+   (execute-kbd-macro "+show-add")'
+output=$(notmuch search 'id:"""quote""@example"' | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; 
Message with quote in Message-Id (inbox search-add show-add)"
+
 test_begin_subtest "Sending a message via (fake) SMTP"
 emacs_deliver_message \
 'Testing message sent via SMTP' \
-- 
1.7.7.2



[PATCH 0/2] Escape message ID queries in Emacs

2012-03-26 Thread Austin Clements
Currently, Emacs does not escape message ID queries and is
inconsistent about quoting them.  This patch centralizes this in one
function that always produces a properly quoted and escaped message ID
query.

With this, Emacs no longer gets confused by Tomi's crazy message,
  id:"id:""1332281811-24710-2b-git-send-email-tomi.ollila at iki.fi"""



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

2012-03-26 Thread Dmitry Kurochkin
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.
> ---

LGTM.  Thanks for fixing this.

Regards,
  Dmitry

>  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 "' \
>[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


[BUG/PATCH 1/2] test: Tests for reply from alternate addresses in emacs

2012-03-26 Thread Dmitry Kurochkin
Adam Wolfe Gordon  writes:
> Since the recent reply changes were pushed, there has been a bug that
> causes emacs to always reply from the primary address, even if the
> JSON or default CLI reply output uses an alternate address.
>
> This adds two tests to the emacs test library based on the two "Reply
> form..." tests in the reply test library. One is currently marked
> broken.
> ---
>  test/emacs |   52 
>  1 files changed, 52 insertions(+), 0 deletions(-)
>
> diff --git a/test/emacs b/test/emacs
> index 8a28705..fa5d706 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -274,6 +274,58 @@ Notmuch Test Suite  
> writes:
>  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 \
> + '[date]="Tue, 05 Jan 2010 15:43:56 -"' \
> + '[body]="reply from alternate address"'

Please remove subject, data and body parameters.  add_message would
provide sane default values.

> +
> +test_emacs "(notmuch-search \"id:\\\"${gen_msg_id}\\\"\")
> + (notmuch-test-wait)
> + (notmuch-search-reply-to-thread)
> + (test-output)"
> +sed -i -e 's/^In-Reply-To: <.*>$/In-Reply-To: /' OUTPUT
> +cat  +From: Notmuch Test Suite 
> +To: Sender 
> +Subject: Re: notmuch-reply-test
> +In-Reply-To: 
> +Fcc: ${MAIL_DIR}/sent
> +--text follows this line--
> +Sender  writes:
> +
> +> reply from alternate address
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED
> +
> +test_begin_subtest "Reply from address in named group list within emacs"
> +add_message '[from]="Sender "' \
> +'[to]=group:test_suite at notmuchmail.org,someone at 
> example.com\;' \
> + [cc]=test_suite_other at notmuchmail.org \
> + [subject]=notmuch-reply-test \
> +'[date]="Tue, 05 Jan 2010 15:43:56 -"' \
> +'[body]="Reply from address in named group list"'

Same here.

Regards,
  Dmitry

> +
> +test_emacs "(notmuch-search \"id:\\\"${gen_msg_id}\\\"\")
> + (notmuch-test-wait)
> + (notmuch-search-reply-to-thread)
> + (test-output)"
> +sed -i -e 's/^In-Reply-To: <.*>$/In-Reply-To: /' OUTPUT
> +cat  +From: Notmuch Test Suite 
> +To: Sender , someone at example.com
> +Subject: Re: notmuch-reply-test
> +In-Reply-To: 
> +Fcc: ${MAIL_DIR}/sent
> +--text follows this line--
> +Sender  writes:
> +
> +> Reply from address in named group list
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED
> +
>  test_begin_subtest "Reply within emacs to a multipart/mixed message"
>  test_emacs '(notmuch-show "id:20091118002059.067214ed at hikari")
>   (notmuch-show-reply)
> -- 
> 1.7.5.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


Bug#628018: [PATCH] notmuch-mutt utility for notmuch/contrib/

2012-03-26 Thread Stefano Zacchiroli
On Mon, Mar 26, 2012 at 05:29:02PM +0200, Stefano Zacchiroli wrote:
> But while it's trivial to make notmuch-mutt itself support
> $XDG_CACHE_HOME, it is less so for the Mutt configuration snippet
> (i.e. the notmuch-mutt.rc file which is part of my submission). AFAICT
> the  function does not support variable
> expansions, not to mention default values while doing so.

... but Mutt configuration has backtick expansions! So never mind the
above.

The attached patch should be all that's needed to proper $XDG_CACHE_HOME
support, according to XDG basedir spec.

If you folks now consider the contrib submission good enough for
inclusion, feel free to just "git am" the 3 patches. If you want me to
implement other changes --- including bureaucratic stuff like merging
patches together --- just let me know!

Cheers.
-- 
Stefano Zacchiroli zack@{upsilon.cc,pps.jussieu.fr,debian.org} . o .
Ma?tre de conf?rences   ..   http://upsilon.cc/zack   ..   . . o
Debian Project Leader...   @zack on identi.ca   ...o o o
? the first rule of tautology club is the first rule of tautology club ?
-- next part --
A non-text attachment was scrubbed...
Name: 0003-notmuch-mutt-XDG_CACHE_HOME-support-as-per-XDG-based.patch
Type: text/x-diff
Size: 2207 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20120326/9764c45f/attachment-0001.patch>
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 828 bytes
Desc: Digital signature
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20120326/9764c45f/attachment-0001.pgp>


[PATCH] emacs: Fix mis-named argument to notmuch-get-bodypart-internal

2012-03-26 Thread Austin Clements
Previously, this function took an argument called "message-id", even
though it was a general query, rather than a message ID.  This changes
it to "query".
---
 emacs/notmuch-lib.el |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 2e367b5..4b17ecc 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -219,13 +219,13 @@ the given type."
 
 ;; Helper for parts which are generally not included in the default
 ;; JSON output.
-(defun notmuch-get-bodypart-internal (message-id part-number process-crypto)
+(defun notmuch-get-bodypart-internal (query part-number process-crypto)
   (let ((args '("show" "--format=raw"))
(part-arg (format "--part=%s" part-number)))
 (setq args (append args (list part-arg)))
 (if process-crypto
(setq args (append args '("--decrypt"
-(setq args (append args (list message-id)))
+(setq args (append args (list query)))
 (with-temp-buffer
   (let ((coding-system-for-read 'no-conversion))
(progn
-- 
1.7.7.2

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


[PATCH 2/2] emacs: Escape all message ID queries

2012-03-26 Thread Austin Clements
This adds a lib function to turn a message ID into a properly escaped
message ID query and uses this function wherever we previously
hand-constructed ID queries.  Wherever this new function is used,
documentation has been clarified to refer to "id: queries" instead of
"message IDs".

This fixes the broken test introduced by the previous patch.
---
 emacs/notmuch-lib.el |6 +-
 emacs/notmuch-message.el |2 +-
 emacs/notmuch-show.el|   14 +++---
 test/emacs   |1 -
 4 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index c146748..2e367b5 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -144,6 +144,10 @@ the user hasn't set this variable with the old or new 
value."
"[No Subject]"
   subject)))
 
+(defun notmuch-id-to-query (id)
+  "Return a query that matches the message with id ID."
+  (concat "id:\"" (replace-regexp-in-string "\"" "\"\"" id t t) "\""))
+
 ;;
 
 (defun notmuch-common-do-stash (text)
@@ -230,7 +234,7 @@ the given type."
 
 (defun notmuch-get-bodypart-content (msg part nth process-crypto)
   (or (plist-get part :content)
-  (notmuch-get-bodypart-internal (concat "id:" (plist-get msg :id)) nth 
process-crypto)))
+  (notmuch-get-bodypart-internal (notmuch-id-to-query (plist-get msg :id)) 
nth process-crypto)))
 
 (defun notmuch-plist-to-alist (plist)
   (loop for (key value . rest) on plist by #'cddr
diff --git a/emacs/notmuch-message.el b/emacs/notmuch-message.el
index 264a5b9..3010281 100644
--- a/emacs/notmuch-message.el
+++ b/emacs/notmuch-message.el
@@ -44,7 +44,7 @@ the \"inbox\" and \"todo\", you would set
(concat "+" str)
  str))
  notmuch-message-replied-tags)))
-   (apply 'notmuch-tag (concat "id:" (car (car rep))) tags)
+   (apply 'notmuch-tag (notmuch-id-to-query (car (car rep))) tags)
 
 (add-hook 'message-send-hook 'notmuch-message-mark-replied)
 
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 0cd7d82..6d3fe62 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -613,7 +613,7 @@ current buffer, if possible."
  ;; times (hundreds!), which results in many calls to
  ;; `notmuch part'.
  (unless content
-   (setq content (notmuch-get-bodypart-internal (concat "id:" 
message-id)
+   (setq content (notmuch-get-bodypart-internal (notmuch-id-to-query 
message-id)
  part-number 
notmuch-show-process-crypto))
(with-current-buffer w3m-current-buffer
  (notmuch-show-w3m-cid-store-internal url
@@ -1325,16 +1325,16 @@ Some useful entries are:
 (plist-get props prop)))
 
 (defun notmuch-show-get-message-id (&optional bare)
-  "Return the Message-Id of the current message.
+  "Return an id: query for the Message-Id of the current message.
 
 If optional argument BARE is non-nil, return
-the Message-Id without prefix and quotes."
+the Message-Id without id: prefix and escaping."
   (if bare
   (notmuch-show-get-prop :id)
-(concat "id:\"" (notmuch-show-get-prop :id) "\"")))
+(notmuch-id-to-query (notmuch-show-get-prop :id
 
 (defun notmuch-show-get-messages-ids ()
-  "Return all message ids of messages in the current thread."
+  "Return all id: queries of messages in the current thread."
   (let ((message-ids))
 (notmuch-show-mapc
  (lambda () (push (notmuch-show-get-message-id) message-ids)))
@@ -1401,7 +1401,7 @@ current thread."
 ;; thread.
 
 (defun notmuch-show-get-message-ids-for-open-messages ()
-  "Return a list of all message IDs for open messages in the current thread."
+  "Return a list of all id: queries for open messages in the current thread."
   (save-excursion
 (let (message-ids done)
   (goto-char (point-min))
@@ -1805,7 +1805,7 @@ thread from search."
   (notmuch-common-do-stash (notmuch-show-get-from)))
 
 (defun notmuch-show-stash-message-id ()
-  "Copy message ID of current message to kill-ring."
+  "Copy id: query matching the current message to kill-ring."
   (interactive)
   (notmuch-common-do-stash (notmuch-show-get-message-id)))
 
diff --git a/test/emacs b/test/emacs
index 62eaedb..8b92d0a 100755
--- a/test/emacs
+++ b/test/emacs
@@ -140,7 +140,6 @@ output=$(notmuch search 'id:"123..456@example"' | 
notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; 
Message with .. in Message-Id (inbox search-add show-add)"
 
 test_begin_subtest "Message with quote in Message-Id:"
-test_subtest_known_broken
 add_message '[id]="\"quote\"@example"' '[subject]="Message with quote in 
Message-Id"'
 test_emacs '(notmuch-search "subject:\"Message with quote\"")
(notmuch-test-wait)
-- 
1.7.7.2

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mai

[PATCH 1/2] test: Add Emacs test for messages with quotes in their message ID

2012-03-26 Thread Austin Clements
Currently this is broken because Emacs doesn't properly escape double
quotes in message IDs.
---
 test/emacs |   12 
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/test/emacs b/test/emacs
index 8a28705..62eaedb 100755
--- a/test/emacs
+++ b/test/emacs
@@ -139,6 +139,18 @@ test_emacs '(notmuch-search "id:\"123..456@example\"")
 output=$(notmuch search 'id:"123..456@example"' | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; 
Message with .. in Message-Id (inbox search-add show-add)"
 
+test_begin_subtest "Message with quote in Message-Id:"
+test_subtest_known_broken
+add_message '[id]="\"quote\"@example"' '[subject]="Message with quote in 
Message-Id"'
+test_emacs '(notmuch-search "subject:\"Message with quote\"")
+   (notmuch-test-wait)
+   (execute-kbd-macro "+search-add")
+(notmuch-search-show-thread)
+   (notmuch-test-wait)
+   (execute-kbd-macro "+show-add")'
+output=$(notmuch search 'id:"""quote""@example"' | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; 
Message with quote in Message-Id (inbox search-add show-add)"
+
 test_begin_subtest "Sending a message via (fake) SMTP"
 emacs_deliver_message \
 'Testing message sent via SMTP' \
-- 
1.7.7.2

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


[PATCH 0/2] Escape message ID queries in Emacs

2012-03-26 Thread Austin Clements
Currently, Emacs does not escape message ID queries and is
inconsistent about quoting them.  This patch centralizes this in one
function that always produces a properly quoted and escaped message ID
query.

With this, Emacs no longer gets confused by Tomi's crazy message,
  id:"id:""1332281811-24710-2b-git-send-email-tomi.oll...@iki.fi"""

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


Bug#628018: [PATCH] notmuch-mutt utility for notmuch/contrib/

2012-03-26 Thread Stefano Zacchiroli
On Mon, Mar 26, 2012 at 08:09:13AM -0700, Jameson Graef Rollins wrote:
> > Another thing is this .cache hardcoding. Should this be resolved
> > first by using $XDG_CACHE_HOME, (then *MAYBE* $XDG_CONFIG_HOME/.cache)
> > and finally $HOME/.cache
> 
> These are good points.  I agree that since ~/.cache is an XDG standard
> we should resolve the appropriate environment variables.

Heya, thanks for your feedback. I agree as well!

But while it's trivial to make notmuch-mutt itself support
$XDG_CACHE_HOME, it is less so for the Mutt configuration snippet
(i.e. the notmuch-mutt.rc file which is part of my submission). AFAICT
the  function does not support variable
expansions, not to mention default values while doing so. As a result, I
can easily support $XDG_CACHE_HOME for, say, the history file, but I
don't know how to make mutt look in some $XDG_CACHE_HOME derived
directory. (Yes, I'm excluding hackish solutions like have mutt look
always in the same dir, and make that dir a "moving" symlink that will
be changed by notmuch-mutt upon execution. That seems to defeat the
benefits of the XDG specification, at least partly.)

Bottom line: I'll be happy to properly support $XDG_CACHE_HOME, but I'm
in need on suggestions of how to do so for the Mutt configuration part.

> If notmuch-mutt is accepted into notmuch upstream then it really becomes
> a part of notmuch and isn't really "public" anymore (at least in the
> sense that you're using).  So I don't really see any problem with having
> it use ~/.cache/notmuch.

My thought exactly.

The proposal of documenting how people should use ~/.cache/notmuch is
great, but I was hoping it can be overlooked for contrib stuff that has
been vetted by you folks.

Thanks for your help (and for notmuch!)
Cheers.
-- 
Stefano Zacchiroli zack@{upsilon.cc,pps.jussieu.fr,debian.org} . o .
Ma?tre de conf?rences   ..   http://upsilon.cc/zack   ..   . . o
Debian Project Leader...   @zack on identi.ca   ...o o o
? the first rule of tautology club is the first rule of tautology club ?


[Stefano Zacchiroli] Bug#628018: [PATCH] notmuch-mutt utility for notmuch/contrib/

2012-03-26 Thread Tomi Ollila

Interesting, 'R' in emacs gives empty content. I might have Austin's 
reply change in this version -- although I suspect the problem is not
there. Everyone using emacs MUA with latest notmuch(1)es experiment
pressing 'R' while reading the mail I'm replying to

Pasting content...

> [ multipart/mixed ]
> [ message/rfc822 ]
> From: Stefano Zacchiroli 
> Subject: Bug#628018: [PATCH] notmuch-mutt utility for notmuch/contrib/
> To: 628018 at bugs.debian.org
> Date: Mon, 26 Mar 2012 11:36:06 +0200
>
> [ multipart/signed ]
> [ Good signature by key: 0x4900707DDC5C07F2DECB02839C31503C6D866396 ]
> [ multipart/mixed ]
> [ text/plain ]
> Here is an updated patch set that ships the notmuch-mutt utility and
> updated the Debian packaging for it. It is now better integrated with
> notmuch: it has been renamed to "notmuch-mutt" (to match the naming
> convention of other notmuch interfaces) and stores all its data under
> ~/.cache/notmuch/mutt/ to avoid polluting user $HOME with other
> directories. I've also fixed the Debian packaging to generate the
> manpage during build.

I think storing stuff to  .../.cache/... is Great !!!

However, if we're "allowing" public software to use .../.cache/notmuch
directory we should have policy to do so. I suggest we add a description
file to notmuch source distribution which describes the public software
that are allowed to contain files in notmuch XDG directories. The file
could have format which describes the file/directory name, the software
using it and some description why this file is there.

Another thing is this .cache hardcoding. Should this be resolved
first by using $XDG_CACHE_HOME, (then *MAYBE* $XDG_CONFIG_HOME/.cache)
and finally $HOME/.cache

Tomi


>
> The first patch in the set adds notmuch-mutt to contrib/, the second
> updates debian/ to build a new "notmuch-mutt" binary package.
>
> The full story is available at http://bugs.debian.org/628018
>
> David: I'm still unable to get through the moderation queue of the
> notmuch mailing list. Would you be so kind to forward this mail there,
> for patch review?
>
> TIA,
> Cheers.


Re: Bug#628018: [PATCH] notmuch-mutt utility for notmuch/contrib/

2012-03-26 Thread Stefano Zacchiroli
On Mon, Mar 26, 2012 at 01:03:32PM -0600, Scott Barker wrote:
> FYI, I use the following in my .muttrc, which includes an expansion of $HOME
> in , and it works fine:
> 
>   macro index / "mutt-notmuch --prompt 
> search$HOME/.cache/mutt_results" 
> "notmuch search"

Right, but I need support for variable expansions with defaults, because
there's no guarantee that $XDG_CACHE_HOME is defined in user
environment. I.e., I need the equivalent of shell ${name:-default}
idiom.

AFAICT that is not supported by Mutt natively. Hence the need of
resorting to shell escaping.

Hope this explains,
-- 
Stefano Zacchiroli zack@{upsilon.cc,pps.jussieu.fr,debian.org} . o .
Maître de conférences   ..   http://upsilon.cc/zack   ..   . . o
Debian Project Leader...   @zack on identi.ca   ...o o o
« the first rule of tautology club is the first rule of tautology club »
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


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

2012-03-26 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..d3b9246 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -20,6 +20,81 @@
 
 #include "notmuch-client.h"
 
+static int
+tag_message (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[])
 {
@@ -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 (message)

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

2012-03-26 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 |  104 +
 1 files changed, 60 insertions(+), 44 deletions(-)

diff --git a/notmuch-tag.c b/notmuch-tag.c
index 0a6b140..05feed3 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -110,6 +110,63 @@ _optimize_tag_query (void *ctx, const char 
*orig_query_string,
 return query_string;
 }
 
+/* Tag messages matching 'query_string' according to 'tag_ops', which
+ * must be an array of tagging operations terminated with an empty
+ * element. */
+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[])
 {
@@ -118,12 +175,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));
@@ -170,14 +225,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;
@@ -189,40 +236,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 v3 2/4] cli: refactor "notmuch tag" data structures for tagging operations

2012-03-26 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. Change the relevant test to reflect the new
behaviour.

Signed-off-by: Jani Nikula 
---
 notmuch-tag.c |   83 ++--
 test/tagging  |2 +-
 2 files changed, 40 insertions(+), 45 deletions(-)

diff --git a/notmuch-tag.c b/notmuch-tag.c
index 36b9b09..0a6b140 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,
@@ -71,15 +75,16 @@ _optimize_tag_query (void *ctx, const char 
*orig_query_string, char *argv[],
 int i;
 unsigned int max_tag_len = 0;
 
+/* Don't optimize if there are no tag changes. */
+if (tag_ops[0].tag == NULL)
+   return talloc_strdup (ctx, orig_query_string);
+
 /* 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 +95,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++) {
+for (i = 0; tag_ops[i].tag && 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++) {
-   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 +113,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 +132,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 

[PATCH v3 1/4] test: add test for both adding and removing a tag at the same time

2012-03-26 Thread Jani Nikula
The current behaviour is that regardless of the order in which the
addition and removal of a tag are specified, the tag is added.

Signed-off-by: Jani Nikula 
---
 test/tagging |8 
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/test/tagging b/test/tagging
index 77202bf..3acf1bc 100755
--- a/test/tagging
+++ b/test/tagging
@@ -38,4 +38,12 @@ test_expect_equal "$output" "\
 thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (:\"  inbox tag1 unread)
 thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 unread)"
 
+test_begin_subtest "Tagging order"
+notmuch tag +tag4 -tag4 One
+notmuch tag -tag4 +tag4 Two
+output=$(notmuch search \* | notmuch_search_sanitize)
+test_expect_equal "$output" "\
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (:\"  inbox tag1 tag4 
unread)
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 tag4 unread)"
+
 test_done
-- 
1.7.5.4

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


[PATCH v3 0/4] cli: notmuch tag/restore refactoring

2012-03-26 Thread Jani Nikula
v3 of id:"cover.1332702915.git.j...@nikula.org" with the following
mostly non-functional changes:

 - add test for the current tagging behaviour in patch 1, and change the
   test in patch 2 when the behaviour is changed
 - handle the no tag changes case in _optimize_tag_query() in patch 2
 - add braces around the "tag_ops[tag_ops_count].remove = (argv[i][0] == '-');"
   assignment
 - document the tag_query() function a bit in patch 3
 - make tag_message() static in patch 4

BR,
Jani.


Jani Nikula (4):
  test: add test for both adding and removing a tag at the same time
  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 |  173 -
 test/tagging  |8 +++
 3 files changed, 178 insertions(+), 151 deletions(-)

-- 
1.7.5.4

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


Bug#628018: [PATCH] notmuch-mutt utility for notmuch/contrib/

2012-03-26 Thread Scott Barker
FYI, I use the following in my .muttrc, which includes an expansion of $HOME
in , and it works fine:

  macro index / "mutt-notmuch --prompt 
search$HOME/.cache/mutt_results" "notmuch 
search"

On Mon, Mar 26, 2012 at 05:29:02PM +0200, Stefano Zacchiroli wrote:
> On Mon, Mar 26, 2012 at 08:09:13AM -0700, Jameson Graef Rollins wrote:
> > > Another thing is this .cache hardcoding. Should this be resolved
> > > first by using $XDG_CACHE_HOME, (then *MAYBE* $XDG_CONFIG_HOME/.cache)
> > > and finally $HOME/.cache
> > 
> > These are good points.  I agree that since ~/.cache is an XDG standard
> > we should resolve the appropriate environment variables.
> 
> Heya, thanks for your feedback. I agree as well!
> 
> But while it's trivial to make notmuch-mutt itself support
> $XDG_CACHE_HOME, it is less so for the Mutt configuration snippet
> (i.e. the notmuch-mutt.rc file which is part of my submission). AFAICT
> the  function does not support variable
> expansions, not to mention default values while doing so. As a result, I
> can easily support $XDG_CACHE_HOME for, say, the history file, but I
> don't know how to make mutt look in some $XDG_CACHE_HOME derived
> directory. (Yes, I'm excluding hackish solutions like have mutt look
> always in the same dir, and make that dir a "moving" symlink that will
> be changed by notmuch-mutt upon execution. That seems to defeat the
> benefits of the XDG specification, at least partly.)
> 
> Bottom line: I'll be happy to properly support $XDG_CACHE_HOME, but I'm
> in need on suggestions of how to do so for the Mutt configuration part.
> 
> > If notmuch-mutt is accepted into notmuch upstream then it really becomes
> > a part of notmuch and isn't really "public" anymore (at least in the
> > sense that you're using).  So I don't really see any problem with having
> > it use ~/.cache/notmuch.
> 
> My thought exactly.
> 
> The proposal of documenting how people should use ~/.cache/notmuch is
> great, but I was hoping it can be overlooked for contrib stuff that has
> been vetted by you folks.
> 
> Thanks for your help (and for notmuch!)
> Cheers.
> -- 
> Stefano Zacchiroli zack@{upsilon.cc,pps.jussieu.fr,debian.org} . o .
> Ma?tre de conf?rences   ..   http://upsilon.cc/zack   ..   . . o
> Debian Project Leader...   @zack on identi.ca   ...o o o
> ? the first rule of tautology club is the first rule of tautology club ?
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

-- 
Scott Barker
Linux Consultant
scott at mostlylinux.ca
http://www.mostlylinux.ca


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

2012-03-26 Thread Tomi Ollila
On Sun, Mar 25 2012, Mark Walters  wrote:

> 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.
> ---

+1

Tomi

>  emacs/notmuch-lib.el |5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
>
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Unofficial notmuch wiki concerns

2012-03-26 Thread Kyle Sexton
All,

I wanted to get everyone's thoughts on something I setup over this past
weekend.  I put up a site at http://notmuchwiki.org with notes I took
while getting started with notmuch[1], along with a skeleton for more
information that I'll be updating as I learn. 

There is a disclaimer on the front page saying:

"Please refer to the official page for more information.  This wiki is
not endorsed, sponsored, or recommended by the official notmuch
community."

My goal is to make a site where the documentation is easy to update,
and attractive.  My concern is having too many places out there for
information.  I tried to assuage that with the disclaimer pointing to
the canonical site.  Does the community think there is space for an
unofficial wiki like this?

[1]: Most of the stuff I have written so far is under 'user setups' at
 http://www.notmuchwiki.org/display/usersetup/mocker

-- 
Kyle Sexton


Re: Bug#628018: [PATCH] notmuch-mutt utility for notmuch/contrib/

2012-03-26 Thread Scott Barker
FYI, I use the following in my .muttrc, which includes an expansion of $HOME
in , and it works fine:

  macro index / "mutt-notmuch --prompt 
search$HOME/.cache/mutt_results" "notmuch 
search"

On Mon, Mar 26, 2012 at 05:29:02PM +0200, Stefano Zacchiroli wrote:
> On Mon, Mar 26, 2012 at 08:09:13AM -0700, Jameson Graef Rollins wrote:
> > > Another thing is this .cache hardcoding. Should this be resolved
> > > first by using $XDG_CACHE_HOME, (then *MAYBE* $XDG_CONFIG_HOME/.cache)
> > > and finally $HOME/.cache
> > 
> > These are good points.  I agree that since ~/.cache is an XDG standard
> > we should resolve the appropriate environment variables.
> 
> Heya, thanks for your feedback. I agree as well!
> 
> But while it's trivial to make notmuch-mutt itself support
> $XDG_CACHE_HOME, it is less so for the Mutt configuration snippet
> (i.e. the notmuch-mutt.rc file which is part of my submission). AFAICT
> the  function does not support variable
> expansions, not to mention default values while doing so. As a result, I
> can easily support $XDG_CACHE_HOME for, say, the history file, but I
> don't know how to make mutt look in some $XDG_CACHE_HOME derived
> directory. (Yes, I'm excluding hackish solutions like have mutt look
> always in the same dir, and make that dir a "moving" symlink that will
> be changed by notmuch-mutt upon execution. That seems to defeat the
> benefits of the XDG specification, at least partly.)
> 
> Bottom line: I'll be happy to properly support $XDG_CACHE_HOME, but I'm
> in need on suggestions of how to do so for the Mutt configuration part.
> 
> > If notmuch-mutt is accepted into notmuch upstream then it really becomes
> > a part of notmuch and isn't really "public" anymore (at least in the
> > sense that you're using).  So I don't really see any problem with having
> > it use ~/.cache/notmuch.
> 
> My thought exactly.
> 
> The proposal of documenting how people should use ~/.cache/notmuch is
> great, but I was hoping it can be overlooked for contrib stuff that has
> been vetted by you folks.
> 
> Thanks for your help (and for notmuch!)
> Cheers.
> -- 
> Stefano Zacchiroli zack@{upsilon.cc,pps.jussieu.fr,debian.org} . o .
> Maître de conférences   ..   http://upsilon.cc/zack   ..   . . o
> Debian Project Leader...   @zack on identi.ca   ...o o o
> « the first rule of tautology club is the first rule of tautology club »
> ___
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

-- 
Scott Barker
Linux Consultant
sc...@mostlylinux.ca
http://www.mostlylinux.ca
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: Bug#628018: [PATCH] notmuch-mutt utility for notmuch/contrib/

2012-03-26 Thread Stefano Zacchiroli
On Mon, Mar 26, 2012 at 05:29:02PM +0200, Stefano Zacchiroli wrote:
> But while it's trivial to make notmuch-mutt itself support
> $XDG_CACHE_HOME, it is less so for the Mutt configuration snippet
> (i.e. the notmuch-mutt.rc file which is part of my submission). AFAICT
> the  function does not support variable
> expansions, not to mention default values while doing so.

... but Mutt configuration has backtick expansions! So never mind the
above.

The attached patch should be all that's needed to proper $XDG_CACHE_HOME
support, according to XDG basedir spec.

If you folks now consider the contrib submission good enough for
inclusion, feel free to just "git am" the 3 patches. If you want me to
implement other changes --- including bureaucratic stuff like merging
patches together --- just let me know!

Cheers.
-- 
Stefano Zacchiroli zack@{upsilon.cc,pps.jussieu.fr,debian.org} . o .
Maître de conférences   ..   http://upsilon.cc/zack   ..   . . o
Debian Project Leader...   @zack on identi.ca   ...o o o
« the first rule of tautology club is the first rule of tautology club »
From f68f36448f5bc63211c12b9e53e9db603ed76178 Mon Sep 17 00:00:00 2001
From: Stefano Zacchiroli 
Date: Mon, 26 Mar 2012 20:47:34 +0200
Subject: [PATCH] notmuch-mutt: $XDG_CACHE_HOME support as per XDG basedir
 spec

---
 contrib/notmuch-mutt/notmuch-mutt|4 +++-
 contrib/notmuch-mutt/notmuch-mutt.rc |4 ++--
 2 files changed, 5 insertions(+), 3 deletions(-)
 mode change 100755 => 100644 contrib/notmuch-mutt/notmuch-mutt

diff --git a/contrib/notmuch-mutt/notmuch-mutt b/contrib/notmuch-mutt/notmuch-mutt
old mode 100755
new mode 100644
index 29674ec..9cd01ec
--- a/contrib/notmuch-mutt/notmuch-mutt
+++ b/contrib/notmuch-mutt/notmuch-mutt
@@ -20,7 +20,9 @@ use String::ShellQuote;
 use Term::ReadLine;
 
 
-my $cache_dir = "$ENV{HOME}/.cache/notmuch/mutt";
+my $xdg_cache_dir = "$ENV{HOME}/.cache";
+$xdg_cache_dir = $ENV{XDG_CACHE_HOME} if $ENV{XDG_CACHE_HOME};
+my $cache_dir = "$xdg_cache_dir/notmuch/mutt";
 
 
 # create an empty maildir (if missing) or empty an existing maildir"
diff --git a/contrib/notmuch-mutt/notmuch-mutt.rc b/contrib/notmuch-mutt/notmuch-mutt.rc
index 269f03f..c0ff000 100644
--- a/contrib/notmuch-mutt/notmuch-mutt.rc
+++ b/contrib/notmuch-mutt/notmuch-mutt.rc
@@ -1,8 +1,8 @@
 macro index  \
-  "unset wait_keynotmuch-mutt --prompt search~/.cache/notmuch/mutt/results" \
+  "unset wait_keynotmuch-mutt --prompt search`echo ${XDG_CACHE_HOME:-$HOME/.cache}/notmuch/mutt/results`" \
   "notmuch: search mail"
 macro index  \
-  "unset wait_keynotmuch-mutt thread~/.cache/notmuch/mutt/resultsset wait_key" \
+  "unset wait_keynotmuch-mutt thread`echo ${XDG_CACHE_HOME:-$HOME/.cache}/notmuch/mutt/results`set wait_key" \
   "notmuch: reconstruct thread"
 macro index  \
   "unset wait_keynotmuch-mutt tag -inbox" \
-- 
1.7.9.1



signature.asc
Description: Digital signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


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

2012-03-26 Thread David Bremner
On Sat, 24 Mar 2012 18:14:35 +0200, Jani Nikula  wrote:
> + if (argv[i][0] == '+' || argv[i][0] == '-') {
> + tag_ops[tag_ops_count++] = (tag_operation_t) {
> + .tag = argv[i] + 1,
> + .remove = argv[i][0] == '-',
> + };

I'm not sure if this is a worthwhile use of a C99. Wouldn't it be
simpler to just use two assignments? and maybe increment the index
after? Still 3 lines of code.

Other than that, this patch looked ok to me.  I think it probably
deserves a NEWS patch that the ordering behaviour changed. I do think
the new order is more sensible, and the old one was never documented.


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


Reply code with TEXT/PLAIN

2012-03-26 Thread Daniel Kahn Gillmor
On 03/26/2012 11:21 AM, Jameson Graef Rollins wrote:
> Does it make sense for the cli to canonicalize the content types to
> always be lower case?  Or should it continue to just pass the content
> type exactly as it appears in the original message?  Given that
> consumers should parse it case insensitively, it maybe doesn't matter,
> but it might also be nice for us to be consistent.

i think "notmuch emits the content-type in the exact form contained in
the underlying message" is a good flavor of consistency.

--dkg


-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 1030 bytes
Desc: OpenPGP digital signature
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20120326/1a994cdb/attachment.pgp>


Unofficial notmuch wiki concerns

2012-03-26 Thread Kyle Sexton
All,

I wanted to get everyone's thoughts on something I setup over this past
weekend.  I put up a site at http://notmuchwiki.org with notes I took
while getting started with notmuch[1], along with a skeleton for more
information that I'll be updating as I learn. 

There is a disclaimer on the front page saying:

"Please refer to the official page for more information.  This wiki is
not endorsed, sponsored, or recommended by the official notmuch
community."

My goal is to make a site where the documentation is easy to update,
and attractive.  My concern is having too many places out there for
information.  I tried to assuage that with the disclaimer pointing to
the canonical site.  Does the community think there is space for an
unofficial wiki like this?

[1]: Most of the stuff I have written so far is under 'user setups' at
 http://www.notmuchwiki.org/display/usersetup/mocker

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


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

2012-03-26 Thread Dmitry Kurochkin
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.
> ---

LGTM.  Thanks for fixing this.

Regards,
  Dmitry

>  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 "' \
>[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


Re: [BUG/PATCH 1/2] test: Tests for reply from alternate addresses in emacs

2012-03-26 Thread Dmitry Kurochkin
Adam Wolfe Gordon  writes:
> Since the recent reply changes were pushed, there has been a bug that
> causes emacs to always reply from the primary address, even if the
> JSON or default CLI reply output uses an alternate address.
>
> This adds two tests to the emacs test library based on the two "Reply
> form..." tests in the reply test library. One is currently marked
> broken.
> ---
>  test/emacs |   52 
>  1 files changed, 52 insertions(+), 0 deletions(-)
>
> diff --git a/test/emacs b/test/emacs
> index 8a28705..fa5d706 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -274,6 +274,58 @@ Notmuch Test Suite  writes:
>  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_ot...@notmuchmail.org \
> +  [subject]=notmuch-reply-test \
> + '[date]="Tue, 05 Jan 2010 15:43:56 -"' \
> + '[body]="reply from alternate address"'

Please remove subject, data and body parameters.  add_message would
provide sane default values.

> +
> +test_emacs "(notmuch-search \"id:\\\"${gen_msg_id}\\\"\")
> + (notmuch-test-wait)
> + (notmuch-search-reply-to-thread)
> + (test-output)"
> +sed -i -e 's/^In-Reply-To: <.*>$/In-Reply-To: /' OUTPUT
> +cat  +From: Notmuch Test Suite 
> +To: Sender 
> +Subject: Re: notmuch-reply-test
> +In-Reply-To: 
> +Fcc: ${MAIL_DIR}/sent
> +--text follows this line--
> +Sender  writes:
> +
> +> reply from alternate address
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED
> +
> +test_begin_subtest "Reply from address in named group list within emacs"
> +add_message '[from]="Sender "' \
> +'[to]=group:test_su...@notmuchmail.org,some...@example.com\;' \
> + [cc]=test_suite_ot...@notmuchmail.org \
> + [subject]=notmuch-reply-test \
> +'[date]="Tue, 05 Jan 2010 15:43:56 -"' \
> +'[body]="Reply from address in named group list"'

Same here.

Regards,
  Dmitry

> +
> +test_emacs "(notmuch-search \"id:\\\"${gen_msg_id}\\\"\")
> + (notmuch-test-wait)
> + (notmuch-search-reply-to-thread)
> + (test-output)"
> +sed -i -e 's/^In-Reply-To: <.*>$/In-Reply-To: /' OUTPUT
> +cat  +From: Notmuch Test Suite 
> +To: Sender , some...@example.com
> +Subject: Re: notmuch-reply-test
> +In-Reply-To: 
> +Fcc: ${MAIL_DIR}/sent
> +--text follows this line--
> +Sender  writes:
> +
> +> Reply from address in named group list
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED
> +
>  test_begin_subtest "Reply within emacs to a multipart/mixed message"
>  test_emacs '(notmuch-show "id:20091118002059.067214ed@hikari")
>   (notmuch-show-reply)
> -- 
> 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


Re: Reply code with TEXT/PLAIN

2012-03-26 Thread Jameson Graef Rollins
On Mon, 26 Mar 2012 11:24:29 -0400, Daniel Kahn Gillmor 
 wrote:
> On 03/26/2012 11:21 AM, Jameson Graef Rollins wrote:
> > Does it make sense for the cli to canonicalize the content types to
> > always be lower case?  Or should it continue to just pass the content
> > type exactly as it appears in the original message?  Given that
> > consumers should parse it case insensitively, it maybe doesn't matter,
> > but it might also be nice for us to be consistent.
> 
> i think "notmuch emits the content-type in the exact form contained in
> the underlying message" is a good flavor of consistency.

Works for me.

jamie.


pgpG2n9JgXMB8.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Reply code with TEXT/PLAIN

2012-03-26 Thread Jameson Graef Rollins
On Mon, 26 Mar 2012 11:24:29 -0400, Daniel Kahn Gillmor  wrote:
> On 03/26/2012 11:21 AM, Jameson Graef Rollins wrote:
> > Does it make sense for the cli to canonicalize the content types to
> > always be lower case?  Or should it continue to just pass the content
> > type exactly as it appears in the original message?  Given that
> > consumers should parse it case insensitively, it maybe doesn't matter,
> > but it might also be nice for us to be consistent.
> 
> i think "notmuch emits the content-type in the exact form contained in
> the underlying message" is a good flavor of consistency.

Works for me.

jamie.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20120326/c679cb40/attachment.pgp>


Re: Bug#628018: [PATCH] notmuch-mutt utility for notmuch/contrib/

2012-03-26 Thread Stefano Zacchiroli
On Mon, Mar 26, 2012 at 08:09:13AM -0700, Jameson Graef Rollins wrote:
> > Another thing is this .cache hardcoding. Should this be resolved
> > first by using $XDG_CACHE_HOME, (then *MAYBE* $XDG_CONFIG_HOME/.cache)
> > and finally $HOME/.cache
> 
> These are good points.  I agree that since ~/.cache is an XDG standard
> we should resolve the appropriate environment variables.

Heya, thanks for your feedback. I agree as well!

But while it's trivial to make notmuch-mutt itself support
$XDG_CACHE_HOME, it is less so for the Mutt configuration snippet
(i.e. the notmuch-mutt.rc file which is part of my submission). AFAICT
the  function does not support variable
expansions, not to mention default values while doing so. As a result, I
can easily support $XDG_CACHE_HOME for, say, the history file, but I
don't know how to make mutt look in some $XDG_CACHE_HOME derived
directory. (Yes, I'm excluding hackish solutions like have mutt look
always in the same dir, and make that dir a "moving" symlink that will
be changed by notmuch-mutt upon execution. That seems to defeat the
benefits of the XDG specification, at least partly.)

Bottom line: I'll be happy to properly support $XDG_CACHE_HOME, but I'm
in need on suggestions of how to do so for the Mutt configuration part.

> If notmuch-mutt is accepted into notmuch upstream then it really becomes
> a part of notmuch and isn't really "public" anymore (at least in the
> sense that you're using).  So I don't really see any problem with having
> it use ~/.cache/notmuch.

My thought exactly.

The proposal of documenting how people should use ~/.cache/notmuch is
great, but I was hoping it can be overlooked for contrib stuff that has
been vetted by you folks.

Thanks for your help (and for notmuch!)
Cheers.
-- 
Stefano Zacchiroli zack@{upsilon.cc,pps.jussieu.fr,debian.org} . o .
Maître de conférences   ..   http://upsilon.cc/zack   ..   . . o
Debian Project Leader...   @zack on identi.ca   ...o o o
« the first rule of tautology club is the first rule of tautology club »
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: Reply code with TEXT/PLAIN

2012-03-26 Thread Daniel Kahn Gillmor
On 03/26/2012 11:21 AM, Jameson Graef Rollins wrote:
> Does it make sense for the cli to canonicalize the content types to
> always be lower case?  Or should it continue to just pass the content
> type exactly as it appears in the original message?  Given that
> consumers should parse it case insensitively, it maybe doesn't matter,
> but it might also be nice for us to be consistent.

i think "notmuch emits the content-type in the exact form contained in
the underlying message" is a good flavor of consistency.

--dkg




signature.asc
Description: OpenPGP digital signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


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

2012-03-26 Thread Jameson Graef Rollins
On Sun, 25 Mar 2012 00:43:28 +, Mark Walters  
wrote:
> 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
>'(

LGTM.

jamie.


pgpdVYz3rnmHw.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


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

2012-03-26 Thread Jameson Graef Rollins
On Sun, 25 Mar 2012 00:43:28 +, Mark Walters  
wrote:
> 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
>'(

LGTM.

jamie.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20120326/025634a8/attachment.pgp>


Re: Reply code with TEXT/PLAIN

2012-03-26 Thread Jameson Graef Rollins
On Sat, 24 Mar 2012 18:19:50 -0600, Adam Wolfe Gordon  
wrote:
> A bit of Googling indicates that MIME types should be case insensitive
> (i.e. TEXT/PLAIN should match text/plain). Given this, I think it
> makes sense to change the emacs function regardless of whether it
> makes sense for the CLI to output them in lower-case.

Does it make sense for the cli to canonicalize the content types to
always be lower case?  Or should it continue to just pass the content
type exactly as it appears in the original message?  Given that
consumers should parse it case insensitively, it maybe doesn't matter,
but it might also be nice for us to be consistent.  Maybe it's just ok
as it is.

jamie.


pgpOP7WJv13cU.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Reply code with TEXT/PLAIN

2012-03-26 Thread Jameson Graef Rollins
On Sat, 24 Mar 2012 18:19:50 -0600, Adam Wolfe Gordon  
wrote:
> A bit of Googling indicates that MIME types should be case insensitive
> (i.e. TEXT/PLAIN should match text/plain). Given this, I think it
> makes sense to change the emacs function regardless of whether it
> makes sense for the CLI to output them in lower-case.

Does it make sense for the cli to canonicalize the content types to
always be lower case?  Or should it continue to just pass the content
type exactly as it appears in the original message?  Given that
consumers should parse it case insensitively, it maybe doesn't matter,
but it might also be nice for us to be consistent.  Maybe it's just ok
as it is.

jamie.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20120326/85a3f9eb/attachment.pgp>


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

2012-03-26 Thread Jameson Graef Rollins
On Sun, 25 Mar 2012 22:18:43 +0300, 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.
> 
> 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.

If this changes the behavior of notmuch then I think there should be a
test for it.  We should probably see a test for the current behavior,
and a modification of it to reflect whatever behavior is changing.

jamie.


pgpIw4x9uUrsj.pgp
Description: PGP signature
___
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-26 Thread Jameson Graef Rollins
On Sun, 25 Mar 2012 22:18:43 +0300, 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.
> 
> 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.

If this changes the behavior of notmuch then I think there should be a
test for it.  We should probably see a test for the current behavior,
and a modification of it to reflect whatever behavior is changing.

jamie.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20120326/7be92e03/attachment.pgp>


Re: [Stefano Zacchiroli] Bug#628018: [PATCH] notmuch-mutt utility for notmuch/contrib/

2012-03-26 Thread Jameson Graef Rollins
On Mon, 26 Mar 2012 15:34:04 +0300, Tomi Ollila  wrote:
> I think storing stuff to  .../.cache/... is Great !!!
> 
> However, if we're "allowing" public software to use .../.cache/notmuch
> directory we should have policy to do so. I suggest we add a description
> file to notmuch source distribution which describes the public software
> that are allowed to contain files in notmuch XDG directories. The file
> could have format which describes the file/directory name, the software
> using it and some description why this file is there.
> 
> Another thing is this .cache hardcoding. Should this be resolved
> first by using $XDG_CACHE_HOME, (then *MAYBE* $XDG_CONFIG_HOME/.cache)
> and finally $HOME/.cache

These are good points.  I agree that since ~/.cache is an XDG standard
we should resolve the appropriate environment variables.

If notmuch-mutt is accepted into notmuch upstream then it really becomes
a part of notmuch and isn't really "public" anymore (at least in the
sense that you're using).  So I don't really see any problem with having
it use ~/.cache/notmuch.

jamie.


pgpleHXOctaR8.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[Stefano Zacchiroli] Bug#628018: [PATCH] notmuch-mutt utility for notmuch/contrib/

2012-03-26 Thread Jameson Graef Rollins
On Mon, 26 Mar 2012 15:34:04 +0300, Tomi Ollila  wrote:
> I think storing stuff to  .../.cache/... is Great !!!
> 
> However, if we're "allowing" public software to use .../.cache/notmuch
> directory we should have policy to do so. I suggest we add a description
> file to notmuch source distribution which describes the public software
> that are allowed to contain files in notmuch XDG directories. The file
> could have format which describes the file/directory name, the software
> using it and some description why this file is there.
> 
> Another thing is this .cache hardcoding. Should this be resolved
> first by using $XDG_CACHE_HOME, (then *MAYBE* $XDG_CONFIG_HOME/.cache)
> and finally $HOME/.cache

These are good points.  I agree that since ~/.cache is an XDG standard
we should resolve the appropriate environment variables.

If notmuch-mutt is accepted into notmuch upstream then it really becomes
a part of notmuch and isn't really "public" anymore (at least in the
sense that you're using).  So I don't really see any problem with having
it use ~/.cache/notmuch.

jamie.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20120326/cb648455/attachment.pgp>


Re: [PATCH 1/2] config: Add 'config list' command

2012-03-26 Thread Jameson Graef Rollins
On Wed, 21 Mar 2012 20:30:43 +0200, Tomi Ollila  wrote:
> On Wed, 21 Mar 2012 09:31:37 +1100, Peter Wang  wrote:
> > Add a command to list all keys in a given configuration section.
> > 
> > One use is as follows: a MUA may prefer to store data in a central
> > notmuch configuration file so that the data is accessible across
> > different machines, e.g. an addressbook.  The list command helps
> > to implement features such as tab completion on the keys.
> 
> Before getting deeper into implementation it is good to remember
> that 'notmuch config' is loosely modeled after 'git config'.
> 
> git config --list outputs somewhat different output.
> 
> Maybe this command should be like 'notmuch config list-section-keys'

Git's interface is completely inconsistent, so I see no reason to try to
emulate it in every regard.  Let's just try to keep the notmuch
interface as nice and consistent as possible.

I do agree, though, that I think I would prefer to see the output in the
same format as "git config --list".  So I would suggest the command and
output be:

$ notmuch config list
database.path=/home/jrollins/.mail
user.name=Jameson Graef Rollins
user.primary_email=jroll...@finestructure.net
user.other_email=
new.tags=new;unread
maildir.synchronize_flags=false
search.exclude_tags=deleted;spam;

That seem like a much more generally useful output.

jamie.


pgpmGkycPlWsL.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 1/2] config: Add 'config list' command

2012-03-26 Thread Jameson Graef Rollins
On Wed, 21 Mar 2012 20:30:43 +0200, Tomi Ollila  wrote:
> On Wed, 21 Mar 2012 09:31:37 +1100, Peter Wang  wrote:
> > Add a command to list all keys in a given configuration section.
> > 
> > One use is as follows: a MUA may prefer to store data in a central
> > notmuch configuration file so that the data is accessible across
> > different machines, e.g. an addressbook.  The list command helps
> > to implement features such as tab completion on the keys.
> 
> Before getting deeper into implementation it is good to remember
> that 'notmuch config' is loosely modeled after 'git config'.
> 
> git config --list outputs somewhat different output.
> 
> Maybe this command should be like 'notmuch config list-section-keys'

Git's interface is completely inconsistent, so I see no reason to try to
emulate it in every regard.  Let's just try to keep the notmuch
interface as nice and consistent as possible.

I do agree, though, that I think I would prefer to see the output in the
same format as "git config --list".  So I would suggest the command and
output be:

$ notmuch config list
database.path=/home/jrollins/.mail
user.name=Jameson Graef Rollins
user.primary_email=jrollins at finestructure.net
user.other_email=
new.tags=new;unread
maildir.synchronize_flags=false
search.exclude_tags=deleted;spam;

That seem like a much more generally useful output.

jamie.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20120326/f7b1652b/attachment.pgp>


Re: [PATCH 1/2] config: Add 'config list' command

2012-03-26 Thread Jameson Graef Rollins
On Wed, 21 Mar 2012 09:31:37 +1100, Peter Wang  wrote:
> Add a command to list all keys in a given configuration section.

Hi, Peter.  Don't forget to include appropriate tests when introducing
new features.  Thanks.

jamie.


pgphweDH7SbNb.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 1/2] config: Add 'config list' command

2012-03-26 Thread Jameson Graef Rollins
On Wed, 21 Mar 2012 09:31:37 +1100, Peter Wang  wrote:
> Add a command to list all keys in a given configuration section.

Hi, Peter.  Don't forget to include appropriate tests when introducing
new features.  Thanks.

jamie.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20120326/fc4cb00e/attachment-0001.pgp>


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

2012-03-26 Thread Jani Nikula
On Mon, 26 Mar 2012 00:26:50 +0300, Tomi Ollila  wrote:
> Jani Nikula  writes:
> 
> > On Sun, 25 Mar 2012 23:45:39 +0300, Tomi Ollila  
> > wrote:
> >> Jani Nikula  writes:
> >> 
> >> > Refactor to make tagging code easier to reuse in the future. No
> >> > functional changes.
> >> >
> >> > Signed-off-by: Jani Nikula 
> >> > ---
> >> 
> >> 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.

Actually, no. In the future (the follow-up patches are in my local
tree...) tag_query() should work also when there are no tagging
operations. I'll either need to fix tag_query() or _optimize_tag_query()
to handle this gracefully. Thanks for spelling this out for me! :)

> >> 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.

Sounds reasonable.

BR,
Jani.



> 
> >> 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_destro

[Stefano Zacchiroli] Bug#628018: [PATCH] notmuch-mutt utility for notmuch/contrib/

2012-03-26 Thread David Bremner
An embedded message was scrubbed...
From: Stefano Zacchiroli 
Subject: Bug#628018: [PATCH] notmuch-mutt utility for notmuch/contrib/
Date: Mon, 26 Mar 2012 11:36:06 +0200
Size: 21709
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20120326/64ecca7c/attachment.mht>


[Stefano Zacchiroli] Bug#628018: [PATCH] mutt-notmuch in notmuch contrib

2012-03-26 Thread David Bremner

Hi, this will probably eventually make it to the list, but until then,
here is a forwarded version. Please keep Stefano in CC, I don't think he's
subscribed.

-- next part --
An embedded message was scrubbed...
From: Stefano Zacchiroli 
Subject: Bug#628018: [PATCH] mutt-notmuch in notmuch contrib
Date: Sun, 25 Mar 2012 16:01:51 +0200
Size: 25698
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20120326/2d1c5c79/attachment-0001.mht>


Re: [Stefano Zacchiroli] Bug#628018: [PATCH] notmuch-mutt utility for notmuch/contrib/

2012-03-26 Thread Tomi Ollila

Interesting, 'R' in emacs gives empty content. I might have Austin's 
reply change in this version -- although I suspect the problem is not
there. Everyone using emacs MUA with latest notmuch(1)es experiment
pressing 'R' while reading the mail I'm replying to

Pasting content...

> [ multipart/mixed ]
> [ message/rfc822 ]
> From: Stefano Zacchiroli 
> Subject: Bug#628018: [PATCH] notmuch-mutt utility for notmuch/contrib/
> To: 628...@bugs.debian.org
> Date: Mon, 26 Mar 2012 11:36:06 +0200
>
> [ multipart/signed ]
> [ Good signature by key: 0x4900707DDC5C07F2DECB02839C31503C6D866396 ]
> [ multipart/mixed ]
> [ text/plain ]
> Here is an updated patch set that ships the notmuch-mutt utility and
> updated the Debian packaging for it. It is now better integrated with
> notmuch: it has been renamed to "notmuch-mutt" (to match the naming
> convention of other notmuch interfaces) and stores all its data under
> ~/.cache/notmuch/mutt/ to avoid polluting user $HOME with other
> directories. I've also fixed the Debian packaging to generate the
> manpage during build.

I think storing stuff to  .../.cache/... is Great !!!

However, if we're "allowing" public software to use .../.cache/notmuch
directory we should have policy to do so. I suggest we add a description
file to notmuch source distribution which describes the public software
that are allowed to contain files in notmuch XDG directories. The file
could have format which describes the file/directory name, the software
using it and some description why this file is there.

Another thing is this .cache hardcoding. Should this be resolved
first by using $XDG_CACHE_HOME, (then *MAYBE* $XDG_CONFIG_HOME/.cache)
and finally $HOME/.cache

Tomi


>
> The first patch in the set adds notmuch-mutt to contrib/, the second
> updates debian/ to build a new "notmuch-mutt" binary package.
>
> The full story is available at http://bugs.debian.org/628018
>
> David: I'm still unable to get through the moderation queue of the
> notmuch mailing list. Would you be so kind to forward this mail there,
> for patch review?
>
> TIA,
> Cheers.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[Stefano Zacchiroli] Bug#628018: [PATCH] notmuch-mutt utility for notmuch/contrib/

2012-03-26 Thread David Bremner
--- Begin Message ---
Here is an updated patch set that ships the notmuch-mutt utility and
updated the Debian packaging for it. It is now better integrated with
notmuch: it has been renamed to "notmuch-mutt" (to match the naming
convention of other notmuch interfaces) and stores all its data under
~/.cache/notmuch/mutt/ to avoid polluting user $HOME with other
directories. I've also fixed the Debian packaging to generate the
manpage during build.

The first patch in the set adds notmuch-mutt to contrib/, the second
updates debian/ to build a new "notmuch-mutt" binary package.

The full story is available at http://bugs.debian.org/628018

David: I'm still unable to get through the moderation queue of the
notmuch mailing list. Would you be so kind to forward this mail there,
for patch review?

TIA,
Cheers.
-- 
Stefano Zacchiroli zack@{upsilon.cc,pps.jussieu.fr,debian.org} . o .
Maître de conférences   ..   http://upsilon.cc/zack   ..   . . o
Debian Project Leader...   @zack on identi.ca   ...o o o
« the first rule of tautology club is the first rule of tautology club »
From cbb43be7d163e7662c0aa4fcb69f173074bb947d Mon Sep 17 00:00:00 2001
From: Stefano Zacchiroli 
Date: Mon, 26 Mar 2012 10:45:58 +0200
Subject: [PATCH 1/2] contrib: new mutt-notmuch utility for Mutt integration

---
 contrib/notmuch-mutt/.gitignore  |2 +
 contrib/notmuch-mutt/Makefile|   12 ++
 contrib/notmuch-mutt/README  |   59 +
 contrib/notmuch-mutt/notmuch-mutt|  234 ++
 contrib/notmuch-mutt/notmuch-mutt.rc |9 ++
 5 files changed, 316 insertions(+), 0 deletions(-)
 create mode 100644 contrib/notmuch-mutt/.gitignore
 create mode 100644 contrib/notmuch-mutt/Makefile
 create mode 100644 contrib/notmuch-mutt/README
 create mode 100755 contrib/notmuch-mutt/notmuch-mutt
 create mode 100644 contrib/notmuch-mutt/notmuch-mutt.rc

diff --git a/contrib/notmuch-mutt/.gitignore b/contrib/notmuch-mutt/.gitignore
new file mode 100644
index 000..682a577
--- /dev/null
+++ b/contrib/notmuch-mutt/.gitignore
@@ -0,0 +1,2 @@
+notmuch-mutt.1
+README.html
diff --git a/contrib/notmuch-mutt/Makefile b/contrib/notmuch-mutt/Makefile
new file mode 100644
index 000..87f9031
--- /dev/null
+++ b/contrib/notmuch-mutt/Makefile
@@ -0,0 +1,12 @@
+NAME = notmuch-mutt
+
+all: $(NAME) $(NAME).1
+
+$(NAME).1: $(NAME)
+	pod2man $< > $@
+
+README.html: README
+	markdown $< > $@
+
+clean:
+	rm -f notmuch-mutt.1 README.html
diff --git a/contrib/notmuch-mutt/README b/contrib/notmuch-mutt/README
new file mode 100644
index 000..382ac91
--- /dev/null
+++ b/contrib/notmuch-mutt/README
@@ -0,0 +1,59 @@
+notmuch-mutt: Notmuch (of a) helper for Mutt
+
+
+notmuch-mutt provide integration among the [Mutt] [1] mail user agent and the
+[Notmuch] [2] mail indexer.
+
+notmuch-mutt offer two main integration features. The first one is the ability
+of stating a **search query interactively** and then jump to a fresh Maildir
+containing its search results only. The second one is the ability to
+**reconstruct threads on the fly** starting from the currently highlighted
+mail, which comes handy when a thread has been split across different maildirs,
+archived, or the like.
+
+notmuch-mutt enables to trigger mail searches via a Mutt macro (usually F8) and
+reconstruct threads via another (usually F9). Check the manpage for the 2-liner
+configuration snippet for your Mutt configuration files (~/.muttrc,
+/etc/Muttrc, or a /etc/Muttrc.d snippet).
+
+A [blog style introduction] [3] to notmuch-mutt is available and includes some
+more rationale for its existence.
+
+Arguably, some of the logics of notmuch-mutt could disappear by adding support
+for a --output=symlinks flag to notmuch.
+
+
+[1]: http://www.mutt.org/
+[2]: http://notmuchmail.org/
+[3]: http://upsilon.cc/~zack/blog/posts/2011/01/how_to_use_Notmuch_with_Mutt/
+
+
+Requirements
+
+
+To *run* notmuch-mutt you will need Perl with the following libraries:
+
+- Mail::Box 
+  (Debian package: libmail-box-perl)
+- Mail::Internet 
+  (Debian package: libmailtools-perl)
+- String::ShellQuote 
+  (Debian package: libstring-shellquote-perl)
+- Term::ReadLine 
+  (Debian package: libterm-readline-gnu-perl)
+
+To *build* notmuch-mutt documentation you will need:
+
+- pod2man (coming with Perl) to generate the manpage
+- markdown to generate README.html out of this file
+
+
+License
+---
+
+notmuch-mutt is copyright (C) 2011-2012 Stefano Zacchiroli .
+
+notmuch-mutt is released under the terms of the GNU General Public License
+(GPL), version 3 or above. A copy of the license is available online at
+.
+
diff --git a/contrib/notmuch-mutt/notmuch-mutt b/contrib/n

[Stefano Zacchiroli] Bug#628018: [PATCH] mutt-notmuch in notmuch contrib

2012-03-26 Thread David Bremner

Hi, this will probably eventually make it to the list, but until then,
here is a forwarded version. Please keep Stefano in CC, I don't think he's
subscribed.

--- Begin Message ---
[ please Cc:-me on follow-ups, as I'm not subscribed to the
  notmuch@notmuchmail.org mailing list ]

On Sun, Mar 25, 2012 at 10:13:38AM -0300, David Bremner wrote:
> 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.

Hi David, thanks for the tip.

Dear notmuch list, please find attached a proposed patch that add a new
contrib utility called mutt-notmuch for integration with Mutt. Its full
rationale can be found in the contained README. Background for this
submission, including previous discussions to ship mutt-notmuch as part
of notmuch, can be found at
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=628018

I welcome your comments on how to adapt the patch to your preferences
for contrib/, err, contributions.

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

I'll be pleased to do so when/if mutt-notmuch is accepted in contrib/,
just out of general laziness for potentially useless work :-)

Thanks,
Cheers.
-- 
Stefano Zacchiroli zack@{upsilon.cc,pps.jussieu.fr,debian.org} . o .
Maître de conférences   ..   http://upsilon.cc/zack   ..   . . o
Debian Project Leader...   @zack on identi.ca   ...o o o
« the first rule of tautology club is the first rule of tautology club »
From 8f5be32912920c2c5eca7084885c0e2498540311 Mon Sep 17 00:00:00 2001
From: Stefano Zacchiroli 
Date: Sun, 25 Mar 2012 15:55:44 +0200
Subject: [PATCH] contrib: new utility mutt-notmuch for Mutt integration

---
 contrib/mutt-notmuch/Makefile   |   12 ++
 contrib/mutt-notmuch/README |   58 +
 contrib/mutt-notmuch/mutt-notmuch   |  234 +++
 contrib/mutt-notmuch/mutt-notmuch.1 |  210 +++
 4 files changed, 514 insertions(+), 0 deletions(-)
 create mode 100644 contrib/mutt-notmuch/Makefile
 create mode 100644 contrib/mutt-notmuch/README
 create mode 100755 contrib/mutt-notmuch/mutt-notmuch
 create mode 100644 contrib/mutt-notmuch/mutt-notmuch.1

diff --git a/contrib/mutt-notmuch/Makefile b/contrib/mutt-notmuch/Makefile
new file mode 100644
index 000..b78953d
--- /dev/null
+++ b/contrib/mutt-notmuch/Makefile
@@ -0,0 +1,12 @@
+NAME = mutt-notmuch
+
+all: $(NAME) $(NAME).1
+
+$(NAME).1: $(NAME)
+	pod2man $< > $@
+
+README.html: README
+	markdown $< > $@
+
+clean:
+	rm -f mutt-notmuch.1 README.html
diff --git a/contrib/mutt-notmuch/README b/contrib/mutt-notmuch/README
new file mode 100644
index 000..e72aa91
--- /dev/null
+++ b/contrib/mutt-notmuch/README
@@ -0,0 +1,58 @@
+mutt-notmuch: Notmuch (of a) helper for Mutt
+
+
+mutt-notmuch provide integration among the [Mutt] [1] mail user agent and the
+[Notmuch] [2] mail indexer.
+
+mutt-notmuch offer two basic integration features. The first one is the ability
+of stating a **search query interactively** and then jump to a fresh Maildir
+containing its search results only. The second one is the ability to
+**reconstruct threads on the fly** starting from the currently highlighted
+mail, which comes handy when a thread has been split across different maildirs,
+archived, or the like.
+
+mutt-notmuch enables to trigger mail searches via a Mutt macro (F8 being my
+choice) and reconstruct threads via another (F9). Check the manpage for the
+2-liner configuration snippet for your ~/.muttrc.
+
+A [blog style introduction] [3] to mutt-notmuch is available and includes some
+more rationale for its existence.
+
+Arguably, some of the logics of mutt-notmuch could disappear by adding support
+for a --output=symlinks flag to notmuch.
+
+
+[1]: http://www.mutt.org/
+[2]: http://notmuchmail.org/
+[3]: http://upsilon.cc/~zack/blog/posts/2011/01/how_to_use_Notmuch_with_Mutt/
+
+
+Requirements
+
+
+To *run* mutt-notmuch you will need Perl with the following libraries:
+
+- Mail::Box 
+  (Debian package: libmail-box-perl)
+- Mail::Internet 
+  (Debian package: libmailtools-perl)
+- String::ShellQuote 
+  (Debian package: libstring-shellquote-perl)
+- Term::ReadLine 
+  (Debian package: libterm-readline-gnu-perl)
+
+To *build* mutt-notmuch documentation you will need:
+
+- pod2man (coming with Perl) to generate the manpage
+- markdown to generate README.html out of this file
+
+
+License
+---
+
+mutt-notmuch is copyright (C) 2011-201

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

2012-03-26 Thread Jani Nikula
On Mon, 26 Mar 2012 00:26:50 +0300, Tomi Ollila  wrote:
> Jani Nikula  writes:
> 
> > On Sun, 25 Mar 2012 23:45:39 +0300, Tomi Ollila  wrote:
> >> Jani Nikula  writes:
> >> 
> >> > Refactor to make tagging code easier to reuse in the future. No
> >> > functional changes.
> >> >
> >> > Signed-off-by: Jani Nikula 
> >> > ---
> >> 
> >> 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.

Actually, no. In the future (the follow-up patches are in my local
tree...) tag_query() should work also when there are no tagging
operations. I'll either need to fix tag_query() or _optimize_tag_query()
to handle this gracefully. Thanks for spelling this out for me! :)

> >> 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.

Sounds reasonable.

BR,
Jani.



> 
> >> 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 (me

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

2012-03-26 Thread Tomi Ollila
Jani Nikula  writes:

> On Sun, 25 Mar 2012 23:45:39 +0300, Tomi Ollila  wrote:
>> Jani Nikula  writes:
>> 
>> > Refactor to make tagging code easier to reuse in the future. No
>> > functional changes.
>> >
>> > Signed-off-by: Jani Nikula 
>> > ---
>> 
>> 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));

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

2012-03-26 Thread Jani Nikula
On Sun, 25 Mar 2012 23:45:39 +0300, Tomi Ollila  wrote:
> Jani Nikula  writes:
> 
> > Refactor to make tagging code easier to reuse in the future. No
> > functional changes.
> >
> > Signed-off-by: Jani Nikula 
> > ---
> 
> 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