[PATCH 3/4] emacs: Make tags that appear in `notmuch-show' clickable

2012-11-18 Thread Mark Walters

Hi

This patch adds buttons for every tag in show mode which means that
tabbing down to an attachment (or other button) is rather slower. As
someone who is not going to use the tag buttons (at least not very
often) Is it possible to avoid this? perhaps by making the button
optional or making the button only mouse clickable? (Since the buttons
in the headerline are not tabbable to then the latter might be
reasonable.)

Best wishes

Mark


On Sun, 18 Nov 2012, Damien Cassou  wrote:
> Signed-off-by: Damien Cassou 
> ---
>  emacs/notmuch-show.el   |9 +
>  emacs/notmuch-tagger.el |   33 +
>  2 files changed, 38 insertions(+), 4 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 988e27c..379c8cd 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -431,10 +431,11 @@ message at DEPTH in the current thread."
>   (notmuch-show-clean-address (plist-get headers :From))
>   " ("
>   date
> - ") ("
> - (propertize (mapconcat 'identity tags " ")
> - 'face 'notmuch-tag-face)
> - ")\n")
> + ") "
> + (propertize
> +  (format-mode-line (notmuch-tagger-present-tags tags))
> +  'face 'notmuch-tag-face)
> + "\n")
>  (overlay-put (make-overlay start (point)) 'face 
> 'notmuch-message-summary-face)))
>  
>  (defun notmuch-show-insert-header (header header-value)
> diff --git a/emacs/notmuch-tagger.el b/emacs/notmuch-tagger.el
> index 19a6c7e..379a905 100644
> --- a/emacs/notmuch-tagger.el
> +++ b/emacs/notmuch-tagger.el
> @@ -53,12 +53,21 @@ test if the library is present before calling this 
> function."
>(let ((tag (header-button-get button 'notmuch-tagger-tag)))
>  (notmuch-tagger-goto-target tag)))
>  
> +(defun notmuch-tagger-body-button-action (button)
> +  "Open `notmuch-search' for the tag referenced by BUTTON."
> +  (let ((tag (button-get button 'notmuch-tagger-tag)))
> +(notmuch-tagger-goto-target tag)))
> +
>  (eval-after-load "header-button"
>'(define-button-type 'notmuch-tagger-header-button-type
>   'supertype 'header
>   'action#'notmuch-tagger-header-button-action
>   'follow-link t))
>  
> +(define-button-type 'notmuch-tagger-body-button-type
> +  'action#'notmuch-tagger-body-button-action
> +  'follow-link t)
> +
>  (defun notmuch-tagger-really-make-header-link (tag)
> "Return a property list that presents a link to TAG.
>  
> @@ -82,6 +91,19 @@ if not."
>(notmuch-tagger-really-make-header-link tag)
>  tag))
>  
> +(defun notmuch-tagger-make-body-link (tag)
> +  "Return a property list that presents a link to TAG.
> +The returned property list will work everywhere except in the
> +header-line. For a link that works on the header-line, prefer
> +`notmuch-tagger-make-header-link'."
> +  (let ((button (copy-sequence tag)))
> +(make-text-button
> + button nil
> + 'type 'notmuch-tagger-body-button-type
> + 'notmuch-tagger-tag tag
> + 'help-echo (format "%s: Search other messages like this" tag))
> +button))
> +
>  (defun notmuch-tagger-present-tags-header-line (tags)
>"Return a property list to present TAGS in emacs header-line."
>(list
> @@ -91,6 +113,17 @@ if not."
>  " ")
> ")"))
>  
> +(defun notmuch-tagger-present-tags (tags)
> +  "Return a property list to present TAGS in emacs.
> +If tags the result of this function is to be used within the
> +header-line, prefer `notmuch-tagger-present-tags-header-line'
> +instead of this function."
> +  (list
> +   "("
> +   (notmuch-tagger-separate-elems
> +(mapcar #'notmuch-tagger-make-body-link tags)
> +" ")
> +   ")"))
>  
>  (provide 'notmuch-tagger)
>  ;;; notmuch-tagger.el ends here
> -- 
> 1.7.10.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


emacs's function special form

2012-11-18 Thread Austin Clements
On Sun, 18 Nov 2012, Ethan  wrote:
> On Sun, Nov 18, 2012 at 7:10 PM, Aaron Ecay  wrote:
>
>> 2012ko azaroak 18an, Ethan Glasser-Camp-ek idatzi zuen:
>> >
>> > - You might want to use #' on lambdas.
>>
>> This is actually unnecessary ? as the info node "(elisp) Anonymous
>> Functions" says, the forms with and without #' are equivalent.  The
>> current notmuch style is not to have #' on lambdas (that is, there are 0
>> instances of #'(lambda ...) in the code base).  IMO that?s correct:
>> the unnecessary #' is just line-noise-ish.
>>
>
> OK, I think I understand. Thanks for the clarification. I found that info
> node very confusing.
>
> It says that #' has effects "assuming function-object is a valid lambda
> expression". So why put #' on variables that are the names of functions?
>
> In fact, outside names of functions, when are you supposed to use #' at all?

You don't need to use #' on a lambda form because lambda is actually a
macro (implemented in Elisp!) that expands to
  #'(lambda ...)
so using #' would be redundant.  But you should use it on function
symbols to hint the byte code compiler that you're referring to a
function rather than a variable.  Using #' on symbols affects the
optimizer (and may improve warnings?), even though the info node doesn't
explicitly say how.  Plus, #' makes code more self-documenting.


[PATCH 2/2] emacs: less guessing of character set in messages

2012-11-18 Thread Tomi Ollila
The macro with-current-notmuch-show-message executes command
`notmuch show --format=raw id:...` which just outputs the contents
of the mail file verbatim (into temporary buffer). In case e.g. utf-8
locale is used the temporary buffer has buffer-file-coding-system as
utf-8. In this case Emacs converts the data to multibyte format, guessing
that input is in utf-8.
However, the "raw" (MIME) message may contain octet data in any other
8bit format, and as no (MIME-)content spesific handling to the message
is done at this point, conversion to other formats may lose information.
By setting coding-system-for-read 'no-conversion drops the conversion part
and makes this handle input as notmuch-get-bodypart-internal() does.
This marks the broken test in previous change fixed.
---
 emacs/notmuch-show.el | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 5b3e70e..44b6f35 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -203,9 +203,10 @@ For example, if you wanted to remove an \"unread\" tag and 
add a
  (let ((id (notmuch-show-get-message-id)))
(let ((buf (generate-new-buffer (concat "*notmuch-msg-" id "*"
  (with-current-buffer buf
-   (call-process notmuch-command nil t nil "show" "--format=raw" id)
-   , at body)
-(kill-buffer buf)
+  (let ((coding-system-for-read 'no-conversion))
+(call-process notmuch-command nil t nil "show" "--format=raw" id)
+, at body)
+  (kill-buffer buf))

 (defun notmuch-show-turn-on-visual-line-mode ()
   "Enable Visual Line mode."
-- 
1.8.0



[PATCH 1/2] test/emacs: test saving of attachment containing 8bit octets

2012-11-18 Thread Tomi Ollila
This test catches the case 8bit octets in an attachment gets converted
or lost when saving attachment to the file.
This test is marked known broken.
---
 test/emacs | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/test/emacs b/test/emacs
index 77265b0..c8db7d0 100755
--- a/test/emacs
+++ b/test/emacs
@@ -516,6 +516,33 @@ test_emacs '(let ((standard-input "\"attachment2.gz\""))
  (notmuch-show-save-part 
"id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a at mail.gmail.com" 5))'
 test_expect_equal_file attachment2.gz "$EXPECTED/attachment"

+test_begin_subtest "Save 8bit attachment from within emacs using 
notmuch-show-save-attachments"
+test_subtest_known_broken
+
+add_message '[subject]="Attachment with 8bit chars"' \
+   '[header]="MIME-Version: 1.0"' \
+   '[content-type]="multipart/mixed; boundary=\"abcd\""' \
+   '[body]="--abcd
+Content-Type: text/plain
+
+Attachment follows:
+
+--abcd
+Content-Type: application/octet-stream; name=\"sample\"
+Content-Transfer-Encoding: 8bit
+Content-Disposition: attachment; filename=\"sample\"
+
+?? Hey ! It compiles ? Ship it !?
+
+--abcd--
+"'
+test_emacs '(notmuch-show "id:'"${gen_msg_id}"'")
+   (delete-file "OUTPUT")
+   (let ((standard-input "\"OUTPUT\""))
+ (notmuch-show-save-attachments))'
+
+test_expect_equal "$(cat OUTPUT)" '?? Hey ! It compiles ? Ship it !?'
+
 test_begin_subtest "View raw message within emacs"
 test_emacs '(notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a at 
mail.gmail.com")
(notmuch-show-view-raw-message)
-- 
1.8.0



emacs's function special form

2012-11-18 Thread Ethan
On Sun, Nov 18, 2012 at 7:10 PM, Aaron Ecay  wrote:

> 2012ko azaroak 18an, Ethan Glasser-Camp-ek idatzi zuen:
> >
> > - You might want to use #' on lambdas.
>
> This is actually unnecessary ? as the info node "(elisp) Anonymous
> Functions" says, the forms with and without #' are equivalent.  The
> current notmuch style is not to have #' on lambdas (that is, there are 0
> instances of #'(lambda ...) in the code base).  IMO that?s correct:
> the unnecessary #' is just line-noise-ish.
>

OK, I think I understand. Thanks for the clarification. I found that info
node very confusing.

It says that #' has effects "assuming function-object is a valid lambda
expression". So why put #' on variables that are the names of functions?

In fact, outside names of functions, when are you supposed to use #' at all?

Ethan
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20121118/c5217d1f/attachment.html>


[PATCH 4/4] emacs: Add unit-tests for clickable tags in notmuch-show

2012-11-18 Thread Damien Cassou
Signed-off-by: Damien Cassou 
---
 test/emacs |   62 
 1 file changed, 62 insertions(+)

diff --git a/test/emacs b/test/emacs
index 77265b0..2335bc4 100755
--- a/test/emacs
+++ b/test/emacs
@@ -813,5 +813,67 @@ test_emacs "(let ((mm-text-html-renderer
 test_expect_success "Rendering HTML mail with images" \
 'grep -q smiley OUTPUT'

+test_begin_subtest "Extracting all tags from a thread"
+add_message \
+'[subject]="Extracting all tags from a thread"' \
+'[body]="body 1"'
+parent=${gen_msg_id}
+add_message \
+'[subject]="Extracting all tags from a thread"' \
+'[body]="body 2"' \
+"[in-reply-to]=\<$parent\>"
+add_message \
+'[subject]="Extracting all tags from a thread"' \
+'[body]="body 3"' \
+"[in-reply-to]=\<$parent\>"
+latest=${gen_msg_id}
+# Extract the thread-id from one of the emails
+thread_id=$(notmuch search --output=threads id:${latest})
+echo THREAD ID: '"'$thread_id'"'
+# Add tag "mytagfoo" to one of the emails
+notmuch tag +mytagfoo id:${latest}
+test_emacs_expect_t \
+"(notmuch-show \"${thread_id}\")
+ (let ((output (notmuch-show-thread-tags))
+   (expected '(\"inbox\" \"mytagfoo\" \"unread\")))
+  (notmuch-test-expect-equal
+ (sort output #'string<)
+ (sort expected #'string<)))"
+
+test_begin_subtest "The tags appear in the header-line of notmuch-show"
+add_message \
+'[subject]="foo bar"' \
+'[body]="body 1"'
+parent=${gen_msg_id}
+# Add tag "mytagfoo" to one of the emails
+notmuch tag +mytagfoo id:${parent}
+# Extract the thread-id from one of the emails
+thread_id=$(notmuch search --output=threads id:${latest})
+test_emacs_expect_t \
+"(notmuch-show \"${thread_id}\")
+ (if (string-match-p \"mytagfoo\" (format-mode-line header-line-format))
+ t
+   \"The tag mytagfoo was not in the header-line-format\")"
+
+test_begin_subtest "The tags of notmuch-show emails are clickable"
+add_message \
+'[subject]="foo bar"' \
+'[body]="body 1"'
+parent=${gen_msg_id}
+# Add tag "mytagfoo" to one of the emails
+notmuch tag +mytagfoo id:${parent}
+# Extract the thread-id from one of the emails
+thread_id=$(notmuch search --output=threads id:${latest})
+test_emacs_expect_t \
+"(notmuch-show \"${thread_id}\")
+(goto-char (point-min))
+(re-search-forward \"mytagfoo\")
+(backward-char) ;; to be 'in' the tag
+(unless (eq major-mode 'notmuch-show-mode)
+   (error \"We must be in notmuch-show at this point but we are in %s.\" 
major-mode))
+(push-button) ;; simulate a press on the RET key
+(if (eq major-mode 'notmuch-search-mode)
+t
+   (format \"We must be in notmuch-search at this point but we are in 
%s.\" major-mode))"

 test_done
-- 
1.7.10.4



[PATCH 3/4] emacs: Make tags that appear in `notmuch-show' clickable

2012-11-18 Thread Damien Cassou
Signed-off-by: Damien Cassou 
---
 emacs/notmuch-show.el   |9 +
 emacs/notmuch-tagger.el |   33 +
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 988e27c..379c8cd 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -431,10 +431,11 @@ message at DEPTH in the current thread."
(notmuch-show-clean-address (plist-get headers :From))
" ("
date
-   ") ("
-   (propertize (mapconcat 'identity tags " ")
-   'face 'notmuch-tag-face)
-   ")\n")
+   ") "
+   (propertize
+(format-mode-line (notmuch-tagger-present-tags tags))
+'face 'notmuch-tag-face)
+   "\n")
 (overlay-put (make-overlay start (point)) 'face 
'notmuch-message-summary-face)))

 (defun notmuch-show-insert-header (header header-value)
diff --git a/emacs/notmuch-tagger.el b/emacs/notmuch-tagger.el
index 19a6c7e..379a905 100644
--- a/emacs/notmuch-tagger.el
+++ b/emacs/notmuch-tagger.el
@@ -53,12 +53,21 @@ test if the library is present before calling this 
function."
   (let ((tag (header-button-get button 'notmuch-tagger-tag)))
 (notmuch-tagger-goto-target tag)))

+(defun notmuch-tagger-body-button-action (button)
+  "Open `notmuch-search' for the tag referenced by BUTTON."
+  (let ((tag (button-get button 'notmuch-tagger-tag)))
+(notmuch-tagger-goto-target tag)))
+
 (eval-after-load "header-button"
   '(define-button-type 'notmuch-tagger-header-button-type
  'supertype 'header
  'action#'notmuch-tagger-header-button-action
  'follow-link t))

+(define-button-type 'notmuch-tagger-body-button-type
+  'action#'notmuch-tagger-body-button-action
+  'follow-link t)
+
 (defun notmuch-tagger-really-make-header-link (tag)
"Return a property list that presents a link to TAG.

@@ -82,6 +91,19 @@ if not."
   (notmuch-tagger-really-make-header-link tag)
 tag))

+(defun notmuch-tagger-make-body-link (tag)
+  "Return a property list that presents a link to TAG.
+The returned property list will work everywhere except in the
+header-line. For a link that works on the header-line, prefer
+`notmuch-tagger-make-header-link'."
+  (let ((button (copy-sequence tag)))
+(make-text-button
+ button nil
+ 'type 'notmuch-tagger-body-button-type
+ 'notmuch-tagger-tag tag
+ 'help-echo (format "%s: Search other messages like this" tag))
+button))
+
 (defun notmuch-tagger-present-tags-header-line (tags)
   "Return a property list to present TAGS in emacs header-line."
   (list
@@ -91,6 +113,17 @@ if not."
 " ")
")"))

+(defun notmuch-tagger-present-tags (tags)
+  "Return a property list to present TAGS in emacs.
+If tags the result of this function is to be used within the
+header-line, prefer `notmuch-tagger-present-tags-header-line'
+instead of this function."
+  (list
+   "("
+   (notmuch-tagger-separate-elems
+(mapcar #'notmuch-tagger-make-body-link tags)
+" ")
+   ")"))

 (provide 'notmuch-tagger)
 ;;; notmuch-tagger.el ends here
-- 
1.7.10.4



[PATCH 2/4] Make tags in header-line clickable

2012-11-18 Thread Damien Cassou
Signed-off-by: Damien Cassou 
---
 emacs/notmuch-tagger.el |   54 ++-
 1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/emacs/notmuch-tagger.el b/emacs/notmuch-tagger.el
index 5ca190e..19a6c7e 100644
--- a/emacs/notmuch-tagger.el
+++ b/emacs/notmuch-tagger.el
@@ -32,11 +32,63 @@
   (setq first nil)
   (push elt res

+(defun notmuch-tagger-header-button-present-p ()
+  "Check if `header-button' can be loaded or is already loaded.
+
+`header-button' is a third-party library which facilitates the
+creation of links in emacs header-line. This function tries to
+`require' `header-button' and returns nil if and only if this
+fails."
+  (require 'header-button nil t))
+
+(defun notmuch-tagger-goto-target (tag)
+  "Show a `notmuch-search' buffer for the TAG."
+  (notmuch-search (concat "tag:" tag)))
+
+(defun notmuch-tagger-header-button-action (button)
+  "Open `notmuch-search' for the tag referenced by BUTTON.
+This function depends on the presence of the `header-button'
+library. Please call `notmuch-tagger-header-button-present-p' to
+test if the library is present before calling this function."
+  (let ((tag (header-button-get button 'notmuch-tagger-tag)))
+(notmuch-tagger-goto-target tag)))
+
+(eval-after-load "header-button"
+  '(define-button-type 'notmuch-tagger-header-button-type
+ 'supertype 'header
+ 'action#'notmuch-tagger-header-button-action
+ 'follow-link t))
+
+(defun notmuch-tagger-really-make-header-link (tag)
+   "Return a property list that presents a link to TAG.
+
+The returned property list will only work in the header-line.
+Additionally, this function depends on the presence of the
+`header-button' library. Please call
+`notmuch-tagger-header-button-present-p' to test if library is
+present before calling this function."
+   (header-button-format
+tag
+:type 'notmuch-tagger-header-button-type
+'notmuch-tagger-tag tag
+'help-echo (format "%s: Search other messages like this" tag)))
+
+(defun notmuch-tagger-make-header-link (tag)
+  "Return a property list to present TAG as a link to search.
+
+This only works if `header-button' is loaded. Simply returns tag
+if not."
+  (if (notmuch-tagger-header-button-present-p)
+  (notmuch-tagger-really-make-header-link tag)
+tag))
+
 (defun notmuch-tagger-present-tags-header-line (tags)
   "Return a property list to present TAGS in emacs header-line."
   (list
"("
-   (notmuch-tagger-separate-elems tags " ")
+   (notmuch-tagger-separate-elems
+(mapcar #'notmuch-tagger-make-header-link tags)
+" ")
")"))


-- 
1.7.10.4



[PATCH 1/4] Add a thread's tags to emacs header-line

2012-11-18 Thread Damien Cassou
Signed-off-by: Damien Cassou 
---
 emacs/notmuch-show.el   |   24 +---
 emacs/notmuch-tagger.el |   44 
 2 files changed, 65 insertions(+), 3 deletions(-)
 create mode 100644 emacs/notmuch-tagger.el

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 5b3e70e..988e27c 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -36,6 +36,7 @@
 (require 'notmuch-mua)
 (require 'notmuch-crypto)
 (require 'notmuch-print)
+(require 'notmuch-tagger)

 (declare-function notmuch-call-notmuch-process "notmuch" ( args))
 (declare-function notmuch-fontify-headers "notmuch" nil)
@@ -1121,11 +1122,28 @@ function is used."

   (jit-lock-register #'notmuch-show-buttonise-links)

-  ;; Set the header line to the subject of the first message.
-  (setq header-line-format (notmuch-show-strip-re 
(notmuch-show-get-subject)))
-
+  (notmuch-show-update-header-line)
   (run-hooks 'notmuch-show-hook

+(defun notmuch-show-thread-tags ()
+  "Return the list of tags for the current thread."
+  (let ((tags (list)))
+(notmuch-show-mapc (lambda ()
+(mapcar (lambda (elt)
+  ;; Avoid adding duplicate tags
+  (add-to-list 'tags elt))
+(notmuch-show-get-tags
+tags))
+
+(defun notmuch-show-update-header-line ()
+  "Make the header-line show the thread's subject and tags."
+  (let ((thread-subject (notmuch-show-strip-re (notmuch-show-get-subject
+(setq header-line-format
+ (list
+  thread-subject
+  " "
+  (notmuch-tagger-present-tags-header-line 
(notmuch-show-thread-tags))
+
 (defun notmuch-show-capture-state ()
   "Capture the state of the current buffer.

diff --git a/emacs/notmuch-tagger.el b/emacs/notmuch-tagger.el
new file mode 100644
index 000..5ca190e
--- /dev/null
+++ b/emacs/notmuch-tagger.el
@@ -0,0 +1,44 @@
+;; notmuch-tagger.el --- Library to improve the way tags are displayed
+;;
+;; Copyright ? Damien Cassou
+;;
+;; This file is part of Notmuch.
+;;
+;; Notmuch is free software: you can redistribute it and/or modify it
+;; under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+;;
+;; Notmuch is distributed in the hope that it will be useful, but
+;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+;; General Public License for more details.
+;;
+;; You should have received a copy of the GNU General Public License
+;; along with Notmuch.  If not, see .
+;;
+;; Authors: Damien Cassou 
+;;; Commentary:
+;;
+;;; Code:
+;;
+(defun notmuch-tagger-separate-elems (list sep)
+  "Return a list with all elements of LIST separated by SEP."
+  (let ((first t)
+(res nil))
+(dolist (elt (reverse list) res)
+  (unless first
+(push sep res))
+  (setq first nil)
+  (push elt res
+
+(defun notmuch-tagger-present-tags-header-line (tags)
+  "Return a property list to present TAGS in emacs header-line."
+  (list
+   "("
+   (notmuch-tagger-separate-elems tags " ")
+   ")"))
+
+
+(provide 'notmuch-tagger)
+;;; notmuch-tagger.el ends here
-- 
1.7.10.4



[PATCH v2] emacs: display tags in notmuch-show with links

2012-11-18 Thread Damien Cassou
This patch obsoletes
id:1352565719-12397-1-git-send-email-damien.cassou at gmail.com

[PATCH 1/4] emacs: Add a thread's tags to emacs header-line
[PATCH 2/4] emacs: Make tags in header-line clickable
[PATCH 3/4] emacs: Make tags that appear in `notmuch-show' clickable
[PATCH 4/4] emacs: Add unit-tests for clickable tags in notmuch-show

This patch makes clickable all tags that appear in notmuch-show
buffers. Each tag is a link to open a new notmuch-search buffer for
this tag. Additionally, the buffer's header-line now shows the
thread's tags (clickable only if the `header-button' library is loaded
or loadable).

This patch is the first one of an upcoming series whose goal is to
integrate notmuch-labeler into notmuch. See the following for more
details:
https://github.com/DamienCassou/notmuch-labeler

With respect to v2, I took care of the comments you made:
- The header-button library is not attached to the patch anymore. The
  tags in the header-line are always displayed but made clickable only
  if the library is loaded or loadable.
- The large patch is separated into a series of 4 smaller patches.

You can follow this patch series on
https://github.com/DamienCassou/notmuch/tree/labeler-integration-slow


[PATCH v2] emacs: display tags in notmuch-show with links

2012-11-18 Thread Aaron Ecay
2012ko azaroak 18an, Ethan Glasser-Camp-ek idatzi zuen:
> 
> - You might want to use #' on lambdas.

This is actually unnecessary ? as the info node "(elisp) Anonymous
Functions" says, the forms with and without #' are equivalent.  The
current notmuch style is not to have #' on lambdas (that is, there are 0
instances of #'(lambda ...) in the code base).  IMO that?s correct:
the unnecessary #' is just line-noise-ish.

-- 
Aaron Ecay


[PATCH] emacs: functions to import sender or recipient into BBDB

2012-11-18 Thread Daniel Bergey
On 2012-10-19 at 18:52, Ethan Glasser-Camp  
wrote:
> Daniel Bergey  writes:
>
>> From a show buffer, bbdb/notmuch-snarf-from imports the sender into
>> bbdb.  bbdb/notmuch-snarf-to attempts to import all recipients.  BBDB
>> displays a buffer with each contact; C-g displays the next contact, or
>> returns to the notmuch-show buffer.
>>
>> This is my first notmuch patch.  Comments very welcome.
>>
>> +(defun bbdb/snarf-between-commas ()
>> +  ; What about names written "Surname, First M" ?
>
> I do think more sophisticated parsing is necessary. If you're lucky,
> somebody else already has a library to parse email addresses in this
> form.

I spent the afternoon digging around in BBDB internals.  They include a
function that parses the contents of a header, and handles commas
intelligently.  It replaces essentially all the code in my small patch,
so I won't respond to your detailed comments, much as I appreciated
them.

I have working code using bbdb-get-addresses, but I have a few questions
before I email the revised patch.

1) Is there a canonical list of header names used by notmuch?  I'm
trying to fill in this alist to match the BBDB interface.  It should be
fine to leave blank any that notmuch doesn't handle.

(defvar bbdb/notmuch-header-by-name
  ;; both are case sensitive
  '( ("From" . :From)
("To" . :To)
("CC" . :Cc)
("BCC" . :Bcc)
("Resent-From" . )
("Reply-To" . )
("Resent-To" . )
("Resent-CC" . ))
  "Alist for dispatching header symbols as used by notmuch-show-get-header
from strings as used by notmuch-show-get-header")

2) What's the consensus on naming conventions?  BBDB has functions for
other mail clients with names like bbdb/gnus-* and bbdb/vm-*.  I
imitated this convention, but maybe it's better to leave the bbdb/*
namespace to BBDB itself, and use a prefix like notmuch-bbdb-* or
notmuch-bbdb/* (Regardless, I moved everything to notmuch-address.el, as
suggested)

Thanks,
Daniel


Notmuch scripts (again), now with more usenet

2012-11-18 Thread Ethan Glasser-Camp
Jan Pobrislo  writes:

> Hi! I was having some hardware issues and had to migrate the site. It should
> be all up again and ready for inclusion.
>
> Added stuff from last time:
> * source function & actions for zaw (https://github.com/zsh-users/zaw)
> * LICENSE (CC0)

Hi! Sorry for the delay, real life got in the way.

In previous emails, David Bremner said he'd like someone with Python
expertise to look at notmuch-new.py. He notes that pylint goes
ballistic, but most of these errors don't bother me much (complaints
about variable naming and missing docstrings).

As for the rest, I'm not a zsh expert, but I looked at zmuch and
notmuch-colorize and they looked fine.

If Bremner is willing to put this package in contrib, I think he should
do so.

Ethan


[PATCH v2] emacs: display tags in notmuch-show with links

2012-11-18 Thread Ethan Glasser-Camp
Damien Cassou  writes:

> This patch obsoletes
> id:1352565719-12397-1-git-send-email-damien.cassou at gmail.com
>
> [PATCH 1/4] emacs: Add a thread's tags to emacs header-line
> [PATCH 2/4] emacs: Make tags in header-line clickable
> [PATCH 3/4] emacs: Make tags that appear in `notmuch-show' clickable
> [PATCH 4/4] emacs: Add unit-tests for clickable tags in notmuch-show
>
> This patch makes clickable all tags that appear in notmuch-show
> buffers. Each tag is a link to open a new notmuch-search buffer for
> this tag. Additionally, the buffer's header-line now shows the
> thread's tags (clickable only if the `header-button' library is loaded
> or loadable).

Looks fine to me. Let me just get the notes from my bikeshed, in case
you get asked to roll another version :)

- You might want to use #' on lambdas.

- It bothers me how similar notmuch-tagger-{body,header}-button-action
  are. I thought it might be better to unify them by seeing what type
  the button argument was. Here's my (untested) approach which you might
  find prettier or uglier.

(notmuch-tagger-all-button-get (button attrib)
  "Utility function to do button-get on different kinds of buttons."
  (cond
((integer-or-marker-p button)
  (button-get button attrib))
((and (featurep 'header-button)
  (listp button))
  (header-button-get button attrib))
(t (error "unknown type of button %s" button))

- The comment for notmuch-tagger-make-body-link reads that it will work
  "everywhere except in the header-line". Does this mean mode-line, menu
  bar, or what? How about just "won't work in the header-line"?

- In patch 3:

 +If tags the result of this function is to be used within the

I think this should just read "If the result".

Ethan


[PATCH 01/18] cli: add stub for insert command

2012-11-18 Thread Mark Walters

Hi

I have now been through essentially the whole series (except the tests)
and broadly like it. Just to summarise my concerns from the individual
replies here:

What should insert do when interrupted and is it safe? I am not
knowledgeable enough to be confident about that.

I think some of the talloc allocations need their return values
checked. I am more worried about this here then in most of the rest of
notmuch as we are writing to the database (and even to the mailstore
itself).

My other concerns (code duplication with notmuch-tag.c and possible
deadlock in the tmp file code and file overwriting) are easily fixable

Best wishes

Mark




On Wed, 25 Jul 2012, Peter Wang  wrote:
> This does nothing yet.
> ---
>  Makefile.local   |1 +
>  notmuch-client.h |3 +++
>  notmuch-insert.c |   27 +++
>  notmuch.c|3 +++
>  4 files changed, 34 insertions(+), 0 deletions(-)
>  create mode 100644 notmuch-insert.c
>
> diff --git a/Makefile.local b/Makefile.local
> index 296995d..950f046 100644
> --- a/Makefile.local
> +++ b/Makefile.local
> @@ -282,6 +282,7 @@ notmuch_client_srcs = \
>   notmuch-config.c\
>   notmuch-count.c \
>   notmuch-dump.c  \
> + notmuch-insert.c\
>   notmuch-new.c   \
>   notmuch-reply.c \
>   notmuch-restore.c   \
> diff --git a/notmuch-client.h b/notmuch-client.h
> index f930798..edbd3ee 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -132,6 +132,9 @@ int
>  notmuch_dump_command (void *ctx, int argc, char *argv[]);
>  
>  int
> +notmuch_insert_command (void *ctx, int argc, char *argv[]);
> +
> +int
>  notmuch_new_command (void *ctx, int argc, char *argv[]);
>  
>  int
> diff --git a/notmuch-insert.c b/notmuch-insert.c
> new file mode 100644
> index 000..0e061a0
> --- /dev/null
> +++ b/notmuch-insert.c
> @@ -0,0 +1,27 @@
> +/* notmuch - Not much of an email program, (just index and search)
> + *
> + * Copyright ? 2012 Peter Wang
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see http://www.gnu.org/licenses/ .
> + *
> + * Author: Peter Wang 
> + */
> +
> +#include "notmuch-client.h"
> +
> +int
> +notmuch_insert_command (void *ctx, int argc, char *argv[])
> +{
> +return 1;
> +}
> diff --git a/notmuch.c b/notmuch.c
> index 477a09c..86239fd 100644
> --- a/notmuch.c
> +++ b/notmuch.c
> @@ -53,6 +53,9 @@ static command_t commands[] = {
>  { "new", notmuch_new_command,
>"[options...]",
>"Find and import new messages to the notmuch database." },
> +{ "insert", notmuch_insert_command,
> +  "[options...] [--] [+|- ...] < message",
> +  "Add a new message into the maildir and notmuch database." },
>  { "search", notmuch_search_command,
>"[options...]  [...]",
>"Search for messages matching the given search terms." },
> -- 
> 1.7.4.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 05/18] insert: move file from Maildir tmp to new

2012-11-18 Thread Mark Walters
On Wed, 25 Jul 2012, Peter Wang  wrote:
> Atomically move the new message file from the Maildir 'tmp' directory
> to 'new'.
> ---
>  notmuch-insert.c |   18 ++
>  1 files changed, 18 insertions(+), 0 deletions(-)
>
> diff --git a/notmuch-insert.c b/notmuch-insert.c
> index 340f7e4..bab1fed 100644
> --- a/notmuch-insert.c
> +++ b/notmuch-insert.c
> @@ -75,6 +75,20 @@ maildir_open_tmp (void *ctx, const char *dir, char 
> **tmppath, char **newpath)
>  }
>  
>  static notmuch_bool_t
> +maildir_move_to_new (const char *tmppath, const char *newpath)
> +{
> +/* We follow the Dovecot recommendation to simply use rename()
> + * instead of link() and unlink().
> + */
> +if (rename (tmppath, newpath) == 0) {
> + return TRUE;
> +}

Do we want to overwrite an existing message with this name? As far as I
can see rename does overwrite and link would not: was that why rename is
better than link/unlink?

I would prefer not to overwrite but maybe there is a reason we need to. 
Would a possible alternative be to loop when finding a tmp file until
both the tmp file and the new file do not exist?

Best wishes

Mark


> +
> +fprintf (stderr, "Error: rename() failed: %s\n", strerror (errno));
> +return FALSE;
> +}
> +
> +static notmuch_bool_t
>  copy_fd_data (int fdin, int fdout)
>  {
>  char buf[4096];
> @@ -132,6 +146,10 @@ insert_message (void *ctx, notmuch_database_t *notmuch, 
> int fdin,
>  
>  close (fdout);
>  
> +if (ret) {
> + ret = maildir_move_to_new (tmppath, newpath);
> +}
> +
>  if (!ret) {
>   unlink (tmppath);
>  }
> -- 
> 1.7.4.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 16/18] insert: add --create-folder option

2012-11-18 Thread Mark Walters

Hi

On Wed, 25 Jul 2012, Peter Wang  wrote:
> Support an option to create a new folder in the maildir.
> ---
>  notmuch-insert.c |   71 
> ++
>  1 files changed, 71 insertions(+), 0 deletions(-)
>
> diff --git a/notmuch-insert.c b/notmuch-insert.c
> index a69dfe6..380c520 100644
> --- a/notmuch-insert.c
> +++ b/notmuch-insert.c
> @@ -48,6 +48,70 @@ check_folder_name (const char *folder)
>  }
>  }
>  
> +static int
> +mkdir_parents (void *ctx, const char *path, int mode)
> +{
> +struct stat st;
> +char *pathcopy;
> +char *start;
> +char *end;
> +int ret;
> +
> +/* First check the common case: directory already exists. */
> +if (stat (path, ) == 0) {
> + return (S_ISDIR (st.st_mode)) ? 0 : -1;
> +}
> +
> +pathcopy = talloc_strdup (ctx, path);
> +ret = 0;
> +
> +for (start = pathcopy; *start != '\0'; start = end + 1) {
> + end = strchr (start + 1, '/');
> + if (!end) {
> + ret = mkdir (path, mode);
> + break;
> + }
> + *end = '\0';
> + ret = mkdir (pathcopy, mode);
> + if (ret != 0 && errno != EEXIST) {
> + break;
> + }
> + *end = '/';
> +}
I am a little confused by this: why is there a +1 in the start = end + 1
line and in the end = strchr line? I believe it doesn't matter but it
makes me wonder if I am missing something.

Best wishes

Mark



> +
> +talloc_free (pathcopy);
> +
> +return ret;
> +}
> +
> +static notmuch_bool_t
> +maildir_create (void *ctx, const char *dir)
> +{
> +const int mode = 0755;
> +char *subdir;
> +char *end;
> +
> +/* Create 'cur' directory, including parent directories. */
> +subdir = talloc_asprintf (ctx, "%s/cur", dir);
> +if (mkdir_parents (ctx, subdir, mode) != 0)
> + return FALSE;
> +
> +end = subdir + strlen (subdir);
> +
> +/* Create 'new' directory. */
> +strcpy (end - 3, "new");
> +if (mkdir (subdir, mode) != 0 && errno != EEXIST)
> + return FALSE;
> +
> +/* Create 'tmp' directory. */
> +strcpy (end - 3, "tmp");
> +if (mkdir (subdir, mode) != 0 && errno != EEXIST)
> + return FALSE;
> +
> +talloc_free (subdir);
> +return TRUE;
> +}
> +
>  static notmuch_bool_t
>  safe_gethostname (char *hostname, size_t hostname_size)
>  {
> @@ -253,6 +317,7 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
>  const char **new_tags;
>  size_t new_tags_length;
>  const char *folder = NULL;
> +notmuch_bool_t create_folder = FALSE;
>  tag_operation_t *tag_ops;
>  int tag_ops_count = 0;
>  char *maildir;
> @@ -262,6 +327,7 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
>  
>  notmuch_opt_desc_t options[] = {
>   { NOTMUCH_OPT_STRING, , "folder", 0, 0 },
> + { NOTMUCH_OPT_BOOLEAN, _folder, "create-folder", 0, 0 },
>   { NOTMUCH_OPT_END, 0, 0, 0, 0 }
>  };
>  
> @@ -328,6 +394,11 @@ notmuch_insert_command (void *ctx, int argc, char 
> *argv[])
>   return 1;
>   }
>   maildir = talloc_asprintf (ctx, "%s/%s", db_path, folder);
> + if (create_folder && ! maildir_create (ctx, maildir)) {
> + fprintf (stderr, "Error: creating maildir %s: %s\n",
> +  maildir, strerror (errno));
> + return 1;
> + }
>  } else {
>   maildir = talloc_asprintf (ctx, "%s", db_path);
>  }
> -- 
> 1.7.4.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 14/18] man: document 'insert' command

2012-11-18 Thread Mark Walters
On Wed, 25 Jul 2012, Peter Wang  wrote:
> Add initial documentation for notmuch insert command.
> ---
>  man/Makefile.local|1 +
>  man/man1/notmuch-insert.1 |   50 
> +
>  2 files changed, 51 insertions(+), 0 deletions(-)
>  create mode 100644 man/man1/notmuch-insert.1
>
> diff --git a/man/Makefile.local b/man/Makefile.local
> index d43a949..f91bfd4 100644
> --- a/man/Makefile.local
> +++ b/man/Makefile.local
> @@ -12,6 +12,7 @@ MAN1 := \
>   $(dir)/man1/notmuch-count.1 \
>   $(dir)/man1/notmuch-dump.1 \
>   $(dir)/man1/notmuch-restore.1 \
> + $(dir)/man1/notmuch-insert.1 \
>   $(dir)/man1/notmuch-new.1 \
>   $(dir)/man1/notmuch-reply.1 \
>   $(dir)/man1/notmuch-search.1 \
> diff --git a/man/man1/notmuch-insert.1 b/man/man1/notmuch-insert.1
> new file mode 100644
> index 000..7d281b5
> --- /dev/null
> +++ b/man/man1/notmuch-insert.1
> @@ -0,0 +1,50 @@
> +.TH NOTMUCH-INSERT 1 2012-xx-xx "Notmuch 0.xx"
> +.SH NAME
> +notmuch-insert \- add a message to the maildir and notmuch database
> +.SH SYNOPSIS
> +
> +.B notmuch insert
> +.RI "[" options "]"
> +.RI "--"
> +.RI "[ +<" tag> "|\-<" tag "> ... ]"
> +
> +.SH DESCRIPTION
> +
> +.B notmuch insert
> +reads a message from standard input and delivers it to the specified
> +maildir folder, then incorporates the message into the notmuch
> +database.  It is an alternative to using a separate tool to deliver
> +the message then running
> +.B notmuch new
> +afterwards.
> +
> +The new message will be tagged with the tags specified by the
> +.B new.tags
> +configuration option.
> +Additional tagging operations may be specified on the command-line.
> +Tags prefixed by '+' are added while those prefixed by '\-' are
> +removed.  notmuch will warn about ambiguous tag removal arguments
> +which may be confused with option arguments.  It is recommended to use
> +the option list terminator "--" to avoid ambiguity.
> +
> +Supported options for
> +.B insert
> +include
> +.RS 4
> +.TP 4
> +.BI "--folder=<" folder ">"
> +
> +Deliver the message to the
> +.RI Maildir/ folder
> +directory, which must already exist.
> +The default is to deliver to the top-level directory.

I think this should say something about notmuch database root: so that
by default it delivers to the top-level directory of the notmuch
database, and otherwise is relative to that. Maybe just replace Maildir
by something like "" or something?

Best wishes

Mark



> +
> +.RE
> +.RE
> +.SH SEE ALSO
> +
> +\fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
> +\fBnotmuch-dump\fR(1), \fBnotmuch-hooks\fR(5), \fBnotmuch-reply\fR(1),
> +\fBnotmuch-restore\fR(1), \fBnotmuch-search\fR(1),
> +\fBnotmuch-search-terms\fR(7), \fBnotmuch-show\fR(1),
> +\fBnotmuch-tag\fR(1)
> -- 
> 1.7.4.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 10/18] insert: parse command-line tag operations

2012-11-18 Thread Mark Walters

Hi

On Wed, 25 Jul 2012, Peter Wang  wrote:
> Parse +tag and -tag on the 'insert' command-line.
> Issue a warning about ambiguous -tag arguments which don't follow
> +tag nor an explicit option list terminator.
> ---
>  notmuch-insert.c |   50 ++
>  1 files changed, 50 insertions(+), 0 deletions(-)
>
> diff --git a/notmuch-insert.c b/notmuch-insert.c
> index 4fb3ea3..6db03e3 100644
> --- a/notmuch-insert.c
> +++ b/notmuch-insert.c
> @@ -24,6 +24,11 @@
>  #include 
>  #include 
>  
> +typedef struct {
> +const char *tag;
> +notmuch_bool_t remove;
> +} tag_operation_t;
> +
>  static notmuch_bool_t
>  check_folder_name (const char *folder)
>  {
> @@ -236,8 +241,11 @@ notmuch_insert_command (void *ctx, int argc, char 
> *argv[])
>  const char **new_tags;
>  size_t new_tags_length;
>  const char *folder = NULL;
> +tag_operation_t *tag_ops;
> +int tag_ops_count = 0;
>  char *maildir;
>  int opt_index;
> +notmuch_bool_t warn_tag_rem;
>  notmuch_bool_t ret;
>  
>  notmuch_opt_desc_t options[] = {
> @@ -253,6 +261,48 @@ notmuch_insert_command (void *ctx, int argc, char 
> *argv[])
>   return 1;
>  }
>  
> +if (opt_index > 0 && strcmp (argv[opt_index - 1], "--") == 0) {
> + warn_tag_rem = FALSE;
> +} else {
> + warn_tag_rem = TRUE;
> +}
> +
> +/* Array of tagging operations (add or remove), terminated with an
> + * empty element. */
> +tag_ops = talloc_array (ctx, tag_operation_t, argc - opt_index + 1);
> +if (tag_ops == NULL) {
> + fprintf (stderr, "Out of memory.\n");
> + return 1;
> +}
> +
> +for (; opt_index < argc; opt_index++) {
> + if (argv[opt_index][0] == '+') {
> + tag_ops[tag_ops_count].tag = argv[opt_index] + 1;
> + tag_ops[tag_ops_count].remove = FALSE;
> + tag_ops_count++;
> + warn_tag_rem = FALSE;
> + } else if (argv[opt_index][0] == '-') {
> + if (warn_tag_rem) {
> + fprintf (stderr,
> +  "Warning: ambiguous argument treated as tag removal: 
> %s\n",
> +  argv[opt_index]);
> + }
> + tag_ops[tag_ops_count].tag = argv[opt_index] + 1;
> + tag_ops[tag_ops_count].remove = TRUE;
> + tag_ops_count++;
> + } else {
> + break;
> + }
> +}

I don't like the code duplication between this and notmuch-tag.c. In
particular the two (now) differ on how they deal with malformed tags
etc. Would it be possible to unify them?

Best wishes

Mark

> +
> +tag_ops[tag_ops_count].tag = NULL;
> +
> +if (opt_index != argc) {
> + fprintf (stderr, "Error: bad argument to notmuch insert: %s\n",
> +  argv[opt_index]);
> + return 1;
> +}
> +
>  config = notmuch_config_open (ctx, NULL, NULL);
>  if (config == NULL)
>   return 1;
> -- 
> 1.7.4.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


Add new dump/restore format and batch tagging.

2012-11-18 Thread Ethan Glasser-Camp
Ethan Glasser-Camp  writes:

> - Patch 4 still has a subject line that ends in a period. I don't think
>   this is mandatory for everyone but some people consider it best
>   practice.

Best practice, of course, would be to remove the period at the end of
the subject line. Patch 12 also has one.

Ethan


[PATCH 06/18] insert: add new message to database

2012-11-18 Thread Mark Walters

Hi

On Wed, 25 Jul 2012, Peter Wang  wrote:
> Add the new message to the notmuch database, renaming the file to encode
> notmuch tags as maildir flags.
> ---
>  notmuch-insert.c |   44 
>  1 files changed, 44 insertions(+), 0 deletions(-)
>
> diff --git a/notmuch-insert.c b/notmuch-insert.c
> index bab1fed..dd449bc 100644
> --- a/notmuch-insert.c
> +++ b/notmuch-insert.c
> @@ -129,6 +129,42 @@ copy_fd_data (int fdin, int fdout)
>  }
>  
>  static notmuch_bool_t
> +save_database (notmuch_database_t *notmuch, const char *path)

I would prefer a different name here: save_database suggests to me that
it is a "whole database" operation, whereas it is actually just adding a
single message. Maybe add_message or notmuch_insert_add_message?

Best wishes

Mark

> +{
> +notmuch_message_t *message;
> +notmuch_status_t status;
> +
> +status = notmuch_database_add_message (notmuch, path, );
> +switch (status) {
> +case NOTMUCH_STATUS_SUCCESS:
> + break;
> +case NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID:
> + fprintf (stderr, "Warning: duplicate message.\n");
> + break;
> +default:
> +case NOTMUCH_STATUS_FILE_NOT_EMAIL:
> +case NOTMUCH_STATUS_READ_ONLY_DATABASE:
> +case NOTMUCH_STATUS_XAPIAN_EXCEPTION:
> +case NOTMUCH_STATUS_OUT_OF_MEMORY:
> +case NOTMUCH_STATUS_FILE_ERROR:
> +case NOTMUCH_STATUS_NULL_POINTER:
> +case NOTMUCH_STATUS_TAG_TOO_LONG:
> +case NOTMUCH_STATUS_UNBALANCED_FREEZE_THAW:
> +case NOTMUCH_STATUS_UNBALANCED_ATOMIC:
> +case NOTMUCH_STATUS_LAST_STATUS:
> + fprintf (stderr, "Error: failed to add `%s' to notmuch database: %s\n",
> +  path, notmuch_status_to_string (status));
> + return FALSE;
> +}
> +
> +notmuch_message_tags_to_maildir_flags (message);
> +
> +notmuch_message_destroy (message);
> +
> +return TRUE;
> +}
> +
> +static notmuch_bool_t
>  insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,
>   const char *dir)
>  {
> @@ -152,6 +188,14 @@ insert_message (void *ctx, notmuch_database_t *notmuch, 
> int fdin,
>  
>  if (!ret) {
>   unlink (tmppath);
> + return FALSE;
> +}
> +
> +ret = save_database (notmuch, newpath);
> +
> +if (!ret) {
> + /* XXX maybe there should be an option to keep the file in maildir? */
> + unlink (newpath);
>  }
>  
>  return ret;
> -- 
> 1.7.4.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


Add new dump/restore format and batch tagging.

2012-11-18 Thread Ethan Glasser-Camp
david at tethera.net writes:

> which was revied by Tomi and Ethan. I think I implemented their
> suggestions.

Actually, I don't think you implemented all of mine.

- Patch 4 still has a subject line that ends in a period. I don't think
  this is mandatory for everyone but some people consider it best
  practice. You still have the spelling "seperate" (also, patch 7 has
  "seperated").

- In patch 4, I still think this would look better if you switched the
  branches.

+if (private_status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) {
+   _notmuch_message_add_term (message, "type", "mail");
+} else {
+   return NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID;
+}

- In patch 5, I still think this looks funny:

+   int num_tags = random () % (max_tags + 1);
+   int this_mid_len = random () % message_id_len + 1;

Additionally, I would like a check that message_id_len is reasonable
(more than 1, say).

- Patch 8:

+int
+parse_tag_stream (void *ctx,
+ notmuch_database_t *notmuch,
+ FILE *input,
+ tag_callback_t callback,
+ tag_op_flag_t flags,
+ volatile sig_atomic_t *interrupted);

Am I going crazy, or does this function not get implemented?

- Patch 11: DUMP_FORMAT can change from DUMP_FORMAT_BATCH_TAG to
  DUMP_FORMAT_SUP if a paren is anywhere on the first line. I'd prefer
  this only happens if we have DUMP_FORMAT_AUTO.

You probably want to move the comment "Dump output is..." closer to the
regex.

I don't see why it's necessary to have this:

+   query_string = query_string + 3;

- Patch 13:

+cp /dev/null EXPECTED.$test_count
+notmuch dump --format=batch-tag -- from:cworth |\
+awk "{ print \"+$enc1 +$enc2 +$enc3 -- \" \$5 }" > EXPECTED.$test_count

What's the purpose of the CP here? It just creates an empty file. You
could do it with touch. Why even bother since you're going to fill it
with stuff in a second? Actually, care to explain the dump and sed call?
It looks like you're using this dump to encode the message IDs. If
format=batch-tag skips a message for some reason or terminates early,
the test won't fail.

- Patch 16:

+message-ids may contained arbitrary non-null characters. Note also

I think this should be "may contain", or something else entirely if
you're trying to describe past behavior of sup?

Ethan


[PATCH 03/18] insert: open Maildir tmp file

2012-11-18 Thread Mark Walters

Hi

I know this has been around for a long time but it still applies so I
will try and review it. I have been using it somewhat and all seems to
work.

As an aside I have a postpone/resume implementation based on top
of it.

One general remark is that I think more comments could be helpful.

On Wed, 25 Jul 2012, Peter Wang  wrote:
> Open a unique file in the Maildir tmp directory.
> The new message is not yet copied into the file.
> ---
>  notmuch-insert.c |   80 
> +-
>  1 files changed, 79 insertions(+), 1 deletions(-)
>
> diff --git a/notmuch-insert.c b/notmuch-insert.c
> index 21424cf..f01a6f2 100644
> --- a/notmuch-insert.c
> +++ b/notmuch-insert.c
> @@ -20,12 +20,86 @@
>  
>  #include "notmuch-client.h"
>  
> +#include 
> +#include 
> +#include 
> +
> +static notmuch_bool_t
> +safe_gethostname (char *hostname, size_t hostname_size)
> +{
> +if (gethostname (hostname, hostname_size) == -1) {
> + strncpy (hostname, "unknown", hostname_size);
> +}
> +hostname[hostname_size - 1] = '\0';
> +
> +return (strchr (hostname, '/') == NULL);
> +}

Here is an example: a comment saying "like gethostname but guarantees
that hostname is  null terminated. Returns true unless hostname contains
a /.  

Also I found hostname_size a confusing term as it the size of the buffer
which is allowed to contain hostname rather than the size of
hostname. Maybe hostname_buffer_size or just len?

> +
> +static int
> +maildir_open_tmp (void *ctx, const char *dir, char **tmppath, char **newpath)
> +{
> +pid_t pid;
> +char hostname[256];
> +struct timeval tv;
> +char *filename;
> +int fd = -1;
> +
> +/* This is the file name format used by Dovecot. */
> +pid = getpid ();
> +if (! safe_gethostname (hostname, sizeof (hostname))) {
> + fprintf (stderr, "Error: invalid host name.\n");
> + return -1;
> +}
> +gettimeofday (, NULL);
> +filename = talloc_asprintf (ctx, "%ld.M%ldP%d.%s",
> + tv.tv_sec, tv.tv_usec, pid, hostname);
> +
> +*tmppath = talloc_asprintf (ctx, "%s/tmp/%s", dir, filename);

Does there need to be rather more error checking after talloc
operations? Eg what if this allocation fails?

> +
> +do {
> + fd = open (*tmppath, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, 0666);
> +} while (fd == -1 && errno == EEXIST);

Am I misunderstanding or does this deadlock if the file *tmppath exists?
Do you want to recalculate tmppath inside the do while loop?

Also is 0666 standard or would 0644 or 0600 be better?

Best wishes

Mark

> +
> +if (fd != -1) {
> + *newpath = talloc_asprintf (ctx, "%s/new/%s", dir, filename);
> +}
> +else {
> + fprintf (stderr, "Error: opening %s: %s\n",
> +  *tmppath, strerror (errno));
> + talloc_free (*tmppath);
> +}
> +
> +talloc_free (filename);
> +
> +return fd;
> +}
> +
> +static notmuch_bool_t
> +insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,
> + const char *dir)
> +{
> +char *tmppath;
> +char *newpath;
> +int fdout;
> +
> +fdout = maildir_open_tmp (ctx, dir, , );
> +if (fdout < 0) {
> + return FALSE;
> +}
> +
> +close (fdout);
> +unlink (tmppath);
> +return FALSE;
> +}
> +
>  int
>  notmuch_insert_command (void *ctx, int argc, char *argv[])
>  {
>  notmuch_config_t *config;
>  notmuch_database_t *notmuch;
>  const char *db_path;
> +char *maildir;
> +notmuch_bool_t ret;
>  
>  config = notmuch_config_open (ctx, NULL, NULL);
>  if (config == NULL)
> @@ -33,11 +107,15 @@ notmuch_insert_command (void *ctx, int argc, char 
> *argv[])
>  
>  db_path = notmuch_config_get_database_path (config);
>  
> +maildir = talloc_asprintf (ctx, "%s", db_path);
> +
>  if (notmuch_database_open (notmuch_config_get_database_path (config),
>  NOTMUCH_DATABASE_MODE_READ_WRITE, ))
>   return 1;
>  
> +ret = insert_message (ctx, notmuch, STDIN_FILENO, maildir);
> +
>  notmuch_database_destroy (notmuch);
>  
> -return 1;
> +return (ret) ? 0 : 1;
>  }
> -- 
> 1.7.4.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 2/2] emacs: less guessing of character set in messages

2012-11-18 Thread Ethan Glasser-Camp
Tomi Ollila  writes:

> The macro with-current-notmuch-show-message executes command
> `notmuch show --format=raw id:...` which just outputs the contents
> of the mail file verbatim (into temporary buffer). In case e.g. utf-8
> locale is used the temporary buffer has buffer-file-coding-system as
> utf-8. In this case Emacs converts the data to multibyte format, guessing
> that input is in utf-8.
> However, the "raw" (MIME) message may contain octet data in any other
> 8bit format, and as no (MIME-)content spesific handling to the message
> is done at this point, conversion to other formats may lose information.
> By setting coding-system-for-read 'no-conversion drops the conversion part
> and makes this handle input as notmuch-get-bodypart-internal() does.
> This marks the broken test in previous change fixed.

This looks good to me, though you might need to apply it with notmuch
show --format="mbox" :).

Ethan


[PATCH 16/16] notmuch-{dump,restore}.1: document new format options

2012-11-18 Thread da...@tethera.net
From: David Bremner 

More or less arbitrarily, notmuch-dump.1 gets the more detailed
description of the format.
---
 man/man1/notmuch-dump.1|   58 +++
 man/man1/notmuch-restore.1 |   59 +++-
 2 files changed, 111 insertions(+), 6 deletions(-)

diff --git a/man/man1/notmuch-dump.1 b/man/man1/notmuch-dump.1
index 230deec..b57c2c1 100644
--- a/man/man1/notmuch-dump.1
+++ b/man/man1/notmuch-dump.1
@@ -5,6 +5,7 @@ notmuch-dump \- creates a plain-text dump of the tags of each 
message
 .SH SYNOPSIS

 .B "notmuch dump"
+.RB  [ "\-\-format=(sup|batch-tag)"  "] [--]"
 .RI "[ --output=<" filename "> ] [--]"
 .RI "[ <" search-term ">...]"

@@ -19,6 +20,63 @@ recreated from the messages themselves.  The output of 
notmuch dump is
 therefore the only critical thing to backup (and much more friendly to
 incremental backup than the native database files.)

+.TP 4
+.B \-\-format=(sup|batch-tag)
+
+Notmuch restore supports two plain text dump formats, both with one message-id
+per line, followed by a list of tags.
+
+.RS 4
+.TP 4
+.B sup
+
+The
+.B sup
+dump file format is specifically chosen to be
+compatible with the format of files produced by sup-dump.
+So if you've previously been using sup for mail, then the
+.B "notmuch restore"
+command provides you a way to import all of your tags (or labels as
+sup calls them).
+Each line has the following form
+
+.RS 4
+.RI < message-id >
+.B (
+.RI < tag "> ..."
+.B )
+
+with zero or more tags are separated by spaces. Note that (malformed)
+message-ids may contained arbitrary non-null characters. Note also
+that tags with spaces will not be correctly restored with this format.
+
+.RE
+
+.RE
+.RS 4
+.TP 4
+.B batch-tag
+
+The
+.B batch-tag
+dump format is intended to more robust against malformed message-ids
+and tags containing whitespace or non-\fBascii\fR(7) characters.
+Each line has the form
+
+.RS 4
+.RI "+<" "encoded-tag" "> " "" "+<" "encoded-tag" "> ... -- " "" " <" 
encoded-message-id >
+
+where encoded means that every byte not matching the regex
+.B [A-Za-z0-9+-_@=.:,]
+is replace by
+.B %nn
+where nn is the two digit hex encoding.
+The astute reader will notice this is a special case of the batch input
+format for \fBnotmuch-tag\fR(1).
+
+.RE
+
+
 With no search terms, a dump of all messages in the database will be
 generated.  A "--" argument instructs notmuch that the
 remaining arguments are search terms.
diff --git a/man/man1/notmuch-restore.1 b/man/man1/notmuch-restore.1
index 2fa8733..3860829 100644
--- a/man/man1/notmuch-restore.1
+++ b/man/man1/notmuch-restore.1
@@ -6,6 +6,7 @@ notmuch-restore \- restores the tags from the given file (see 
notmuch dump)

 .B "notmuch restore"
 .RB [ "--accumulate" ]
+.RB [ "--format=(auto|batch-tag|sup)" ]
 .RI "[ --input=<" filename "> ]"

 .SH DESCRIPTION
@@ -15,19 +16,51 @@ Restores the tags from the given file (see

 The input is read from the given filename, if any, or from stdin.

-Note: The dump file format is specifically chosen to be
+
+Supported options for
+.B restore
+include
+.RS 4
+.TP 4
+.B \-\-accumulate
+
+The union of the existing and new tags is applied, instead of
+replacing each message's tags as they are read in from the dump file.
+
+.RE
+.RS 4
+.TP 4
+.B \-\-format=(sup|batch-tag|auto)
+
+Notmuch restore supports two plain text dump formats, with one message-id
+per line, and a list of tags.
+For details of the actual formats, see \fBnotmuch-dump\fR(1).
+
+.RS 4
+.TP 4
+.B sup
+
+The
+.B sup
+dump file format is specifically chosen to be
 compatible with the format of files produced by sup-dump.
 So if you've previously been using sup for mail, then the
 .B "notmuch restore"
 command provides you a way to import all of your tags (or labels as
 sup calls them).

-The --accumulate switch causes the union of the existing and new tags to be
-applied, instead of replacing each message's tags as they are read in from the
-dump file.
+.RE
+.RS 4
+.TP 4
+.B batch-tag

-See \fBnotmuch-search-terms\fR(7)
-for details of the supported syntax for .
+The
+.B batch-tag
+dump format is intended to more robust against malformed message-ids
+and tags containing whitespace or non-\fBascii\fR(7) characters.  This
+format hex-escapes all characters those outside of a small character
+set, intended to be suitable for e.g. pathnames in most UNIX-like
+systems.

 .B "notmuch restore"
 updates the maildir flags according to tag changes if the
@@ -36,6 +69,20 @@ configuration option is enabled. See \fBnotmuch-config\fR(1) 
for
 details.

 .RE
+
+.RS 4
+.TP 4
+.B auto
+
+This option (the default) tries to guess the format from the
+input. For correctly formed input in either supported format, this
+heuristic, based the fact that batch-tag format contains no parentheses,
+should be accurate.
+
+.RE
+
+.RE
+
 .SH SEE ALSO

 \fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
-- 
1.7.10.4



[PATCH 15/16] man: document notmuch tag --batch, --input options

2012-11-18 Thread da...@tethera.net
From: Jani Nikula 

---
 man/man1/notmuch-tag.1 |   52 +++-
 1 file changed, 51 insertions(+), 1 deletion(-)

diff --git a/man/man1/notmuch-tag.1 b/man/man1/notmuch-tag.1
index 0f86582..751db7b 100644
--- a/man/man1/notmuch-tag.1
+++ b/man/man1/notmuch-tag.1
@@ -4,7 +4,12 @@ notmuch-tag \- add/remove tags for all messages matching the 
search terms

 .SH SYNOPSIS
 .B notmuch tag
-.RI  "+<" tag> "|\-<" tag "> [...] [\-\-] <" search-term ">..."
+.RI "+<" tag ">|\-<" tag "> [...] [\-\-] <" search-terms ">"
+
+.B notmuch tag
+.RI "--batch"
+.RI "[ --input=<" filename "> ]"
+

 .SH DESCRIPTION

@@ -29,6 +34,51 @@ updates the maildir flags according to tag changes if the
 configuration option is enabled. See \fBnotmuch-config\fR(1) for
 details.

+Supported options for
+.B tag
+include
+.RS 4
+.TP 4
+.BR \-\-batch
+
+Read batch tagging operations from standard input. This is more
+efficient than repeated
+.B notmuch tag
+invocations. See
+.B TAG FILE FORMAT
+below for the input format. This option is not compatible with
+specifying tagging on the command line.
+.RE
+
+.RS 4
+.TP 4
+.BR "\-\-input=" 
+
+Read input from given file, instead of from stdin. Implies
+.BR --batch .
+
+.SH TAG FILE FORMAT
+
+The input must consist of lines of the format:
+
+.RI "T +<" tag ">|\-<" tag "> [...] [\-\-] <" search-terms ">"
+
+Each line is interpreted similarly to
+.B notmuch tag
+command line arguments. The delimiter is one or more spaces ' '. Any
+characters in  and 
+.B may
+be hex encoded with %NN where NN is the hexadecimal value of the
+character. Any ' ' and '%' characters in  and 
+.B must
+be hex encoded (using %20 and %25, respectively). Any characters that
+are not part of  or 
+.B must not
+be hex encoded.
+
+Leading and trailing space ' ' is ignored. Empty lines and lines
+beginning with '#' are ignored.
+
 .SH SEE ALSO

 \fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
-- 
1.7.10.4



[PATCH 14/16] tag-util: optimization of tag application

2012-11-18 Thread da...@tethera.net
From: David Bremner 

The idea is not to bother with restore operations if they don't change
the set of tags. This is actually a relatively common case.

In order to avoid fancy datastructures, this method is quadratic in
the number of tags; at least on my mail database this doesn't seem to
be a big problem.
---
 notmuch-tag.c |2 +-
 tag-util.c|   59 +
 tag-util.h|3 ++-
 3 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/notmuch-tag.c b/notmuch-tag.c
index ca120d5..33cf78d 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -140,7 +140,7 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const 
char *query_string,
 notmuch_messages_valid (messages) && ! interrupted;
 notmuch_messages_move_to_next (messages)) {
message = notmuch_messages_get (messages);
-   tag_op_list_apply (message, tag_ops, flags);
+   tag_op_list_apply (message, tag_ops, flags | TAG_FLAG_PRE_OPTIMIZED);
notmuch_message_destroy (message);
 }

diff --git a/tag-util.c b/tag-util.c
index 5329b1f..de0928b 100644
--- a/tag-util.c
+++ b/tag-util.c
@@ -117,6 +117,62 @@ message_error (notmuch_message_t *message,
 fprintf (stderr, "Status: %s\n", notmuch_status_to_string (status));
 }

+static int
+makes_changes (notmuch_message_t *message,
+  tag_op_list_t *list,
+  tag_op_flag_t flags)
+{
+
+int i;
+
+notmuch_tags_t *tags;
+notmuch_bool_t changes = FALSE;
+
+/* First, do we delete an existing tag? */
+changes = FALSE;
+for (tags = notmuch_message_get_tags (message);
+! changes && notmuch_tags_valid (tags);
+notmuch_tags_move_to_next (tags)) {
+   const char *cur_tag = notmuch_tags_get (tags);
+   int last_op =  (flags & TAG_FLAG_REMOVE_ALL) ? -1 : 0;
+
+   for (i = 0; i < list->count; i++) {
+   if (strcmp (cur_tag, list->ops[i].tag) == 0) {
+   last_op = list->ops[i].remove ? -1 : 1;
+   }
+   }
+
+   changes = (last_op == -1);
+}
+notmuch_tags_destroy (tags);
+
+if (changes)
+   return TRUE;
+
+/* Now check for adding new tags */
+for (i = 0; i < list->count; i++) {
+   notmuch_bool_t exists = FALSE;
+
+   for (tags = notmuch_message_get_tags (message);
+notmuch_tags_valid (tags);
+notmuch_tags_move_to_next (tags)) {
+   const char *cur_tag = notmuch_tags_get (tags);
+   if (strcmp (cur_tag, list->ops[i].tag) == 0) {
+   exists = TRUE;
+   break;
+   }
+   }
+   notmuch_tags_destroy (tags);
+
+   /* the following test is conservative, it's ok to think we
+* make changes when we don't */
+   if ( ! exists && ! list->ops[i].remove )
+   return TRUE;
+}
+return FALSE;
+
+}
+
 notmuch_status_t
 tag_op_list_apply (notmuch_message_t *message,
   tag_op_list_t *list,
@@ -127,6 +183,9 @@ tag_op_list_apply (notmuch_message_t *message,
 notmuch_status_t status = 0;
 tag_operation_t *tag_ops = list->ops;

+if (! (flags & TAG_FLAG_PRE_OPTIMIZED) && ! makes_changes (message, list, 
flags))
+   return NOTMUCH_STATUS_SUCCESS;
+
 status = notmuch_message_freeze (message);
 if (status) {
message_error (message, status, "freezing message");
diff --git a/tag-util.h b/tag-util.h
index b381b8e..998c1b9 100644
--- a/tag-util.h
+++ b/tag-util.h
@@ -19,7 +19,8 @@ typedef struct {
 /* Use powers of 2 */
 typedef enum  { TAG_FLAG_NONE = 0,
TAG_FLAG_MAILDIR_SYNC = 1,
-   TAG_FLAG_REMOVE_ALL = 2 } tag_op_flag_t;
+   TAG_FLAG_REMOVE_ALL = 2,
+   TAG_FLAG_PRE_OPTIMIZED = 4 } tag_op_flag_t;


 typedef int (*tag_callback_t)(void *ctx,
-- 
1.7.10.4



[PATCH 13/16] test: second set of dump/restore --format=batch-tag tests

2012-11-18 Thread da...@tethera.net
From: David Bremner 

These one need the completed functionality in notmuch-restore. Fairly
exotic tags are tested, but no weird message id's.

We test each possible input to autodetection, both explicit (with
--format=auto) and implicit (without --format).
---
 test/dump-restore |   73 +
 1 file changed, 73 insertions(+)

diff --git a/test/dump-restore b/test/dump-restore
index e08b656..7b67d03 100755
--- a/test/dump-restore
+++ b/test/dump-restore
@@ -85,6 +85,79 @@ test_begin_subtest "dump --output=outfile -- from:cworth"
 notmuch dump --output=dump-outfile-dash-inbox.actual -- from:cworth
 test_expect_equal_file dump-cworth.expected dump-outfile-dash-inbox.actual

+test_begin_subtest "Check for a safe set of message-ids"
+notmuch search --output=messages from:cworth > EXPECTED.$test_count
+notmuch search --output=messages from:cworth |\
+   $TEST_DIRECTORY/hex-xcode --direction=encode > OUTPUT.$test_count
+test_expect_equal_file OUTPUT.$test_count EXPECTED.$test_count
+
+test_begin_subtest "format=batch-tag, # round-trip"
+notmuch dump --format=sup | sort > EXPECTED.$test_count
+notmuch dump --format=batch-tag | notmuch restore --format=batch-tag
+notmuch dump --format=sup | sort > OUTPUT.$test_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+
+tag1='comic_swear=$&^%$^%\\//-+$^%$'
+enc1=$($TEST_DIRECTORY/hex-xcode --direction=encode "$tag1")
+
+tag2=$(printf 'this\n tag\t has\n spaces')
+enc2=$($TEST_DIRECTORY/hex-xcode --direction=encode "$tag2")
+
+enc3='%c3%91%c3%a5%c3%b0%c3%a3%c3%a5%c3%a9-%c3%8f%c3%8a'
+tag3=$($TEST_DIRECTORY/hex-xcode --direction=decode $enc3)
+
+notmuch dump --format=batch-tag > BACKUP
+
+notmuch tag +"$tag1" +"$tag2" +"$tag3" -inbox -unread "*"
+
+test_begin_subtest 'format=batch-tag, round trip with strange tags'
+notmuch dump --format=batch-tag > EXPECTED.$test_count
+notmuch dump --format=batch-tag | notmuch restore --format=batch-tag
+notmuch dump --format=batch-tag > OUTPUT.$test_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+
+test_begin_subtest 'format=batch-tag, checking encoded output'
+cp /dev/null EXPECTED.$test_count
+notmuch dump --format=batch-tag -- from:cworth |\
+awk "{ print \"+$enc1 +$enc2 +$enc3 -- \" \$5 }" > EXPECTED.$test_count
+
+notmuch dump --format=batch-tag -- from:cworth  > OUTPUT.$test_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+
+test_begin_subtest 'restoring sane tags'
+notmuch restore --format=batch-tag < BACKUP
+notmuch dump --format=batch-tag > OUTPUT.$test_count
+test_expect_equal_file BACKUP OUTPUT.$test_count
+
+test_begin_subtest 'format=batch-tag, restore=auto'
+notmuch dump --format=batch-tag > EXPECTED.$test_count
+notmuch tag -inbox -unread "*"
+notmuch restore --format=auto < EXPECTED.$test_count
+notmuch dump --format=batch-tag > OUTPUT.$test_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+
+test_begin_subtest 'format=sup, restore=auto'
+notmuch dump --format=sup > EXPECTED.$test_count
+notmuch tag -inbox -unread "*"
+notmuch restore --format=auto < EXPECTED.$test_count
+notmuch dump --format=sup > OUTPUT.$test_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+
+test_begin_subtest 'format=batch-tag, restore=default'
+notmuch dump --format=batch-tag > EXPECTED.$test_count
+notmuch tag -inbox -unread "*"
+notmuch restore < EXPECTED.$test_count
+notmuch dump --format=batch-tag > OUTPUT.$test_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+
+test_begin_subtest 'format=sup, restore=default'
+notmuch dump --format=sup > EXPECTED.$test_count
+notmuch tag -inbox -unread "*"
+notmuch restore < EXPECTED.$test_count
+notmuch dump --format=sup > OUTPUT.$test_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+
+notmuch dump --format=batch-tag --output=save.tags

 test_expect_success 'roundtripping random message-ids and tags' \
 '${TEST_DIRECTORY}/random-corpus --num-messages=100 
--config-path=${NOTMUCH_CONFIG} &&
-- 
1.7.10.4



[PATCH 12/16] test: update dump-restore roundtripping test for batch-tag format.

2012-11-18 Thread da...@tethera.net
From: David Bremner 

Now we can actually round trip these crazy tags and and message ids.
hex-xcode is no longer needed as it's built in.
---
 test/dump-restore |   13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/test/dump-restore b/test/dump-restore
index a2204fb..e08b656 100755
--- a/test/dump-restore
+++ b/test/dump-restore
@@ -85,13 +85,14 @@ test_begin_subtest "dump --output=outfile -- from:cworth"
 notmuch dump --output=dump-outfile-dash-inbox.actual -- from:cworth
 test_expect_equal_file dump-cworth.expected dump-outfile-dash-inbox.actual

+
 test_expect_success 'roundtripping random message-ids and tags' \
-'test_subtest_known_broken &&
- ${TEST_DIRECTORY}/random-corpus --num-messages=10 
--config-path=${NOTMUCH_CONFIG} &&
- notmuch dump | ${TEST_DIRECTORY}/hex-xcode --direction=encode > 
EXPECTED.$test_count &&
+'${TEST_DIRECTORY}/random-corpus --num-messages=100 
--config-path=${NOTMUCH_CONFIG} &&
+ notmuch dump --format=batch-tag | sort > EXPECTED.$test_count &&
  notmuch tag -random-corpus tag:random-corpus &&
- ${TEST_DIRECTORY}/hex-xcode --direction=decode < EXPECTED.$test_count | 
notmuch restore 2>/dev/null &&
- notmuch dump | ${TEST_DIRECTORY}/hex-xcode --direction=encode > 
OUTPUT.$test_count &&
+ notmuch restore --format=batch-tag --input=EXPECTED.$test_count &&
+ notmuch dump --format=batch-tag | sort > OUTPUT.$test_count &&
  test_cmp EXPECTED.$test_count OUTPUT.$test_count'
-
 test_done
+
+# Note the database is "poisoned" for sup format at this point.
-- 
1.7.10.4



[PATCH 11/16] notmuch-restore: add support for input format 'batch-tag'

2012-11-18 Thread da...@tethera.net
From: David Bremner 

This is the same as the batch input for notmuch tag, except by default
it removes all tags before modifying a given message id and only "id:"
is supported.
---
 notmuch-restore.c |  180 +++--
 1 file changed, 105 insertions(+), 75 deletions(-)

diff --git a/notmuch-restore.c b/notmuch-restore.c
index f03dcac..1776e99 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -19,18 +19,21 @@
  */

 #include "notmuch-client.h"
+#include "dump-restore-private.h"
+#include "tag-util.h"
+
+static volatile sig_atomic_t interrupted;
+static regex_t regex;

 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)
+tag_message (unused (void *ctx),
+notmuch_database_t *notmuch,
+const char *message_id,
+tag_op_list_t *tag_ops,
+tag_op_flag_t 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, );
@@ -44,55 +47,46 @@ tag_message (notmuch_database_t *notmuch, const char 
*message_id,

 /* 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 ( (flags & TAG_FLAG_REMOVE_ALL) || ! (tag_op_list_empty (tag_ops)))
+   tag_op_list_apply (message, tag_ops, flags);

-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;
+if (message)
+   notmuch_message_destroy (message);

-notmuch_message_freeze (message);
+return ret;
+}

-if (remove_all)
-   notmuch_message_remove_all_tags (message);
+static int
+parse_sup_line (void *ctx, char *line,
+   char **query_str, tag_op_list_t *tag_ops)
+{

-next = file_tags;
-while (next) {
-   tag = strsep (, " ");
-   if (*tag == '\0')
-   continue;
-   status = notmuch_message_add_tag (message, tag);
-   if (status) {
-   fprintf (stderr, "Error applying tag %s to message %s:\n",
-tag, message_id);
-   fprintf (stderr, "%s\n", notmuch_status_to_string (status));
-   ret = 1;
-   }
-}
+regmatch_t match[3];
+char *file_tags;
+int rerr;

-notmuch_message_thaw (message);
+tag_op_list_reset (tag_ops);

-if (synchronize_flags)
-   notmuch_message_tags_to_maildir_flags (message);
+chomp_newline (line);

-  DONE:
-if (message)
-   notmuch_message_destroy (message);
+/* Silently ignore blank lines */
+if (line[0] == '\0') {
+   return 1;
+}

-return ret;
+rerr = xregexec (, line, 3, match, 0);
+if (rerr == REG_NOMATCH) {
+   fprintf (stderr, "Warning: Ignoring invalid input line: %s\n",
+line);
+   return 1;
+}
+
+*query_str = talloc_strndup (ctx, line + match[1].rm_so,
+match[1].rm_eo - match[1].rm_so);
+file_tags = talloc_strndup (ctx, line + match[2].rm_so,
+   match[2].rm_eo - match[2].rm_so);
+
+return tag_op_list_from_string (ctx, file_tags, FALSE, tag_ops);
 }

 int
@@ -100,16 +94,19 @@ notmuch_restore_command (unused (void *ctx), int argc, 
char *argv[])
 {
 notmuch_config_t *config;
 notmuch_database_t *notmuch;
-notmuch_bool_t synchronize_flags;
 notmuch_bool_t accumulate = FALSE;
+tag_op_flag_t flags = 0;
+tag_op_list_t *tag_ops;
+
 char *input_file_name = NULL;
 FILE *input = stdin;
 char *line = NULL;
 size_t line_size;
 ssize_t line_len;
-regex_t regex;
-int rerr;
+
+int ret = 0;
 int opt_index;
+int input_format = DUMP_FORMAT_AUTO;

 config = notmuch_config_open (ctx, NULL, NULL);
 if (config == NULL)
@@ -119,9 +116,15 @@ notmuch_restore_command (unused (void *ctx), int argc, 
char *argv[])
   NOTMUCH_DATABASE_MODE_READ_WRITE, ))
return 1;

-synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
+if (notmuch_config_get_maildir_synchronize_flags (config))
+   flags |= TAG_FLAG_MAILDIR_SYNC;

 

[PATCH 10/16] test: add test for notmuch tag --batch option

2012-11-18 Thread da...@tethera.net
From: Jani Nikula 

Basic test of functionality, along with all combinations of options.
---
 test/tagging |   35 +++
 1 file changed, 35 insertions(+)

diff --git a/test/tagging b/test/tagging
index 980ff92..574bc10 100755
--- a/test/tagging
+++ b/test/tagging
@@ -46,6 +46,41 @@ 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 tag4 unread)"

+test_begin_subtest "--batch"
+notmuch tag --batch < batch.in  <

[PATCH 09/16] cli: add support for batch tagging operations to "notmuch tag"

2012-11-18 Thread da...@tethera.net
From: Jani Nikula 

Add support for batch tagging operations through stdin to "notmuch
tag". This can be enabled with the new --stdin command line option to
"notmuch new". The input must consist of lines of the format:

+|- [...] [--] 

Each line is interpreted similarly to "notmuch tag" command line
arguments. The delimiter is one or more spaces ' '. Any characters in
 and  MAY be hex encoded with %NN where NN is the
hexadecimal value of the character. Any ' ' and '%' characters in
 and  MUST be hex encoded (using %20 and %25,
respectively). Any characters that are not part of  or
 MUST NOT be hex encoded.

Leading and trailing space ' ' is ignored. Empty lines and lines
beginning with '#' are ignored.

Signed-off-by: Jani Nikula 

Hacked-like-crazy-by: David Bremner 
---
 notmuch-tag.c |  177 +++--
 1 file changed, 109 insertions(+), 68 deletions(-)

diff --git a/notmuch-tag.c b/notmuch-tag.c
index 88d559b..ca120d5 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -19,6 +19,7 @@
  */

 #include "notmuch-client.h"
+#include "tag-util.h"

 static volatile sig_atomic_t interrupted;

@@ -54,14 +55,9 @@ _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,
-const tag_operation_t *tag_ops)
+const tag_op_list_t *list)
 {
 /* This is subtler than it looks.  Xapian ignores the '-' operator
  * at the beginning both queries and parenthesized groups and,
@@ -75,6 +71,7 @@ _optimize_tag_query (void *ctx, const char *orig_query_string,
 const char *join = "";
 int i;
 unsigned int max_tag_len = 0;
+const tag_operation_t *tag_ops = list->ops;

 /* Don't optimize if there are no tag changes. */
 if (tag_ops[0].tag == NULL)
@@ -116,12 +113,11 @@ _optimize_tag_query (void *ctx, const char 
*orig_query_string,
  * 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)
+  tag_op_list_t *tag_ops, tag_op_flag_t 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. */
@@ -144,21 +140,7 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const 
char *query_string,
 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);
-
+   tag_op_list_apply (message, tag_ops, flags);
notmuch_message_destroy (message);
 }

@@ -170,15 +152,17 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const 
char *query_string,
 int
 notmuch_tag_command (void *ctx, int argc, char *argv[])
 {
-tag_operation_t *tag_ops;
-int tag_ops_count = 0;
-char *query_string;
+tag_op_list_t *tag_ops = NULL;
+char *query_string = NULL;
 notmuch_config_t *config;
 notmuch_database_t *notmuch;
 struct sigaction action;
-notmuch_bool_t synchronize_flags;
-int i;
-int ret;
+tag_op_flag_t synchronize_flags = TAG_FLAG_NONE;
+notmuch_bool_t batch = FALSE;
+FILE *input = stdin;
+char *input_file_name = NULL;
+int i, opt_index;
+int ret = 0;

 /* Setup our handler for SIGINT */
 memset (, 0, sizeof (struct sigaction));
@@ -187,53 +171,76 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
 action.sa_flags = SA_RESTART;
 sigaction (SIGINT, , NULL);

-argc--; argv++; /* skip subcommand argument */
+notmuch_opt_desc_t options[] = {
+   { NOTMUCH_OPT_BOOLEAN, , "batch", 0, 0 },
+   { NOTMUCH_OPT_STRING, _file_name, "input", 'i', 0 },
+   { 0, 0, 0, 0, 0 }
+};

-/* 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");
+opt_index = parse_arguments (argc, argv, options, 1);
+if (opt_index < 0)
return 1;
+
+if (input_file_name) {
+   batch = TRUE;
+   input = fopen (input_file_name, "r");
+   if (input == NULL) {
+   fprintf (stderr, "Error opening %s for reading: %s\n",
+input_file_name, 

[PATCH 08/16] tag-util.[ch]: New files for common tagging routines

2012-11-18 Thread da...@tethera.net
From: David Bremner 

These are meant to be shared between notmuch-tag and notmuch-restore.

The bulk of the routines implement a "tag operation list" abstract
data type act as a structured representation of a set of tag
operations (typically coming from a single tag command or line of
input).
---
 Makefile.local |1 +
 tag-util.c |  273 
 tag-util.h |   71 +++
 3 files changed, 345 insertions(+)
 create mode 100644 tag-util.c
 create mode 100644 tag-util.h

diff --git a/Makefile.local b/Makefile.local
index 2b91946..854867d 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -274,6 +274,7 @@ notmuch_client_srcs =   \
query-string.c  \
mime-node.c \
crypto.c\
+   tag-util.c

 notmuch_client_modules = $(notmuch_client_srcs:.c=.o)

diff --git a/tag-util.c b/tag-util.c
new file mode 100644
index 000..5329b1f
--- /dev/null
+++ b/tag-util.c
@@ -0,0 +1,273 @@
+#include "tag-util.h"
+#include "hex-escape.h"
+
+static char *
+strtok_len (char *s, const char *delim, size_t *len)
+{
+/* skip initial delims */
+s += strspn (s, delim);
+
+/* length of token */
+*len = strcspn (s, delim);
+
+return *len ? s : NULL;
+}
+
+/* Tag messages according to 'input', which must consist of lines of
+ * the format:
+ *
+ * +|- [...] [--] 
+ *
+ * Each line is interpreted similarly to "notmuch tag" command line
+ * arguments. The delimiter is one or more spaces ' '. Any characters
+ * in  and  MAY be hex encoded with %NN where NN is
+ * the hexadecimal value of the character. Any ' ' and '%' characters
+ * in  and  MUST be hex encoded (using %20 and %25,
+ * respectively). Any characters that are not part of  or
+ *  MUST NOT be hex encoded.
+ *
+ * Leading and trailing space ' ' is ignored. Empty lines and lines
+ * beginning with '#' are ignored.
+ */
+int
+parse_tag_line (void *ctx, char *line,
+   char **query_string,
+   tag_op_list_t *tag_ops)
+{
+char *tok = line;
+size_t tok_len = 0;
+
+chomp_newline (line);
+tag_op_list_reset (tag_ops);
+
+/* Parse tags. */
+while ((tok = strtok_len (tok + tok_len, " ", _len)) != NULL) {
+   notmuch_bool_t remove;
+   char *tag;
+
+   /* Optional explicit end of tags marker. */
+   if (strncmp (tok, "--", tok_len) == 0) {
+   tok = strtok_len (tok + tok_len, " ", _len);
+   break;
+   }
+
+   /* Implicit end of tags. */
+   if (*tok != '-' && *tok != '+')
+   break;
+
+   /* If tag is terminated by NUL, there's no query string. */
+   if (*(tok + tok_len) == '\0') {
+   tok = NULL;
+   break;
+   }
+
+   /* Terminate, and start next token after terminator. */
+   *(tok + tok_len++) = '\0';
+
+   remove = (*tok == '-');
+   tag = tok + 1;
+
+   /* Refuse empty tags. */
+   if (*tag == '\0') {
+   tok = NULL;
+   break;
+   }
+
+   /* Decode tag. */
+   if (hex_decode_inplace (tag) != HEX_SUCCESS) {
+   tok = NULL;
+   break;
+   }
+
+   if (tag_op_list_append (ctx, tag_ops, tag, remove))
+   return -1;
+}
+
+if (tok == NULL || tag_ops->count == 0) {
+   /* FIXME: line has been modified! */
+   fprintf (stderr, "Warning: Ignoring invalid input line: %s\n",
+line);
+   return 1;
+}
+
+/* tok now points to the query string */
+if (hex_decode_inplace (tok) != HEX_SUCCESS) {
+   /* FIXME: line has been modified! */
+   fprintf (stderr, "Warning: Ignoring invalid input line: %s\n",
+line);
+   return 1;
+}
+
+*query_string = tok;
+
+return 0;
+}
+
+static inline void
+message_error (notmuch_message_t *message,
+  notmuch_status_t status,
+  const char *format, ...)
+{
+va_list va_args;
+
+va_start (va_args, format);
+
+vfprintf (stderr, format, va_args);
+fprintf (stderr, "Message-ID: %s\n", notmuch_message_get_message_id 
(message));
+fprintf (stderr, "Status: %s\n", notmuch_status_to_string (status));
+}
+
+notmuch_status_t
+tag_op_list_apply (notmuch_message_t *message,
+  tag_op_list_t *list,
+  tag_op_flag_t flags)
+{
+int i;
+
+notmuch_status_t status = 0;
+tag_operation_t *tag_ops = list->ops;
+
+status = notmuch_message_freeze (message);
+if (status) {
+   message_error (message, status, "freezing message");
+   return status;
+}
+
+if (flags & TAG_FLAG_REMOVE_ALL) {
+   status = notmuch_message_remove_all_tags (message);
+   if (status) {
+   message_error (message, status, "removing all tags" );
+   return status;
+   }
+}
+
+for (i = 0; i < list->count; i++) {
+   if (tag_ops[i].remove) {
+   status = notmuch_message_remove_tag (message, 

[PATCH 07/16] notmuch-dump: add --format=(batch-tag|sup)

2012-11-18 Thread da...@tethera.net
From: David Bremner 

sup is the old format, and remains the default, at least until
restore is converted to parse this format.

Each line of the batch- format is valid for notmuch tag, (with the
"notmuch tag" omitted from the front of the line). The dump format
only uses query strings of a single message-id.

Each space seperated tag/message-id is 'hex-encoded' to remove
troubling characters.  In particular this format won't have the same
problem with e.g. spaces in message-ids or tags; they will be
round-trip-able.
---
 dump-restore-private.h |   13 +
 notmuch-dump.c |   42 --
 2 files changed, 49 insertions(+), 6 deletions(-)
 create mode 100644 dump-restore-private.h

diff --git a/dump-restore-private.h b/dump-restore-private.h
new file mode 100644
index 000..896a004
--- /dev/null
+++ b/dump-restore-private.h
@@ -0,0 +1,13 @@
+#ifndef DUMP_RESTORE_PRIVATE_H
+#define DUMP_RESTORE_PRIVATE_H
+
+#include "hex-escape.h"
+#include "command-line-arguments.h"
+
+typedef enum dump_formats {
+DUMP_FORMAT_AUTO,
+DUMP_FORMAT_BATCH_TAG,
+DUMP_FORMAT_SUP
+} dump_format_t;
+
+#endif
diff --git a/notmuch-dump.c b/notmuch-dump.c
index 88f598a..045ca9e 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -19,6 +19,7 @@
  */

 #include "notmuch-client.h"
+#include "dump-restore-private.h"

 int
 notmuch_dump_command (unused (void *ctx), int argc, char *argv[])
@@ -43,7 +44,13 @@ notmuch_dump_command (unused (void *ctx), int argc, char 
*argv[])
 char *output_file_name = NULL;
 int opt_index;

+int output_format = DUMP_FORMAT_SUP;
+
 notmuch_opt_desc_t options[] = {
+   { NOTMUCH_OPT_KEYWORD, _format, "format", 'f',
+ (notmuch_keyword_t []){ { "sup", DUMP_FORMAT_SUP },
+ { "batch-tag", DUMP_FORMAT_BATCH_TAG },
+ { 0, 0 } } },
{ NOTMUCH_OPT_STRING, _file_name, "output", 'o', 0  },
{ 0, 0, 0, 0, 0 }
 };
@@ -83,27 +90,50 @@ notmuch_dump_command (unused (void *ctx), int argc, char 
*argv[])
  */
 notmuch_query_set_sort (query, NOTMUCH_SORT_UNSORTED);

+char *buffer = NULL;
+size_t buffer_size = 0;
+
 for (messages = notmuch_query_search_messages (query);
 notmuch_messages_valid (messages);
 notmuch_messages_move_to_next (messages)) {
int first = 1;
+   const char *message_id;
+
message = notmuch_messages_get (messages);
+   message_id = notmuch_message_get_message_id (message);

-   fprintf (output,
-"%s (", notmuch_message_get_message_id (message));
+   if (output_format == DUMP_FORMAT_SUP) {
+   fprintf (output, "%s (", message_id);
+   }

for (tags = notmuch_message_get_tags (message);
 notmuch_tags_valid (tags);
 notmuch_tags_move_to_next (tags)) {
-   if (! first)
-   fprintf (output, " ");
+   const char *tag_str = notmuch_tags_get (tags);

-   fprintf (output, "%s", notmuch_tags_get (tags));
+   if (! first)
+   fputs (" ", output);

first = 0;
+
+   if (output_format == DUMP_FORMAT_SUP) {
+   fputs (tag_str, output);
+   } else {
+   if (hex_encode (notmuch, tag_str,
+   , _size) != HEX_SUCCESS)
+   return 1;
+   fprintf (output, "+%s", buffer);
+   }
}

-   fprintf (output, ")\n");
+   if (output_format == DUMP_FORMAT_SUP) {
+   fputs (")\n", output);
+   } else {
+   if (hex_encode (notmuch, message_id,
+   , _size) != HEX_SUCCESS)
+   return 1;
+   fprintf (output, " -- id:%s\n", buffer);
+   }

notmuch_message_destroy (message);
 }
-- 
1.7.10.4



[PATCH 06/16] test: add broken roundtrip test

2012-11-18 Thread da...@tethera.net
From: David Bremner 

We demonstrate the current notmuch restore parser being confused by
message-id's and tags containing non alpha numeric characters
(particularly space and parentheses are problematic because they are
not escaped by notmuch dump).

We save the files as hex escaped on disk so that the output from the
failing test will not confuse the terminal emulator of people running
the test.
---
 test/dump-restore |9 +
 1 file changed, 9 insertions(+)

diff --git a/test/dump-restore b/test/dump-restore
index b05399c..a2204fb 100755
--- a/test/dump-restore
+++ b/test/dump-restore
@@ -85,4 +85,13 @@ test_begin_subtest "dump --output=outfile -- from:cworth"
 notmuch dump --output=dump-outfile-dash-inbox.actual -- from:cworth
 test_expect_equal_file dump-cworth.expected dump-outfile-dash-inbox.actual

+test_expect_success 'roundtripping random message-ids and tags' \
+'test_subtest_known_broken &&
+ ${TEST_DIRECTORY}/random-corpus --num-messages=10 
--config-path=${NOTMUCH_CONFIG} &&
+ notmuch dump | ${TEST_DIRECTORY}/hex-xcode --direction=encode > 
EXPECTED.$test_count &&
+ notmuch tag -random-corpus tag:random-corpus &&
+ ${TEST_DIRECTORY}/hex-xcode --direction=decode < EXPECTED.$test_count | 
notmuch restore 2>/dev/null &&
+ notmuch dump | ${TEST_DIRECTORY}/hex-xcode --direction=encode > 
OUTPUT.$test_count &&
+ test_cmp EXPECTED.$test_count OUTPUT.$test_count'
+
 test_done
-- 
1.7.10.4



[PATCH 05/16] test: add generator for random "stub" messages

2012-11-18 Thread da...@tethera.net
From: David Bremner 

Initial use case is testing dump and restore, so we only have
message-ids and tags.

The message ID's are nothing like RFC compliant, but it doesn't seem
any harder to roundtrip random UTF-8 strings than RFC-compliant ones.

Tags are UTF-8, even though notmuch is in principle more generous than
that.

updated for id:m2wr04ocro.fsf at guru.guru-group.fi

- talk about Unicode value rather some specific encoding
- call talloc_realloc less times
---
 test/.gitignore  |1 +
 test/Makefile.local  |   10 +++
 test/basic   |1 +
 test/random-corpus.c |  197 ++
 4 files changed, 209 insertions(+)
 create mode 100644 test/random-corpus.c

diff --git a/test/.gitignore b/test/.gitignore
index be7ab5e..1eff7ce 100644
--- a/test/.gitignore
+++ b/test/.gitignore
@@ -4,4 +4,5 @@ smtp-dummy
 symbol-test
 arg-test
 hex-xcode
+random-corpus
 tmp.*
diff --git a/test/Makefile.local b/test/Makefile.local
index 8479f91..6a9f15e 100644
--- a/test/Makefile.local
+++ b/test/Makefile.local
@@ -16,6 +16,14 @@ $(dir)/arg-test: $(dir)/arg-test.o command-line-arguments.o 
util/libutil.a
 $(dir)/hex-xcode: $(dir)/hex-xcode.o command-line-arguments.o util/libutil.a
$(call quiet,CC) -I. $^ -o $@ -ltalloc

+random_corpus_deps =  $(dir)/random-corpus.o  $(dir)/database-test.o \
+   notmuch-config.o command-line-arguments.o \
+   lib/libnotmuch.a util/libutil.a \
+   parse-time-string/libparse-time-string.a
+
+$(dir)/random-corpus: $(random_corpus_deps)
+   $(call quiet,CC) $(CFLAGS_FINAL) $^ -o $@ $(CONFIGURE_LDFLAGS)
+
 $(dir)/smtp-dummy: $(smtp_dummy_modules)
$(call quiet,CC) $^ -o $@

@@ -29,6 +37,7 @@ $(dir)/parse-time: $(dir)/parse-time.o 
parse-time-string/parse-time-string.o

 TEST_BINARIES=$(dir)/arg-test \
  $(dir)/hex-xcode \
+ $(dir)/random-corpus \
  $(dir)/parse-time \
  $(dir)/smtp-dummy \
  $(dir)/symbol-test
@@ -46,5 +55,6 @@ CLEAN := $(CLEAN) $(dir)/smtp-dummy $(dir)/smtp-dummy.o \
 $(dir)/arg-test $(dir)/arg-test.o \
 $(dir)/hex-xcode $(dir)/hex-xcode.o \
 $(dir)/database-test.o \
+$(dir)/random-corpus $(dir)/random-corpus.o \
 $(dir)/parse-time $(dir)/parse-time.o \
 $(dir)/corpus.mail $(dir)/test-results $(dir)/tmp.*
diff --git a/test/basic b/test/basic
index 2a571ac..f93469f 100755
--- a/test/basic
+++ b/test/basic
@@ -59,6 +59,7 @@ available=$(find "$TEST_DIRECTORY" -maxdepth 1 -type f -perm 
+111  \
 ! -name hex-xcode  \
 ! -name notmuch-test   \
 ! -name parse-time \
+! -name random-corpus  \
 ! -name smtp-dummy \
 ! -name symbol-test\
 ! -name test-verbose   \
diff --git a/test/random-corpus.c b/test/random-corpus.c
new file mode 100644
index 000..91e98b1
--- /dev/null
+++ b/test/random-corpus.c
@@ -0,0 +1,197 @@
+/*
+ * Generate a random corpus of stub messages.
+ *
+ * Initial use case is testing dump and restore, so we only have
+ * message-ids and tags.
+ *
+ * Generated message-id's and tags are intentionally nasty.
+ *
+ * Copyright (c) 2012 David Bremner
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ *
+ * Author: David Bremner 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "notmuch-client.h"
+#include "command-line-arguments.h"
+#include "database-test.h"
+
+/* Current largest Unicode value defined. Note that most of these will
+ * be printed as boxes in most fonts.
+ */
+
+#define GLYPH_MAX 0x10FFFE
+
+static gunichar
+random_unichar ()
+{
+int start = 1, stop = GLYPH_MAX;
+int class = random() % 2;
+
+/*
+ *  Choose about half ascii as test characters, as ascii
+ *  punctation and whitespace is the main cause of problems for
+ *  the (old) restore parser
+*/
+switch (class) {
+case 0:
+   /* ascii */
+   start = 0x01;
+   stop = 0x7f;
+   break;
+case 1:
+   /* the rest of unicode */
+   start = 0x80;
+   stop = GLYPH_MAX;
+}
+
+if (start == stop)
+   return start;
+else
+   return start + (random() % (stop - start + 1));
+}
+
+static char *

[PATCH 04/16] test: add database routines for testing.

2012-11-18 Thread da...@tethera.net
From: David Bremner 

Initially, provide a way to create "stub" messages in the notmuch
database without corresponding files.  This is essentially cut and
paste from lib/database.cc. This is a seperate file since we don't
want to export these symbols from libnotmuch or bloat the library with
non-exported code.
---
 test/Makefile.local  |1 +
 test/database-test.c |   72 ++
 test/database-test.h |   21 +++
 3 files changed, 94 insertions(+)
 create mode 100644 test/database-test.c
 create mode 100644 test/database-test.h

diff --git a/test/Makefile.local b/test/Makefile.local
index 8da4c56..8479f91 100644
--- a/test/Makefile.local
+++ b/test/Makefile.local
@@ -45,5 +45,6 @@ CLEAN := $(CLEAN) $(dir)/smtp-dummy $(dir)/smtp-dummy.o \
 $(dir)/symbol-test $(dir)/symbol-test.o \
 $(dir)/arg-test $(dir)/arg-test.o \
 $(dir)/hex-xcode $(dir)/hex-xcode.o \
+$(dir)/database-test.o \
 $(dir)/parse-time $(dir)/parse-time.o \
 $(dir)/corpus.mail $(dir)/test-results $(dir)/tmp.*
diff --git a/test/database-test.c b/test/database-test.c
new file mode 100644
index 000..f0f1c8e
--- /dev/null
+++ b/test/database-test.c
@@ -0,0 +1,72 @@
+/*
+ * Database routines intended only for testing, not exported from
+ * library.
+ *
+ * Copyright (c) 2012 David Bremner
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ *
+ * Author: David Bremner 
+ */
+
+#include "notmuch-private.h"
+#include "database-test.h"
+
+notmuch_status_t
+notmuch_database_add_stub_message (notmuch_database_t *notmuch,
+  const char *message_id,
+  const char **tags)
+{
+const char **tag;
+notmuch_status_t ret;
+notmuch_private_status_t private_status;
+notmuch_message_t *message;
+
+ret = _notmuch_database_ensure_writable (notmuch);
+if (ret)
+   return ret;
+
+message = _notmuch_message_create_for_message_id (notmuch,
+ message_id,
+ _status);
+if (message == NULL) {
+   return COERCE_STATUS (private_status,
+ "Unexpected status value from 
_notmuch_message_create_for_message_id");
+
+}
+
+if (private_status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) {
+   _notmuch_message_add_term (message, "type", "mail");
+} else {
+   return NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID;
+}
+
+if (tags) {
+   ret = notmuch_message_freeze (message);
+   if (ret)
+   return ret;
+
+   for (tag = tags; *tag; tag++) {
+   ret = notmuch_message_add_tag (message, *tag);
+   if (ret)
+   return ret;
+   }
+}
+
+ret = notmuch_message_thaw (message);
+if (ret)
+   return ret;
+
+return NOTMUCH_STATUS_SUCCESS;
+}
diff --git a/test/database-test.h b/test/database-test.h
new file mode 100644
index 000..84f7988
--- /dev/null
+++ b/test/database-test.h
@@ -0,0 +1,21 @@
+#ifndef _DATABASE_TEST_H
+#define _DATABASE_TEST_H
+/* Add a new stub message to the given notmuch database.
+ *
+ * At least the following return values are possible:
+ *
+ * NOTMUCH_STATUS_SUCCESS: Message successfully added to database.
+ *
+ * NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID: Message has the same message
+ * ID as another message already in the database.
+ *
+ * NOTMUCH_STATUS_READ_ONLY_DATABASE: Database was opened in read-only
+ * mode so no message can be added.
+ */
+
+notmuch_status_t
+notmuch_database_add_stub_message (notmuch_database_t *database,
+  const char *message_id,
+  const char **tag_list);
+
+#endif
-- 
1.7.10.4



[PATCH 03/16] test/hex-escaping: new test for hex escaping routines

2012-11-18 Thread da...@tethera.net
From: David Bremner 

These are more like unit tests, to (try to) make sure the library
functionality is working before building more complicated things on
top of it.
---
 test/hex-escaping |   26 ++
 test/notmuch-test |1 +
 2 files changed, 27 insertions(+)
 create mode 100755 test/hex-escaping

diff --git a/test/hex-escaping b/test/hex-escaping
new file mode 100755
index 000..f34cc8c
--- /dev/null
+++ b/test/hex-escaping
@@ -0,0 +1,26 @@
+#!/usr/bin/env bash
+test_description="hex encoding and decoding"
+. ./test-lib.sh
+
+test_begin_subtest "round trip"
+find $TEST_DIRECTORY/corpus -type f -print | sort | xargs cat > EXPECTED
+$TEST_DIRECTORY/hex-xcode --direction=encode < EXPECTED | 
$TEST_DIRECTORY/hex-xcode --direction=decode > OUTPUT
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "punctuation"
+tag1='comic_swear=$&^%$^%\\//-+$^%$'
+tag_enc1=$($TEST_DIRECTORY/hex-xcode --direction=encode "$tag1")
+test_expect_equal "$tag_enc1" 
"comic_swear=%24%26%5e%25%24%5e%25%5c%5c%2f%2f-+%24%5e%25%24"
+
+test_begin_subtest "round trip newlines"
+printf 'this\n tag\t has\n spaces\n' > EXPECTED.$test_count
+$TEST_DIRECTORY/hex-xcode --direction=encode  < EXPECTED.$test_count |\
+   $TEST_DIRECTORY/hex-xcode --direction=decode > OUTPUT.$test_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+
+test_begin_subtest "round trip 8bit chars"
+echo '%c3%91%c3%a5%c3%b0%c3%a3%c3%a5%c3%a9-%c3%8f%c3%8a' > EXPECTED.$test_count
+$TEST_DIRECTORY/hex-xcode --direction=decode  < EXPECTED.$test_count |\
+   $TEST_DIRECTORY/hex-xcode --direction=encode > OUTPUT.$test_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+test_done
diff --git a/test/notmuch-test b/test/notmuch-test
index 9a1b375..d2e90e2 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -60,6 +60,7 @@ TESTS="
   emacs-hello
   emacs-show
   missing-headers
+  hex-escaping
   parse-time-string
   search-date
 "
-- 
1.7.10.4



[PATCH 02/16] test/hex-xcode: new test binary

2012-11-18 Thread da...@tethera.net
From: David Bremner 

This program is used both as a test-bed/unit-tester for
../util/hex-escape.c, and also as a utility in future tests of dump
and restore.
---
 test/.gitignore |1 +
 test/Makefile.local |   13 ++-
 test/basic  |1 +
 test/hex-xcode.c|  103 +++
 4 files changed, 116 insertions(+), 2 deletions(-)
 create mode 100644 test/hex-xcode.c

diff --git a/test/.gitignore b/test/.gitignore
index e63c689..be7ab5e 100644
--- a/test/.gitignore
+++ b/test/.gitignore
@@ -3,4 +3,5 @@ corpus.mail
 smtp-dummy
 symbol-test
 arg-test
+hex-xcode
 tmp.*
diff --git a/test/Makefile.local b/test/Makefile.local
index 9ae130a..8da4c56 100644
--- a/test/Makefile.local
+++ b/test/Makefile.local
@@ -13,6 +13,9 @@ smtp_dummy_modules = $(smtp_dummy_srcs:.c=.o)
 $(dir)/arg-test: $(dir)/arg-test.o command-line-arguments.o util/libutil.a
$(call quiet,CC) -I. $^ -o $@

+$(dir)/hex-xcode: $(dir)/hex-xcode.o command-line-arguments.o util/libutil.a
+   $(call quiet,CC) -I. $^ -o $@ -ltalloc
+
 $(dir)/smtp-dummy: $(smtp_dummy_modules)
$(call quiet,CC) $^ -o $@

@@ -24,8 +27,13 @@ $(dir)/parse-time: $(dir)/parse-time.o 
parse-time-string/parse-time-string.o

 .PHONY: test check

-test-binaries: $(dir)/arg-test $(dir)/smtp-dummy $(dir)/symbol-test \
-   $(dir)/parse-time
+TEST_BINARIES=$(dir)/arg-test \
+ $(dir)/hex-xcode \
+ $(dir)/parse-time \
+ $(dir)/smtp-dummy \
+ $(dir)/symbol-test
+
+test-binaries: $(TEST_BINARIES)

 test:  all test-binaries
@${dir}/notmuch-test $(OPTIONS)
@@ -36,5 +44,6 @@ SRCS := $(SRCS) $(smtp_dummy_srcs)
 CLEAN := $(CLEAN) $(dir)/smtp-dummy $(dir)/smtp-dummy.o \
 $(dir)/symbol-test $(dir)/symbol-test.o \
 $(dir)/arg-test $(dir)/arg-test.o \
+$(dir)/hex-xcode $(dir)/hex-xcode.o \
 $(dir)/parse-time $(dir)/parse-time.o \
 $(dir)/corpus.mail $(dir)/test-results $(dir)/tmp.*
diff --git a/test/basic b/test/basic
index 1b842d2..2a571ac 100755
--- a/test/basic
+++ b/test/basic
@@ -56,6 +56,7 @@ tests_in_suite=$(for i in $TESTS; do echo $i; done | sort)
 available=$(find "$TEST_DIRECTORY" -maxdepth 1 -type f -perm +111  \
 ! -name aggregate-results.sh   \
 ! -name arg-test   \
+! -name hex-xcode  \
 ! -name notmuch-test   \
 ! -name parse-time \
 ! -name smtp-dummy \
diff --git a/test/hex-xcode.c b/test/hex-xcode.c
new file mode 100644
index 000..eec6541
--- /dev/null
+++ b/test/hex-xcode.c
@@ -0,0 +1,103 @@
+/* No, nothing to to with IDE from Apple Inc.
+   testbed for ../util/hex-escape.c.
+
+   usage:
+   hex-xcode [--direction=(encode|decode)] [--omit-newline] < file
+   hex-xcode [--direction=(encode|decode)] [--omit-newline] arg1 arg2 arg3 ...
+
+ */
+
+#include "notmuch-client.h"
+#include "hex-escape.h"
+#include 
+
+
+enum direction {
+ENCODE,
+DECODE
+};
+
+static int
+xcode (void *ctx, enum direction dir, char *in, char **buf_p, size_t *size_p)
+{
+hex_status_t status;
+
+if (dir == ENCODE)
+   status = hex_encode (ctx, in, buf_p, size_p);
+else
+   status = hex_decode (ctx, in, buf_p, size_p);
+
+if (status == HEX_SUCCESS)
+   fputs (*buf_p, stdout);
+
+return status;
+}
+
+
+int
+main (int argc, char **argv)
+{
+
+
+enum direction dir = DECODE;
+int omit_newline = FALSE;
+
+notmuch_opt_desc_t options[] = {
+   { NOTMUCH_OPT_KEYWORD, , "direction", 'd',
+ (notmuch_keyword_t []){ { "encode", ENCODE },
+ { "decode", DECODE },
+ { 0, 0 } } },
+   { NOTMUCH_OPT_BOOLEAN, _newline, "omit-newline", 'n', 0 },
+   { 0, 0, 0, 0, 0 }
+};
+
+int opt_index = parse_arguments (argc, argv, options, 1);
+
+if (opt_index < 0)
+   exit (1);
+
+void *ctx = talloc_new (NULL);
+
+char *line = NULL;
+size_t line_size;
+ssize_t line_len;
+
+char *buffer = NULL;
+size_t buf_size = 0;
+
+notmuch_bool_t read_stdin = TRUE;
+
+for (; opt_index < argc; opt_index++) {
+
+   if (xcode (ctx, dir, argv[opt_index],
+  , _size) != HEX_SUCCESS)
+   return 1;
+
+   if (!omit_newline)
+   putchar ('\n');
+
+   read_stdin = FALSE;
+}
+
+if (!read_stdin)
+   return 0;
+
+while ((line_len = getline (, _size, stdin)) != -1) {
+
+   chomp_newline (line);
+
+   if (xcode (ctx, dir, line, , _size) != HEX_SUCCESS)
+   return 1;
+
+   if (!omit_newline)
+   putchar ('\n');
+
+}
+
+if (line)
+   free (line);
+
+talloc_free (ctx);
+
+return 0;
+}
-- 
1.7.10.4



[PATCH 01/16] hex-escape: (en|de)code strings to/from restricted character set

2012-11-18 Thread da...@tethera.net
From: David Bremner 

The character set is chosen to be suitable for pathnames, and the same
as that used by contrib/nmbug

[With additions by Jani Nikula]
---
 util/Makefile.local |2 +-
 util/hex-escape.c   |  168 +++
 util/hex-escape.h   |   41 +
 3 files changed, 210 insertions(+), 1 deletion(-)
 create mode 100644 util/hex-escape.c
 create mode 100644 util/hex-escape.h

diff --git a/util/Makefile.local b/util/Makefile.local
index c7cae61..3ca623e 100644
--- a/util/Makefile.local
+++ b/util/Makefile.local
@@ -3,7 +3,7 @@
 dir := util
 extra_cflags += -I$(srcdir)/$(dir)

-libutil_c_srcs := $(dir)/xutil.c $(dir)/error_util.c
+libutil_c_srcs := $(dir)/xutil.c $(dir)/error_util.c $(dir)/hex-escape.c

 libutil_modules := $(libutil_c_srcs:.c=.o)

diff --git a/util/hex-escape.c b/util/hex-escape.c
new file mode 100644
index 000..d8905d0
--- /dev/null
+++ b/util/hex-escape.c
@@ -0,0 +1,168 @@
+/* hex-escape.c -  Manage encoding and decoding of byte strings into path names
+ *
+ * Copyright (c) 2011 David Bremner
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ *
+ * Author: David Bremner 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include "error_util.h"
+#include "hex-escape.h"
+
+static const size_t default_buf_size = 1024;
+
+static const char *output_charset =
+"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+-_@=.:,";
+
+static const char escape_char = '%';
+
+static int
+is_output (char c)
+{
+return (strchr (output_charset, c) != NULL);
+}
+
+static int
+maybe_realloc (void *ctx, size_t needed, char **out, size_t *out_size)
+{
+if (*out_size < needed) {
+
+   if (*out == NULL)
+   *out = talloc_size (ctx, needed);
+   else
+   *out = talloc_realloc (ctx, *out, char, needed);
+
+   if (*out == NULL)
+   return 0;
+
+   *out_size = needed;
+}
+return 1;
+}
+
+hex_status_t
+hex_encode (void *ctx, const char *in, char **out, size_t *out_size)
+{
+
+const unsigned char *p;
+char *q;
+
+size_t escape_count = 0;
+size_t len = 0;
+size_t needed;
+
+assert (ctx); assert (in); assert (out); assert (out_size);
+
+for (p = (unsigned char *) in; *p; p++) {
+   escape_count += (!is_output (*p));
+   len++;
+}
+
+needed = len + escape_count * 2 + 1;
+
+if (*out == NULL)
+   *out_size = 0;
+
+if (!maybe_realloc (ctx, needed, out, out_size))
+   return HEX_OUT_OF_MEMORY;
+
+q = *out;
+p = (unsigned char *) in;
+
+while (*p) {
+   if (is_output (*p)) {
+   *q++ = *p++;
+   } else {
+   sprintf (q, "%%%02x", *p++);
+   q += 3;
+   }
+}
+
+*q = '\0';
+return HEX_SUCCESS;
+}
+
+/* Hex decode 'in' to 'out'.
+ *
+ * This must succeed for in == out to support hex_decode_inplace().
+ */
+static hex_status_t
+hex_decode_internal (const char *in, unsigned char *out)
+{
+char buf[3];
+
+while (*in) {
+   if (*in == escape_char) {
+   char *endp;
+
+   /* This also handles unexpected end-of-string. */
+   if (!isxdigit ((unsigned char) in[1]) ||
+   !isxdigit ((unsigned char) in[2]))
+   return HEX_SYNTAX_ERROR;
+
+   buf[0] = in[1];
+   buf[1] = in[2];
+   buf[2] = '\0';
+
+   *out = strtoul (buf, , 16);
+
+   if (endp != buf + 2)
+   return HEX_SYNTAX_ERROR;
+
+   in += 3;
+   out++;
+   } else {
+   *out++ = *in++;
+   }
+}
+
+*out = '\0';
+
+return HEX_SUCCESS;
+}
+
+hex_status_t
+hex_decode_inplace (char *s)
+{
+/* A decoded string is never longer than the encoded one, so it is
+ * safe to decode a string onto itself. */
+return hex_decode_internal (s, (unsigned char *) s);
+}
+
+hex_status_t
+hex_decode (void *ctx, const char *in, char **out, size_t * out_size)
+{
+const char *p;
+size_t escape_count = 0;
+size_t needed = 0;
+
+assert (ctx); assert (in); assert (out); assert (out_size);
+
+size_t len = strlen (in);
+
+for (p = in; *p; p++)
+   escape_count += (*p == escape_char);
+
+needed = len - escape_count * 2 + 1;
+
+if (!maybe_realloc (ctx, needed, out, out_size))
+   return HEX_OUT_OF_MEMORY;
+
+return hex_decode_internal 

Add new dump/restore format and batch tagging.

2012-11-18 Thread da...@tethera.net
This is a patch series with some history, if you will forgive the
semi-inadvertant pun. I list that at that end, for the curious.

If this series goes in, in the future we might consider whether
restore --accumulate provides important functionality over batch
tagging; at the moment I suggest leaving it as the extra code to
support it is minimal, and it does support peoples existing scripts
using the old dump/restore format.

History
---

About 1 year ago, Petter Reinholdtsen observed a problem with dumping
and restoring message-id's with spaces in them. 

id:2flfwhht87d.fsf at diskless.uio.no

There followed a proposed fix

  id:1323808075-7417-1-git-send-email-david at tethera.net

Which Dmitry had a few helpful things to say about the hex encoding
libs.

Jani took that foundation and proposed two versions of the batch
tagging, most recently at

id:cover.1334404979.git.jani at nikula.org. 

There was some discussion with Jamie about the file format for batch
tagging in the thread

 id:cover.1333231401.git.jani at nikula.org

id:1323808075-7417-1-git-send-email-david at tethera.net
The first 6 of these patches obsolete the series

id:1345382314-5330-1-git-send-email-david at tethera.net

which was revied by Tomi and Ethan. I think I implemented their
suggestions.

Although I didn't re-read that whole thread, I believe this version of
the patches address's Jamie's concerns by using exactly the same
format for restore and tag --batch (renamed from Jani's choice of
--stdin).



[BUG] Saving attachments containing UTF-8 chars

2012-11-18 Thread Ethan Glasser-Camp
Ethan Glasser-Camp  writes:

> Writing this buffer using C-x C-w encodes it correctly too. So I think
> this is an emacs MIME problem. We call mm-save-part, which calls
> mm-save-part-to-file, which calls mm-with-unibyte-buffer. Hmm..
>
> Indeed, it seems that inserting this character into a file that's been
> marked "unibyte" using (set-buffer-multibyte nil) turns it into the ^Y
> character (ASCII code 0x19 -- the character that comes out in the patch
> file). There's probably a technical reason that this should be true, but
> I can't think of why that would be.

The more I think about this, the more convinced I become that this is a
bug in emacs's multibyte handling. I've filed a bug, see:

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=12925

But I think Tomi's going to formalize his hacky patch and send that out
later too. When he does, it has my +1.

Ethan


[BUG] Saving attachments containing UTF-8 chars

2012-11-18 Thread Ethan Glasser-Camp
Tomi Ollila  writes:

> I can verify this bug: I copied 'rawmail' to my mail store and attempted
> to 'w' the attacment and got the same result (after notmuch new).
>
> The saving code first does
> notmuch show --format=raw id:"508953E6.70006 at gmail.com"
> which decodes OK on command line, and to the buffer when 
> kill-buffer is outcommented in (with-current-notmuch-show-message ...) 
> macro.

I was able to see this behavior, and Tomi did a good job tracking down
where it was :)

I even see the bytes as presented in the file. When moving point to the
problematic character, and doing M-x describe-char, it says:

  buffer code: #xE2 #x80 #x99
file code: #xE2 #x80 #x99 (encoded by coding system utf-8)

buffer-file-coding-system is, of course, utf-8.

Writing this buffer using C-x C-w encodes it correctly too. So I think
this is an emacs MIME problem. We call mm-save-part, which calls
mm-save-part-to-file, which calls mm-with-unibyte-buffer. Hmm..

Indeed, it seems that inserting this character into a file that's been
marked "unibyte" using (set-buffer-multibyte nil) turns it into the ^Y
character (ASCII code 0x19 -- the character that comes out in the patch
file). There's probably a technical reason that this should be true, but
I can't think of why that would be.

> I attempted a set of trial-&-error tricks to get the attachment
> saved "correctly", and at least this seems to do the trick:
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index f273eb4..a6a85c0 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -203,9 +203,11 @@ For example, if you wanted to remove an \"unread\" tag 
> and add a
>   (let ((id (notmuch-show-get-message-id)))
> (let ((buf (generate-new-buffer (concat "*notmuch-msg-" id "*"
>   (with-current-buffer buf
> - (call-process notmuch-command nil t nil "show" "--format=raw" id)
> -   , at body)
> -  (kill-buffer buf)
> +(let ((coding-system-for-read 'no-conversion)
> +  (coding-system-for-write 'no-conversion))
> +  (call-process notmuch-command nil t nil "show" "--format=raw" id)
> +  , at body))
> +%%(kill-buffer buf)
[snip]
> (kill-buffer is outcommented above for testing purposes)
>
> To test this this needs to me evaluated and then the functions
> using this macro (notmuch-show-save-attachments  in this case)
>
> Smart suggestions for proper fix ?

Well, we could limit it just to saving attachments (putting the let
around the with-current-notmuch-show-message). That feels like it could
be right, because intuitively saving an attachment should be done
without any conversions. Or even the above doesn't seem so bad. My vague
feeling is that messages should always be ASCII, or at least mm-* will
interpret it that way, so decoding them into any other character set
might cause problems. Anyone understand character sets?

Ethan


Re: [PATCH 03/18] insert: open Maildir tmp file

2012-11-18 Thread Mark Walters

Hi

I know this has been around for a long time but it still applies so I
will try and review it. I have been using it somewhat and all seems to
work.

As an aside I have a postpone/resume implementation based on top
of it.

One general remark is that I think more comments could be helpful.

On Wed, 25 Jul 2012, Peter Wang noval...@gmail.com wrote:
 Open a unique file in the Maildir tmp directory.
 The new message is not yet copied into the file.
 ---
  notmuch-insert.c |   80 
 +-
  1 files changed, 79 insertions(+), 1 deletions(-)

 diff --git a/notmuch-insert.c b/notmuch-insert.c
 index 21424cf..f01a6f2 100644
 --- a/notmuch-insert.c
 +++ b/notmuch-insert.c
 @@ -20,12 +20,86 @@
  
  #include notmuch-client.h
  
 +#include sys/types.h
 +#include sys/stat.h
 +#include fcntl.h
 +
 +static notmuch_bool_t
 +safe_gethostname (char *hostname, size_t hostname_size)
 +{
 +if (gethostname (hostname, hostname_size) == -1) {
 + strncpy (hostname, unknown, hostname_size);
 +}
 +hostname[hostname_size - 1] = '\0';
 +
 +return (strchr (hostname, '/') == NULL);
 +}

Here is an example: a comment saying like gethostname but guarantees
that hostname is  null terminated. Returns true unless hostname contains
a /.  

Also I found hostname_size a confusing term as it the size of the buffer
which is allowed to contain hostname rather than the size of
hostname. Maybe hostname_buffer_size or just len?

 +
 +static int
 +maildir_open_tmp (void *ctx, const char *dir, char **tmppath, char **newpath)
 +{
 +pid_t pid;
 +char hostname[256];
 +struct timeval tv;
 +char *filename;
 +int fd = -1;
 +
 +/* This is the file name format used by Dovecot. */
 +pid = getpid ();
 +if (! safe_gethostname (hostname, sizeof (hostname))) {
 + fprintf (stderr, Error: invalid host name.\n);
 + return -1;
 +}
 +gettimeofday (tv, NULL);
 +filename = talloc_asprintf (ctx, %ld.M%ldP%d.%s,
 + tv.tv_sec, tv.tv_usec, pid, hostname);
 +
 +*tmppath = talloc_asprintf (ctx, %s/tmp/%s, dir, filename);

Does there need to be rather more error checking after talloc
operations? Eg what if this allocation fails?

 +
 +do {
 + fd = open (*tmppath, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, 0666);
 +} while (fd == -1  errno == EEXIST);

Am I misunderstanding or does this deadlock if the file *tmppath exists?
Do you want to recalculate tmppath inside the do while loop?

Also is 0666 standard or would 0644 or 0600 be better?

Best wishes

Mark

 +
 +if (fd != -1) {
 + *newpath = talloc_asprintf (ctx, %s/new/%s, dir, filename);
 +}
 +else {
 + fprintf (stderr, Error: opening %s: %s\n,
 +  *tmppath, strerror (errno));
 + talloc_free (*tmppath);
 +}
 +
 +talloc_free (filename);
 +
 +return fd;
 +}
 +
 +static notmuch_bool_t
 +insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,
 + const char *dir)
 +{
 +char *tmppath;
 +char *newpath;
 +int fdout;
 +
 +fdout = maildir_open_tmp (ctx, dir, tmppath, newpath);
 +if (fdout  0) {
 + return FALSE;
 +}
 +
 +close (fdout);
 +unlink (tmppath);
 +return FALSE;
 +}
 +
  int
  notmuch_insert_command (void *ctx, int argc, char *argv[])
  {
  notmuch_config_t *config;
  notmuch_database_t *notmuch;
  const char *db_path;
 +char *maildir;
 +notmuch_bool_t ret;
  
  config = notmuch_config_open (ctx, NULL, NULL);
  if (config == NULL)
 @@ -33,11 +107,15 @@ notmuch_insert_command (void *ctx, int argc, char 
 *argv[])
  
  db_path = notmuch_config_get_database_path (config);
  
 +maildir = talloc_asprintf (ctx, %s, db_path);
 +
  if (notmuch_database_open (notmuch_config_get_database_path (config),
  NOTMUCH_DATABASE_MODE_READ_WRITE, notmuch))
   return 1;
  
 +ret = insert_message (ctx, notmuch, STDIN_FILENO, maildir);
 +
  notmuch_database_destroy (notmuch);
  
 -return 1;
 +return (ret) ? 0 : 1;
  }
 -- 
 1.7.4.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: [PATCH 06/18] insert: add new message to database

2012-11-18 Thread Mark Walters

Hi

On Wed, 25 Jul 2012, Peter Wang noval...@gmail.com wrote:
 Add the new message to the notmuch database, renaming the file to encode
 notmuch tags as maildir flags.
 ---
  notmuch-insert.c |   44 
  1 files changed, 44 insertions(+), 0 deletions(-)

 diff --git a/notmuch-insert.c b/notmuch-insert.c
 index bab1fed..dd449bc 100644
 --- a/notmuch-insert.c
 +++ b/notmuch-insert.c
 @@ -129,6 +129,42 @@ copy_fd_data (int fdin, int fdout)
  }
  
  static notmuch_bool_t
 +save_database (notmuch_database_t *notmuch, const char *path)

I would prefer a different name here: save_database suggests to me that
it is a whole database operation, whereas it is actually just adding a
single message. Maybe add_message or notmuch_insert_add_message?

Best wishes

Mark

 +{
 +notmuch_message_t *message;
 +notmuch_status_t status;
 +
 +status = notmuch_database_add_message (notmuch, path, message);
 +switch (status) {
 +case NOTMUCH_STATUS_SUCCESS:
 + break;
 +case NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID:
 + fprintf (stderr, Warning: duplicate message.\n);
 + break;
 +default:
 +case NOTMUCH_STATUS_FILE_NOT_EMAIL:
 +case NOTMUCH_STATUS_READ_ONLY_DATABASE:
 +case NOTMUCH_STATUS_XAPIAN_EXCEPTION:
 +case NOTMUCH_STATUS_OUT_OF_MEMORY:
 +case NOTMUCH_STATUS_FILE_ERROR:
 +case NOTMUCH_STATUS_NULL_POINTER:
 +case NOTMUCH_STATUS_TAG_TOO_LONG:
 +case NOTMUCH_STATUS_UNBALANCED_FREEZE_THAW:
 +case NOTMUCH_STATUS_UNBALANCED_ATOMIC:
 +case NOTMUCH_STATUS_LAST_STATUS:
 + fprintf (stderr, Error: failed to add `%s' to notmuch database: %s\n,
 +  path, notmuch_status_to_string (status));
 + return FALSE;
 +}
 +
 +notmuch_message_tags_to_maildir_flags (message);
 +
 +notmuch_message_destroy (message);
 +
 +return TRUE;
 +}
 +
 +static notmuch_bool_t
  insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,
   const char *dir)
  {
 @@ -152,6 +188,14 @@ insert_message (void *ctx, notmuch_database_t *notmuch, 
 int fdin,
  
  if (!ret) {
   unlink (tmppath);
 + return FALSE;
 +}
 +
 +ret = save_database (notmuch, newpath);
 +
 +if (!ret) {
 + /* XXX maybe there should be an option to keep the file in maildir? */
 + unlink (newpath);
  }
  
  return ret;
 -- 
 1.7.4.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: [PATCH 10/18] insert: parse command-line tag operations

2012-11-18 Thread Mark Walters

Hi

On Wed, 25 Jul 2012, Peter Wang noval...@gmail.com wrote:
 Parse +tag and -tag on the 'insert' command-line.
 Issue a warning about ambiguous -tag arguments which don't follow
 +tag nor an explicit option list terminator.
 ---
  notmuch-insert.c |   50 ++
  1 files changed, 50 insertions(+), 0 deletions(-)

 diff --git a/notmuch-insert.c b/notmuch-insert.c
 index 4fb3ea3..6db03e3 100644
 --- a/notmuch-insert.c
 +++ b/notmuch-insert.c
 @@ -24,6 +24,11 @@
  #include sys/stat.h
  #include fcntl.h
  
 +typedef struct {
 +const char *tag;
 +notmuch_bool_t remove;
 +} tag_operation_t;
 +
  static notmuch_bool_t
  check_folder_name (const char *folder)
  {
 @@ -236,8 +241,11 @@ notmuch_insert_command (void *ctx, int argc, char 
 *argv[])
  const char **new_tags;
  size_t new_tags_length;
  const char *folder = NULL;
 +tag_operation_t *tag_ops;
 +int tag_ops_count = 0;
  char *maildir;
  int opt_index;
 +notmuch_bool_t warn_tag_rem;
  notmuch_bool_t ret;
  
  notmuch_opt_desc_t options[] = {
 @@ -253,6 +261,48 @@ notmuch_insert_command (void *ctx, int argc, char 
 *argv[])
   return 1;
  }
  
 +if (opt_index  0  strcmp (argv[opt_index - 1], --) == 0) {
 + warn_tag_rem = FALSE;
 +} else {
 + warn_tag_rem = TRUE;
 +}
 +
 +/* Array of tagging operations (add or remove), terminated with an
 + * empty element. */
 +tag_ops = talloc_array (ctx, tag_operation_t, argc - opt_index + 1);
 +if (tag_ops == NULL) {
 + fprintf (stderr, Out of memory.\n);
 + return 1;
 +}
 +
 +for (; opt_index  argc; opt_index++) {
 + if (argv[opt_index][0] == '+') {
 + tag_ops[tag_ops_count].tag = argv[opt_index] + 1;
 + tag_ops[tag_ops_count].remove = FALSE;
 + tag_ops_count++;
 + warn_tag_rem = FALSE;
 + } else if (argv[opt_index][0] == '-') {
 + if (warn_tag_rem) {
 + fprintf (stderr,
 +  Warning: ambiguous argument treated as tag removal: 
 %s\n,
 +  argv[opt_index]);
 + }
 + tag_ops[tag_ops_count].tag = argv[opt_index] + 1;
 + tag_ops[tag_ops_count].remove = TRUE;
 + tag_ops_count++;
 + } else {
 + break;
 + }
 +}

I don't like the code duplication between this and notmuch-tag.c. In
particular the two (now) differ on how they deal with malformed tags
etc. Would it be possible to unify them?

Best wishes

Mark

 +
 +tag_ops[tag_ops_count].tag = NULL;
 +
 +if (opt_index != argc) {
 + fprintf (stderr, Error: bad argument to notmuch insert: %s\n,
 +  argv[opt_index]);
 + return 1;
 +}
 +
  config = notmuch_config_open (ctx, NULL, NULL);
  if (config == NULL)
   return 1;
 -- 
 1.7.4.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: [PATCH 14/18] man: document 'insert' command

2012-11-18 Thread Mark Walters
On Wed, 25 Jul 2012, Peter Wang noval...@gmail.com wrote:
 Add initial documentation for notmuch insert command.
 ---
  man/Makefile.local|1 +
  man/man1/notmuch-insert.1 |   50 
 +
  2 files changed, 51 insertions(+), 0 deletions(-)
  create mode 100644 man/man1/notmuch-insert.1

 diff --git a/man/Makefile.local b/man/Makefile.local
 index d43a949..f91bfd4 100644
 --- a/man/Makefile.local
 +++ b/man/Makefile.local
 @@ -12,6 +12,7 @@ MAN1 := \
   $(dir)/man1/notmuch-count.1 \
   $(dir)/man1/notmuch-dump.1 \
   $(dir)/man1/notmuch-restore.1 \
 + $(dir)/man1/notmuch-insert.1 \
   $(dir)/man1/notmuch-new.1 \
   $(dir)/man1/notmuch-reply.1 \
   $(dir)/man1/notmuch-search.1 \
 diff --git a/man/man1/notmuch-insert.1 b/man/man1/notmuch-insert.1
 new file mode 100644
 index 000..7d281b5
 --- /dev/null
 +++ b/man/man1/notmuch-insert.1
 @@ -0,0 +1,50 @@
 +.TH NOTMUCH-INSERT 1 2012-xx-xx Notmuch 0.xx
 +.SH NAME
 +notmuch-insert \- add a message to the maildir and notmuch database
 +.SH SYNOPSIS
 +
 +.B notmuch insert
 +.RI [ options ]
 +.RI --
 +.RI [ + tag |\- tag  ... ]
 +
 +.SH DESCRIPTION
 +
 +.B notmuch insert
 +reads a message from standard input and delivers it to the specified
 +maildir folder, then incorporates the message into the notmuch
 +database.  It is an alternative to using a separate tool to deliver
 +the message then running
 +.B notmuch new
 +afterwards.
 +
 +The new message will be tagged with the tags specified by the
 +.B new.tags
 +configuration option.
 +Additional tagging operations may be specified on the command-line.
 +Tags prefixed by '+' are added while those prefixed by '\-' are
 +removed.  notmuch will warn about ambiguous tag removal arguments
 +which may be confused with option arguments.  It is recommended to use
 +the option list terminator -- to avoid ambiguity.
 +
 +Supported options for
 +.B insert
 +include
 +.RS 4
 +.TP 4
 +.BI --folder= folder 
 +
 +Deliver the message to the
 +.RI Maildir/ folder
 +directory, which must already exist.
 +The default is to deliver to the top-level directory.

I think this should say something about notmuch database root: so that
by default it delivers to the top-level directory of the notmuch
database, and otherwise is relative to that. Maybe just replace Maildir
by something like notmuch-database-root or something?

Best wishes

Mark



 +
 +.RE
 +.RE
 +.SH SEE ALSO
 +
 +\fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
 +\fBnotmuch-dump\fR(1), \fBnotmuch-hooks\fR(5), \fBnotmuch-reply\fR(1),
 +\fBnotmuch-restore\fR(1), \fBnotmuch-search\fR(1),
 +\fBnotmuch-search-terms\fR(7), \fBnotmuch-show\fR(1),
 +\fBnotmuch-tag\fR(1)
 -- 
 1.7.4.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: [PATCH 16/18] insert: add --create-folder option

2012-11-18 Thread Mark Walters

Hi

On Wed, 25 Jul 2012, Peter Wang noval...@gmail.com wrote:
 Support an option to create a new folder in the maildir.
 ---
  notmuch-insert.c |   71 
 ++
  1 files changed, 71 insertions(+), 0 deletions(-)

 diff --git a/notmuch-insert.c b/notmuch-insert.c
 index a69dfe6..380c520 100644
 --- a/notmuch-insert.c
 +++ b/notmuch-insert.c
 @@ -48,6 +48,70 @@ check_folder_name (const char *folder)
  }
  }
  
 +static int
 +mkdir_parents (void *ctx, const char *path, int mode)
 +{
 +struct stat st;
 +char *pathcopy;
 +char *start;
 +char *end;
 +int ret;
 +
 +/* First check the common case: directory already exists. */
 +if (stat (path, st) == 0) {
 + return (S_ISDIR (st.st_mode)) ? 0 : -1;
 +}
 +
 +pathcopy = talloc_strdup (ctx, path);
 +ret = 0;
 +
 +for (start = pathcopy; *start != '\0'; start = end + 1) {
 + end = strchr (start + 1, '/');
 + if (!end) {
 + ret = mkdir (path, mode);
 + break;
 + }
 + *end = '\0';
 + ret = mkdir (pathcopy, mode);
 + if (ret != 0  errno != EEXIST) {
 + break;
 + }
 + *end = '/';
 +}
I am a little confused by this: why is there a +1 in the start = end + 1
line and in the end = strchr line? I believe it doesn't matter but it
makes me wonder if I am missing something.

Best wishes

Mark



 +
 +talloc_free (pathcopy);
 +
 +return ret;
 +}
 +
 +static notmuch_bool_t
 +maildir_create (void *ctx, const char *dir)
 +{
 +const int mode = 0755;
 +char *subdir;
 +char *end;
 +
 +/* Create 'cur' directory, including parent directories. */
 +subdir = talloc_asprintf (ctx, %s/cur, dir);
 +if (mkdir_parents (ctx, subdir, mode) != 0)
 + return FALSE;
 +
 +end = subdir + strlen (subdir);
 +
 +/* Create 'new' directory. */
 +strcpy (end - 3, new);
 +if (mkdir (subdir, mode) != 0  errno != EEXIST)
 + return FALSE;
 +
 +/* Create 'tmp' directory. */
 +strcpy (end - 3, tmp);
 +if (mkdir (subdir, mode) != 0  errno != EEXIST)
 + return FALSE;
 +
 +talloc_free (subdir);
 +return TRUE;
 +}
 +
  static notmuch_bool_t
  safe_gethostname (char *hostname, size_t hostname_size)
  {
 @@ -253,6 +317,7 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
  const char **new_tags;
  size_t new_tags_length;
  const char *folder = NULL;
 +notmuch_bool_t create_folder = FALSE;
  tag_operation_t *tag_ops;
  int tag_ops_count = 0;
  char *maildir;
 @@ -262,6 +327,7 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
  
  notmuch_opt_desc_t options[] = {
   { NOTMUCH_OPT_STRING, folder, folder, 0, 0 },
 + { NOTMUCH_OPT_BOOLEAN, create_folder, create-folder, 0, 0 },
   { NOTMUCH_OPT_END, 0, 0, 0, 0 }
  };
  
 @@ -328,6 +394,11 @@ notmuch_insert_command (void *ctx, int argc, char 
 *argv[])
   return 1;
   }
   maildir = talloc_asprintf (ctx, %s/%s, db_path, folder);
 + if (create_folder  ! maildir_create (ctx, maildir)) {
 + fprintf (stderr, Error: creating maildir %s: %s\n,
 +  maildir, strerror (errno));
 + return 1;
 + }
  } else {
   maildir = talloc_asprintf (ctx, %s, db_path);
  }
 -- 
 1.7.4.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: [PATCH 05/18] insert: move file from Maildir tmp to new

2012-11-18 Thread Mark Walters
On Wed, 25 Jul 2012, Peter Wang noval...@gmail.com wrote:
 Atomically move the new message file from the Maildir 'tmp' directory
 to 'new'.
 ---
  notmuch-insert.c |   18 ++
  1 files changed, 18 insertions(+), 0 deletions(-)

 diff --git a/notmuch-insert.c b/notmuch-insert.c
 index 340f7e4..bab1fed 100644
 --- a/notmuch-insert.c
 +++ b/notmuch-insert.c
 @@ -75,6 +75,20 @@ maildir_open_tmp (void *ctx, const char *dir, char 
 **tmppath, char **newpath)
  }
  
  static notmuch_bool_t
 +maildir_move_to_new (const char *tmppath, const char *newpath)
 +{
 +/* We follow the Dovecot recommendation to simply use rename()
 + * instead of link() and unlink().
 + */
 +if (rename (tmppath, newpath) == 0) {
 + return TRUE;
 +}

Do we want to overwrite an existing message with this name? As far as I
can see rename does overwrite and link would not: was that why rename is
better than link/unlink?

I would prefer not to overwrite but maybe there is a reason we need to. 
Would a possible alternative be to loop when finding a tmp file until
both the tmp file and the new file do not exist?

Best wishes

Mark


 +
 +fprintf (stderr, Error: rename() failed: %s\n, strerror (errno));
 +return FALSE;
 +}
 +
 +static notmuch_bool_t
  copy_fd_data (int fdin, int fdout)
  {
  char buf[4096];
 @@ -132,6 +146,10 @@ insert_message (void *ctx, notmuch_database_t *notmuch, 
 int fdin,
  
  close (fdout);
  
 +if (ret) {
 + ret = maildir_move_to_new (tmppath, newpath);
 +}
 +
  if (!ret) {
   unlink (tmppath);
  }
 -- 
 1.7.4.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: [PATCH 01/18] cli: add stub for insert command

2012-11-18 Thread Mark Walters

Hi

I have now been through essentially the whole series (except the tests)
and broadly like it. Just to summarise my concerns from the individual
replies here:

What should insert do when interrupted and is it safe? I am not
knowledgeable enough to be confident about that.

I think some of the talloc allocations need their return values
checked. I am more worried about this here then in most of the rest of
notmuch as we are writing to the database (and even to the mailstore
itself).

My other concerns (code duplication with notmuch-tag.c and possible
deadlock in the tmp file code and file overwriting) are easily fixable

Best wishes

Mark




On Wed, 25 Jul 2012, Peter Wang noval...@gmail.com wrote:
 This does nothing yet.
 ---
  Makefile.local   |1 +
  notmuch-client.h |3 +++
  notmuch-insert.c |   27 +++
  notmuch.c|3 +++
  4 files changed, 34 insertions(+), 0 deletions(-)
  create mode 100644 notmuch-insert.c

 diff --git a/Makefile.local b/Makefile.local
 index 296995d..950f046 100644
 --- a/Makefile.local
 +++ b/Makefile.local
 @@ -282,6 +282,7 @@ notmuch_client_srcs = \
   notmuch-config.c\
   notmuch-count.c \
   notmuch-dump.c  \
 + notmuch-insert.c\
   notmuch-new.c   \
   notmuch-reply.c \
   notmuch-restore.c   \
 diff --git a/notmuch-client.h b/notmuch-client.h
 index f930798..edbd3ee 100644
 --- a/notmuch-client.h
 +++ b/notmuch-client.h
 @@ -132,6 +132,9 @@ int
  notmuch_dump_command (void *ctx, int argc, char *argv[]);
  
  int
 +notmuch_insert_command (void *ctx, int argc, char *argv[]);
 +
 +int
  notmuch_new_command (void *ctx, int argc, char *argv[]);
  
  int
 diff --git a/notmuch-insert.c b/notmuch-insert.c
 new file mode 100644
 index 000..0e061a0
 --- /dev/null
 +++ b/notmuch-insert.c
 @@ -0,0 +1,27 @@
 +/* notmuch - Not much of an email program, (just index and search)
 + *
 + * Copyright © 2012 Peter Wang
 + *
 + * This program is free software: you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License as published by
 + * the Free Software Foundation, either version 3 of the License, or
 + * (at your option) any later version.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program.  If not, see http://www.gnu.org/licenses/ .
 + *
 + * Author: Peter Wang noval...@gmail.com
 + */
 +
 +#include notmuch-client.h
 +
 +int
 +notmuch_insert_command (void *ctx, int argc, char *argv[])
 +{
 +return 1;
 +}
 diff --git a/notmuch.c b/notmuch.c
 index 477a09c..86239fd 100644
 --- a/notmuch.c
 +++ b/notmuch.c
 @@ -53,6 +53,9 @@ static command_t commands[] = {
  { new, notmuch_new_command,
[options...],
Find and import new messages to the notmuch database. },
 +{ insert, notmuch_insert_command,
 +  [options...] [--] [+tag|-tag ...]  message,
 +  Add a new message into the maildir and notmuch database. },
  { search, notmuch_search_command,
[options...] search-terms [...],
Search for messages matching the given search terms. },
 -- 
 1.7.4.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] Saving attachments containing UTF-8 chars

2012-11-18 Thread Ethan Glasser-Camp
Ethan Glasser-Camp ethan.glasser.c...@gmail.com writes:

 Writing this buffer using C-x C-w encodes it correctly too. So I think
 this is an emacs MIME problem. We call mm-save-part, which calls
 mm-save-part-to-file, which calls mm-with-unibyte-buffer. Hmm..

 Indeed, it seems that inserting this character into a file that's been
 marked unibyte using (set-buffer-multibyte nil) turns it into the ^Y
 character (ASCII code 0x19 -- the character that comes out in the patch
 file). There's probably a technical reason that this should be true, but
 I can't think of why that would be.

The more I think about this, the more convinced I become that this is a
bug in emacs's multibyte handling. I've filed a bug, see:

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=12925

But I think Tomi's going to formalize his hacky patch and send that out
later too. When he does, it has my +1.

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


Add new dump/restore format and batch tagging.

2012-11-18 Thread david
This is a patch series with some history, if you will forgive the
semi-inadvertant pun. I list that at that end, for the curious.

If this series goes in, in the future we might consider whether
restore --accumulate provides important functionality over batch
tagging; at the moment I suggest leaving it as the extra code to
support it is minimal, and it does support peoples existing scripts
using the old dump/restore format.

History
---

About 1 year ago, Petter Reinholdtsen observed a problem with dumping
and restoring message-id's with spaces in them. 

id:2flfwhht87d@diskless.uio.no

There followed a proposed fix

  id:1323808075-7417-1-git-send-email-da...@tethera.net

Which Dmitry had a few helpful things to say about the hex encoding
libs.

Jani took that foundation and proposed two versions of the batch
tagging, most recently at

id:cover.1334404979.git.j...@nikula.org. 

There was some discussion with Jamie about the file format for batch
tagging in the thread

 id:cover.1333231401.git.j...@nikula.org

id:1323808075-7417-1-git-send-email-da...@tethera.net
The first 6 of these patches obsolete the series

id:1345382314-5330-1-git-send-email-da...@tethera.net

which was revied by Tomi and Ethan. I think I implemented their
suggestions.

Although I didn't re-read that whole thread, I believe this version of
the patches address's Jamie's concerns by using exactly the same
format for restore and tag --batch (renamed from Jani's choice of
--stdin).

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


[PATCH 02/16] test/hex-xcode: new test binary

2012-11-18 Thread david
From: David Bremner brem...@debian.org

This program is used both as a test-bed/unit-tester for
../util/hex-escape.c, and also as a utility in future tests of dump
and restore.
---
 test/.gitignore |1 +
 test/Makefile.local |   13 ++-
 test/basic  |1 +
 test/hex-xcode.c|  103 +++
 4 files changed, 116 insertions(+), 2 deletions(-)
 create mode 100644 test/hex-xcode.c

diff --git a/test/.gitignore b/test/.gitignore
index e63c689..be7ab5e 100644
--- a/test/.gitignore
+++ b/test/.gitignore
@@ -3,4 +3,5 @@ corpus.mail
 smtp-dummy
 symbol-test
 arg-test
+hex-xcode
 tmp.*
diff --git a/test/Makefile.local b/test/Makefile.local
index 9ae130a..8da4c56 100644
--- a/test/Makefile.local
+++ b/test/Makefile.local
@@ -13,6 +13,9 @@ smtp_dummy_modules = $(smtp_dummy_srcs:.c=.o)
 $(dir)/arg-test: $(dir)/arg-test.o command-line-arguments.o util/libutil.a
$(call quiet,CC) -I. $^ -o $@
 
+$(dir)/hex-xcode: $(dir)/hex-xcode.o command-line-arguments.o util/libutil.a
+   $(call quiet,CC) -I. $^ -o $@ -ltalloc
+
 $(dir)/smtp-dummy: $(smtp_dummy_modules)
$(call quiet,CC) $^ -o $@
 
@@ -24,8 +27,13 @@ $(dir)/parse-time: $(dir)/parse-time.o 
parse-time-string/parse-time-string.o
 
 .PHONY: test check
 
-test-binaries: $(dir)/arg-test $(dir)/smtp-dummy $(dir)/symbol-test \
-   $(dir)/parse-time
+TEST_BINARIES=$(dir)/arg-test \
+ $(dir)/hex-xcode \
+ $(dir)/parse-time \
+ $(dir)/smtp-dummy \
+ $(dir)/symbol-test
+
+test-binaries: $(TEST_BINARIES)
 
 test:  all test-binaries
@${dir}/notmuch-test $(OPTIONS)
@@ -36,5 +44,6 @@ SRCS := $(SRCS) $(smtp_dummy_srcs)
 CLEAN := $(CLEAN) $(dir)/smtp-dummy $(dir)/smtp-dummy.o \
 $(dir)/symbol-test $(dir)/symbol-test.o \
 $(dir)/arg-test $(dir)/arg-test.o \
+$(dir)/hex-xcode $(dir)/hex-xcode.o \
 $(dir)/parse-time $(dir)/parse-time.o \
 $(dir)/corpus.mail $(dir)/test-results $(dir)/tmp.*
diff --git a/test/basic b/test/basic
index 1b842d2..2a571ac 100755
--- a/test/basic
+++ b/test/basic
@@ -56,6 +56,7 @@ tests_in_suite=$(for i in $TESTS; do echo $i; done | sort)
 available=$(find $TEST_DIRECTORY -maxdepth 1 -type f -perm +111  \
 ! -name aggregate-results.sh   \
 ! -name arg-test   \
+! -name hex-xcode  \
 ! -name notmuch-test   \
 ! -name parse-time \
 ! -name smtp-dummy \
diff --git a/test/hex-xcode.c b/test/hex-xcode.c
new file mode 100644
index 000..eec6541
--- /dev/null
+++ b/test/hex-xcode.c
@@ -0,0 +1,103 @@
+/* No, nothing to to with IDE from Apple Inc.
+   testbed for ../util/hex-escape.c.
+
+   usage:
+   hex-xcode [--direction=(encode|decode)] [--omit-newline]  file
+   hex-xcode [--direction=(encode|decode)] [--omit-newline] arg1 arg2 arg3 ...
+
+ */
+
+#include notmuch-client.h
+#include hex-escape.h
+#include assert.h
+
+
+enum direction {
+ENCODE,
+DECODE
+};
+
+static int
+xcode (void *ctx, enum direction dir, char *in, char **buf_p, size_t *size_p)
+{
+hex_status_t status;
+
+if (dir == ENCODE)
+   status = hex_encode (ctx, in, buf_p, size_p);
+else
+   status = hex_decode (ctx, in, buf_p, size_p);
+
+if (status == HEX_SUCCESS)
+   fputs (*buf_p, stdout);
+
+return status;
+}
+
+
+int
+main (int argc, char **argv)
+{
+
+
+enum direction dir = DECODE;
+int omit_newline = FALSE;
+
+notmuch_opt_desc_t options[] = {
+   { NOTMUCH_OPT_KEYWORD, dir, direction, 'd',
+ (notmuch_keyword_t []){ { encode, ENCODE },
+ { decode, DECODE },
+ { 0, 0 } } },
+   { NOTMUCH_OPT_BOOLEAN, omit_newline, omit-newline, 'n', 0 },
+   { 0, 0, 0, 0, 0 }
+};
+
+int opt_index = parse_arguments (argc, argv, options, 1);
+
+if (opt_index  0)
+   exit (1);
+
+void *ctx = talloc_new (NULL);
+
+char *line = NULL;
+size_t line_size;
+ssize_t line_len;
+
+char *buffer = NULL;
+size_t buf_size = 0;
+
+notmuch_bool_t read_stdin = TRUE;
+
+for (; opt_index  argc; opt_index++) {
+
+   if (xcode (ctx, dir, argv[opt_index],
+  buffer, buf_size) != HEX_SUCCESS)
+   return 1;
+
+   if (!omit_newline)
+   putchar ('\n');
+
+   read_stdin = FALSE;
+}
+
+if (!read_stdin)
+   return 0;
+
+while ((line_len = getline (line, line_size, stdin)) != -1) {
+
+   chomp_newline (line);
+
+   if (xcode (ctx, dir, line, buffer, buf_size) != HEX_SUCCESS)
+   return 1;
+
+   if (!omit_newline)
+   putchar ('\n');
+
+}
+
+if (line)
+   free (line);
+
+talloc_free (ctx);
+
+return 0;
+}
-- 
1.7.10.4

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


[PATCH 03/16] test/hex-escaping: new test for hex escaping routines

2012-11-18 Thread david
From: David Bremner brem...@debian.org

These are more like unit tests, to (try to) make sure the library
functionality is working before building more complicated things on
top of it.
---
 test/hex-escaping |   26 ++
 test/notmuch-test |1 +
 2 files changed, 27 insertions(+)
 create mode 100755 test/hex-escaping

diff --git a/test/hex-escaping b/test/hex-escaping
new file mode 100755
index 000..f34cc8c
--- /dev/null
+++ b/test/hex-escaping
@@ -0,0 +1,26 @@
+#!/usr/bin/env bash
+test_description=hex encoding and decoding
+. ./test-lib.sh
+
+test_begin_subtest round trip
+find $TEST_DIRECTORY/corpus -type f -print | sort | xargs cat  EXPECTED
+$TEST_DIRECTORY/hex-xcode --direction=encode  EXPECTED | 
$TEST_DIRECTORY/hex-xcode --direction=decode  OUTPUT
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest punctuation
+tag1='comic_swear=$^%$^%\\//-+$^%$'
+tag_enc1=$($TEST_DIRECTORY/hex-xcode --direction=encode $tag1)
+test_expect_equal $tag_enc1 
comic_swear=%24%26%5e%25%24%5e%25%5c%5c%2f%2f-+%24%5e%25%24
+
+test_begin_subtest round trip newlines
+printf 'this\n tag\t has\n spaces\n'  EXPECTED.$test_count
+$TEST_DIRECTORY/hex-xcode --direction=encode   EXPECTED.$test_count |\
+   $TEST_DIRECTORY/hex-xcode --direction=decode  OUTPUT.$test_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+
+test_begin_subtest round trip 8bit chars
+echo '%c3%91%c3%a5%c3%b0%c3%a3%c3%a5%c3%a9-%c3%8f%c3%8a'  EXPECTED.$test_count
+$TEST_DIRECTORY/hex-xcode --direction=decode   EXPECTED.$test_count |\
+   $TEST_DIRECTORY/hex-xcode --direction=encode  OUTPUT.$test_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+test_done
diff --git a/test/notmuch-test b/test/notmuch-test
index 9a1b375..d2e90e2 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -60,6 +60,7 @@ TESTS=
   emacs-hello
   emacs-show
   missing-headers
+  hex-escaping
   parse-time-string
   search-date
 
-- 
1.7.10.4

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


[PATCH 01/16] hex-escape: (en|de)code strings to/from restricted character set

2012-11-18 Thread david
From: David Bremner brem...@debian.org

The character set is chosen to be suitable for pathnames, and the same
as that used by contrib/nmbug

[With additions by Jani Nikula]
---
 util/Makefile.local |2 +-
 util/hex-escape.c   |  168 +++
 util/hex-escape.h   |   41 +
 3 files changed, 210 insertions(+), 1 deletion(-)
 create mode 100644 util/hex-escape.c
 create mode 100644 util/hex-escape.h

diff --git a/util/Makefile.local b/util/Makefile.local
index c7cae61..3ca623e 100644
--- a/util/Makefile.local
+++ b/util/Makefile.local
@@ -3,7 +3,7 @@
 dir := util
 extra_cflags += -I$(srcdir)/$(dir)
 
-libutil_c_srcs := $(dir)/xutil.c $(dir)/error_util.c
+libutil_c_srcs := $(dir)/xutil.c $(dir)/error_util.c $(dir)/hex-escape.c
 
 libutil_modules := $(libutil_c_srcs:.c=.o)
 
diff --git a/util/hex-escape.c b/util/hex-escape.c
new file mode 100644
index 000..d8905d0
--- /dev/null
+++ b/util/hex-escape.c
@@ -0,0 +1,168 @@
+/* hex-escape.c -  Manage encoding and decoding of byte strings into path names
+ *
+ * Copyright (c) 2011 David Bremner
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ *
+ * Author: David Bremner da...@tethera.net
+ */
+
+#include assert.h
+#include string.h
+#include talloc.h
+#include ctype.h
+#include error_util.h
+#include hex-escape.h
+
+static const size_t default_buf_size = 1024;
+
+static const char *output_charset =
+ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+-_@=.:,;
+
+static const char escape_char = '%';
+
+static int
+is_output (char c)
+{
+return (strchr (output_charset, c) != NULL);
+}
+
+static int
+maybe_realloc (void *ctx, size_t needed, char **out, size_t *out_size)
+{
+if (*out_size  needed) {
+
+   if (*out == NULL)
+   *out = talloc_size (ctx, needed);
+   else
+   *out = talloc_realloc (ctx, *out, char, needed);
+
+   if (*out == NULL)
+   return 0;
+
+   *out_size = needed;
+}
+return 1;
+}
+
+hex_status_t
+hex_encode (void *ctx, const char *in, char **out, size_t *out_size)
+{
+
+const unsigned char *p;
+char *q;
+
+size_t escape_count = 0;
+size_t len = 0;
+size_t needed;
+
+assert (ctx); assert (in); assert (out); assert (out_size);
+
+for (p = (unsigned char *) in; *p; p++) {
+   escape_count += (!is_output (*p));
+   len++;
+}
+
+needed = len + escape_count * 2 + 1;
+
+if (*out == NULL)
+   *out_size = 0;
+
+if (!maybe_realloc (ctx, needed, out, out_size))
+   return HEX_OUT_OF_MEMORY;
+
+q = *out;
+p = (unsigned char *) in;
+
+while (*p) {
+   if (is_output (*p)) {
+   *q++ = *p++;
+   } else {
+   sprintf (q, %%%02x, *p++);
+   q += 3;
+   }
+}
+
+*q = '\0';
+return HEX_SUCCESS;
+}
+
+/* Hex decode 'in' to 'out'.
+ *
+ * This must succeed for in == out to support hex_decode_inplace().
+ */
+static hex_status_t
+hex_decode_internal (const char *in, unsigned char *out)
+{
+char buf[3];
+
+while (*in) {
+   if (*in == escape_char) {
+   char *endp;
+
+   /* This also handles unexpected end-of-string. */
+   if (!isxdigit ((unsigned char) in[1]) ||
+   !isxdigit ((unsigned char) in[2]))
+   return HEX_SYNTAX_ERROR;
+
+   buf[0] = in[1];
+   buf[1] = in[2];
+   buf[2] = '\0';
+
+   *out = strtoul (buf, endp, 16);
+
+   if (endp != buf + 2)
+   return HEX_SYNTAX_ERROR;
+
+   in += 3;
+   out++;
+   } else {
+   *out++ = *in++;
+   }
+}
+
+*out = '\0';
+
+return HEX_SUCCESS;
+}
+
+hex_status_t
+hex_decode_inplace (char *s)
+{
+/* A decoded string is never longer than the encoded one, so it is
+ * safe to decode a string onto itself. */
+return hex_decode_internal (s, (unsigned char *) s);
+}
+
+hex_status_t
+hex_decode (void *ctx, const char *in, char **out, size_t * out_size)
+{
+const char *p;
+size_t escape_count = 0;
+size_t needed = 0;
+
+assert (ctx); assert (in); assert (out); assert (out_size);
+
+size_t len = strlen (in);
+
+for (p = in; *p; p++)
+   escape_count += (*p == escape_char);
+
+needed = len - escape_count * 2 + 1;
+
+if (!maybe_realloc (ctx, needed, out, out_size))
+   return 

[PATCH 06/16] test: add broken roundtrip test

2012-11-18 Thread david
From: David Bremner brem...@debian.org

We demonstrate the current notmuch restore parser being confused by
message-id's and tags containing non alpha numeric characters
(particularly space and parentheses are problematic because they are
not escaped by notmuch dump).

We save the files as hex escaped on disk so that the output from the
failing test will not confuse the terminal emulator of people running
the test.
---
 test/dump-restore |9 +
 1 file changed, 9 insertions(+)

diff --git a/test/dump-restore b/test/dump-restore
index b05399c..a2204fb 100755
--- a/test/dump-restore
+++ b/test/dump-restore
@@ -85,4 +85,13 @@ test_begin_subtest dump --output=outfile -- from:cworth
 notmuch dump --output=dump-outfile-dash-inbox.actual -- from:cworth
 test_expect_equal_file dump-cworth.expected dump-outfile-dash-inbox.actual
 
+test_expect_success 'roundtripping random message-ids and tags' \
+'test_subtest_known_broken 
+ ${TEST_DIRECTORY}/random-corpus --num-messages=10 
--config-path=${NOTMUCH_CONFIG} 
+ notmuch dump | ${TEST_DIRECTORY}/hex-xcode --direction=encode  
EXPECTED.$test_count 
+ notmuch tag -random-corpus tag:random-corpus 
+ ${TEST_DIRECTORY}/hex-xcode --direction=decode  EXPECTED.$test_count | 
notmuch restore 2/dev/null 
+ notmuch dump | ${TEST_DIRECTORY}/hex-xcode --direction=encode  
OUTPUT.$test_count 
+ test_cmp EXPECTED.$test_count OUTPUT.$test_count'
+
 test_done
-- 
1.7.10.4

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


[PATCH 04/16] test: add database routines for testing.

2012-11-18 Thread david
From: David Bremner brem...@debian.org

Initially, provide a way to create stub messages in the notmuch
database without corresponding files.  This is essentially cut and
paste from lib/database.cc. This is a seperate file since we don't
want to export these symbols from libnotmuch or bloat the library with
non-exported code.
---
 test/Makefile.local  |1 +
 test/database-test.c |   72 ++
 test/database-test.h |   21 +++
 3 files changed, 94 insertions(+)
 create mode 100644 test/database-test.c
 create mode 100644 test/database-test.h

diff --git a/test/Makefile.local b/test/Makefile.local
index 8da4c56..8479f91 100644
--- a/test/Makefile.local
+++ b/test/Makefile.local
@@ -45,5 +45,6 @@ CLEAN := $(CLEAN) $(dir)/smtp-dummy $(dir)/smtp-dummy.o \
 $(dir)/symbol-test $(dir)/symbol-test.o \
 $(dir)/arg-test $(dir)/arg-test.o \
 $(dir)/hex-xcode $(dir)/hex-xcode.o \
+$(dir)/database-test.o \
 $(dir)/parse-time $(dir)/parse-time.o \
 $(dir)/corpus.mail $(dir)/test-results $(dir)/tmp.*
diff --git a/test/database-test.c b/test/database-test.c
new file mode 100644
index 000..f0f1c8e
--- /dev/null
+++ b/test/database-test.c
@@ -0,0 +1,72 @@
+/*
+ * Database routines intended only for testing, not exported from
+ * library.
+ *
+ * Copyright (c) 2012 David Bremner
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ *
+ * Author: David Bremner da...@tethera.net
+ */
+
+#include notmuch-private.h
+#include database-test.h
+
+notmuch_status_t
+notmuch_database_add_stub_message (notmuch_database_t *notmuch,
+  const char *message_id,
+  const char **tags)
+{
+const char **tag;
+notmuch_status_t ret;
+notmuch_private_status_t private_status;
+notmuch_message_t *message;
+
+ret = _notmuch_database_ensure_writable (notmuch);
+if (ret)
+   return ret;
+
+message = _notmuch_message_create_for_message_id (notmuch,
+ message_id,
+ private_status);
+if (message == NULL) {
+   return COERCE_STATUS (private_status,
+ Unexpected status value from 
_notmuch_message_create_for_message_id);
+
+}
+
+if (private_status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) {
+   _notmuch_message_add_term (message, type, mail);
+} else {
+   return NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID;
+}
+
+if (tags) {
+   ret = notmuch_message_freeze (message);
+   if (ret)
+   return ret;
+
+   for (tag = tags; *tag; tag++) {
+   ret = notmuch_message_add_tag (message, *tag);
+   if (ret)
+   return ret;
+   }
+}
+
+ret = notmuch_message_thaw (message);
+if (ret)
+   return ret;
+
+return NOTMUCH_STATUS_SUCCESS;
+}
diff --git a/test/database-test.h b/test/database-test.h
new file mode 100644
index 000..84f7988
--- /dev/null
+++ b/test/database-test.h
@@ -0,0 +1,21 @@
+#ifndef _DATABASE_TEST_H
+#define _DATABASE_TEST_H
+/* Add a new stub message to the given notmuch database.
+ *
+ * At least the following return values are possible:
+ *
+ * NOTMUCH_STATUS_SUCCESS: Message successfully added to database.
+ *
+ * NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID: Message has the same message
+ * ID as another message already in the database.
+ *
+ * NOTMUCH_STATUS_READ_ONLY_DATABASE: Database was opened in read-only
+ * mode so no message can be added.
+ */
+
+notmuch_status_t
+notmuch_database_add_stub_message (notmuch_database_t *database,
+  const char *message_id,
+  const char **tag_list);
+
+#endif
-- 
1.7.10.4

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


[PATCH 07/16] notmuch-dump: add --format=(batch-tag|sup)

2012-11-18 Thread david
From: David Bremner brem...@debian.org

sup is the old format, and remains the default, at least until
restore is converted to parse this format.

Each line of the batch- format is valid for notmuch tag, (with the
notmuch tag omitted from the front of the line). The dump format
only uses query strings of a single message-id.

Each space seperated tag/message-id is 'hex-encoded' to remove
troubling characters.  In particular this format won't have the same
problem with e.g. spaces in message-ids or tags; they will be
round-trip-able.
---
 dump-restore-private.h |   13 +
 notmuch-dump.c |   42 --
 2 files changed, 49 insertions(+), 6 deletions(-)
 create mode 100644 dump-restore-private.h

diff --git a/dump-restore-private.h b/dump-restore-private.h
new file mode 100644
index 000..896a004
--- /dev/null
+++ b/dump-restore-private.h
@@ -0,0 +1,13 @@
+#ifndef DUMP_RESTORE_PRIVATE_H
+#define DUMP_RESTORE_PRIVATE_H
+
+#include hex-escape.h
+#include command-line-arguments.h
+
+typedef enum dump_formats {
+DUMP_FORMAT_AUTO,
+DUMP_FORMAT_BATCH_TAG,
+DUMP_FORMAT_SUP
+} dump_format_t;
+
+#endif
diff --git a/notmuch-dump.c b/notmuch-dump.c
index 88f598a..045ca9e 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -19,6 +19,7 @@
  */
 
 #include notmuch-client.h
+#include dump-restore-private.h
 
 int
 notmuch_dump_command (unused (void *ctx), int argc, char *argv[])
@@ -43,7 +44,13 @@ notmuch_dump_command (unused (void *ctx), int argc, char 
*argv[])
 char *output_file_name = NULL;
 int opt_index;
 
+int output_format = DUMP_FORMAT_SUP;
+
 notmuch_opt_desc_t options[] = {
+   { NOTMUCH_OPT_KEYWORD, output_format, format, 'f',
+ (notmuch_keyword_t []){ { sup, DUMP_FORMAT_SUP },
+ { batch-tag, DUMP_FORMAT_BATCH_TAG },
+ { 0, 0 } } },
{ NOTMUCH_OPT_STRING, output_file_name, output, 'o', 0  },
{ 0, 0, 0, 0, 0 }
 };
@@ -83,27 +90,50 @@ notmuch_dump_command (unused (void *ctx), int argc, char 
*argv[])
  */
 notmuch_query_set_sort (query, NOTMUCH_SORT_UNSORTED);
 
+char *buffer = NULL;
+size_t buffer_size = 0;
+
 for (messages = notmuch_query_search_messages (query);
 notmuch_messages_valid (messages);
 notmuch_messages_move_to_next (messages)) {
int first = 1;
+   const char *message_id;
+
message = notmuch_messages_get (messages);
+   message_id = notmuch_message_get_message_id (message);
 
-   fprintf (output,
-%s (, notmuch_message_get_message_id (message));
+   if (output_format == DUMP_FORMAT_SUP) {
+   fprintf (output, %s (, message_id);
+   }
 
for (tags = notmuch_message_get_tags (message);
 notmuch_tags_valid (tags);
 notmuch_tags_move_to_next (tags)) {
-   if (! first)
-   fprintf (output,  );
+   const char *tag_str = notmuch_tags_get (tags);
 
-   fprintf (output, %s, notmuch_tags_get (tags));
+   if (! first)
+   fputs ( , output);
 
first = 0;
+
+   if (output_format == DUMP_FORMAT_SUP) {
+   fputs (tag_str, output);
+   } else {
+   if (hex_encode (notmuch, tag_str,
+   buffer, buffer_size) != HEX_SUCCESS)
+   return 1;
+   fprintf (output, +%s, buffer);
+   }
}
 
-   fprintf (output, )\n);
+   if (output_format == DUMP_FORMAT_SUP) {
+   fputs ()\n, output);
+   } else {
+   if (hex_encode (notmuch, message_id,
+   buffer, buffer_size) != HEX_SUCCESS)
+   return 1;
+   fprintf (output,  -- id:%s\n, buffer);
+   }
 
notmuch_message_destroy (message);
 }
-- 
1.7.10.4

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


[PATCH 05/16] test: add generator for random stub messages

2012-11-18 Thread david
From: David Bremner brem...@debian.org

Initial use case is testing dump and restore, so we only have
message-ids and tags.

The message ID's are nothing like RFC compliant, but it doesn't seem
any harder to roundtrip random UTF-8 strings than RFC-compliant ones.

Tags are UTF-8, even though notmuch is in principle more generous than
that.

updated for id:m2wr04ocro@guru.guru-group.fi

- talk about Unicode value rather some specific encoding
- call talloc_realloc less times
---
 test/.gitignore  |1 +
 test/Makefile.local  |   10 +++
 test/basic   |1 +
 test/random-corpus.c |  197 ++
 4 files changed, 209 insertions(+)
 create mode 100644 test/random-corpus.c

diff --git a/test/.gitignore b/test/.gitignore
index be7ab5e..1eff7ce 100644
--- a/test/.gitignore
+++ b/test/.gitignore
@@ -4,4 +4,5 @@ smtp-dummy
 symbol-test
 arg-test
 hex-xcode
+random-corpus
 tmp.*
diff --git a/test/Makefile.local b/test/Makefile.local
index 8479f91..6a9f15e 100644
--- a/test/Makefile.local
+++ b/test/Makefile.local
@@ -16,6 +16,14 @@ $(dir)/arg-test: $(dir)/arg-test.o command-line-arguments.o 
util/libutil.a
 $(dir)/hex-xcode: $(dir)/hex-xcode.o command-line-arguments.o util/libutil.a
$(call quiet,CC) -I. $^ -o $@ -ltalloc
 
+random_corpus_deps =  $(dir)/random-corpus.o  $(dir)/database-test.o \
+   notmuch-config.o command-line-arguments.o \
+   lib/libnotmuch.a util/libutil.a \
+   parse-time-string/libparse-time-string.a
+
+$(dir)/random-corpus: $(random_corpus_deps)
+   $(call quiet,CC) $(CFLAGS_FINAL) $^ -o $@ $(CONFIGURE_LDFLAGS)
+
 $(dir)/smtp-dummy: $(smtp_dummy_modules)
$(call quiet,CC) $^ -o $@
 
@@ -29,6 +37,7 @@ $(dir)/parse-time: $(dir)/parse-time.o 
parse-time-string/parse-time-string.o
 
 TEST_BINARIES=$(dir)/arg-test \
  $(dir)/hex-xcode \
+ $(dir)/random-corpus \
  $(dir)/parse-time \
  $(dir)/smtp-dummy \
  $(dir)/symbol-test
@@ -46,5 +55,6 @@ CLEAN := $(CLEAN) $(dir)/smtp-dummy $(dir)/smtp-dummy.o \
 $(dir)/arg-test $(dir)/arg-test.o \
 $(dir)/hex-xcode $(dir)/hex-xcode.o \
 $(dir)/database-test.o \
+$(dir)/random-corpus $(dir)/random-corpus.o \
 $(dir)/parse-time $(dir)/parse-time.o \
 $(dir)/corpus.mail $(dir)/test-results $(dir)/tmp.*
diff --git a/test/basic b/test/basic
index 2a571ac..f93469f 100755
--- a/test/basic
+++ b/test/basic
@@ -59,6 +59,7 @@ available=$(find $TEST_DIRECTORY -maxdepth 1 -type f -perm 
+111  \
 ! -name hex-xcode  \
 ! -name notmuch-test   \
 ! -name parse-time \
+! -name random-corpus  \
 ! -name smtp-dummy \
 ! -name symbol-test\
 ! -name test-verbose   \
diff --git a/test/random-corpus.c b/test/random-corpus.c
new file mode 100644
index 000..91e98b1
--- /dev/null
+++ b/test/random-corpus.c
@@ -0,0 +1,197 @@
+/*
+ * Generate a random corpus of stub messages.
+ *
+ * Initial use case is testing dump and restore, so we only have
+ * message-ids and tags.
+ *
+ * Generated message-id's and tags are intentionally nasty.
+ *
+ * Copyright (c) 2012 David Bremner
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ *
+ * Author: David Bremner da...@tethera.net
+ */
+
+#include stdlib.h
+#include assert.h
+#include talloc.h
+#include string.h
+#include glib.h
+#include math.h
+
+#include notmuch-client.h
+#include command-line-arguments.h
+#include database-test.h
+
+/* Current largest Unicode value defined. Note that most of these will
+ * be printed as boxes in most fonts.
+ */
+
+#define GLYPH_MAX 0x10FFFE
+
+static gunichar
+random_unichar ()
+{
+int start = 1, stop = GLYPH_MAX;
+int class = random() % 2;
+
+/*
+ *  Choose about half ascii as test characters, as ascii
+ *  punctation and whitespace is the main cause of problems for
+ *  the (old) restore parser
+*/
+switch (class) {
+case 0:
+   /* ascii */
+   start = 0x01;
+   stop = 0x7f;
+   break;
+case 1:
+   /* the rest of unicode */
+   start = 0x80;
+   stop = GLYPH_MAX;
+}
+
+if (start == stop)
+   return start;
+else
+   return start + (random() 

[PATCH 12/16] test: update dump-restore roundtripping test for batch-tag format.

2012-11-18 Thread david
From: David Bremner brem...@debian.org

Now we can actually round trip these crazy tags and and message ids.
hex-xcode is no longer needed as it's built in.
---
 test/dump-restore |   13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/test/dump-restore b/test/dump-restore
index a2204fb..e08b656 100755
--- a/test/dump-restore
+++ b/test/dump-restore
@@ -85,13 +85,14 @@ test_begin_subtest dump --output=outfile -- from:cworth
 notmuch dump --output=dump-outfile-dash-inbox.actual -- from:cworth
 test_expect_equal_file dump-cworth.expected dump-outfile-dash-inbox.actual
 
+
 test_expect_success 'roundtripping random message-ids and tags' \
-'test_subtest_known_broken 
- ${TEST_DIRECTORY}/random-corpus --num-messages=10 
--config-path=${NOTMUCH_CONFIG} 
- notmuch dump | ${TEST_DIRECTORY}/hex-xcode --direction=encode  
EXPECTED.$test_count 
+'${TEST_DIRECTORY}/random-corpus --num-messages=100 
--config-path=${NOTMUCH_CONFIG} 
+ notmuch dump --format=batch-tag | sort  EXPECTED.$test_count 
  notmuch tag -random-corpus tag:random-corpus 
- ${TEST_DIRECTORY}/hex-xcode --direction=decode  EXPECTED.$test_count | 
notmuch restore 2/dev/null 
- notmuch dump | ${TEST_DIRECTORY}/hex-xcode --direction=encode  
OUTPUT.$test_count 
+ notmuch restore --format=batch-tag --input=EXPECTED.$test_count 
+ notmuch dump --format=batch-tag | sort  OUTPUT.$test_count 
  test_cmp EXPECTED.$test_count OUTPUT.$test_count'
-
 test_done
+
+# Note the database is poisoned for sup format at this point.
-- 
1.7.10.4

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


[PATCH 08/16] tag-util.[ch]: New files for common tagging routines

2012-11-18 Thread david
From: David Bremner brem...@debian.org

These are meant to be shared between notmuch-tag and notmuch-restore.

The bulk of the routines implement a tag operation list abstract
data type act as a structured representation of a set of tag
operations (typically coming from a single tag command or line of
input).
---
 Makefile.local |1 +
 tag-util.c |  273 
 tag-util.h |   71 +++
 3 files changed, 345 insertions(+)
 create mode 100644 tag-util.c
 create mode 100644 tag-util.h

diff --git a/Makefile.local b/Makefile.local
index 2b91946..854867d 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -274,6 +274,7 @@ notmuch_client_srcs =   \
query-string.c  \
mime-node.c \
crypto.c\
+   tag-util.c
 
 notmuch_client_modules = $(notmuch_client_srcs:.c=.o)
 
diff --git a/tag-util.c b/tag-util.c
new file mode 100644
index 000..5329b1f
--- /dev/null
+++ b/tag-util.c
@@ -0,0 +1,273 @@
+#include tag-util.h
+#include hex-escape.h
+
+static char *
+strtok_len (char *s, const char *delim, size_t *len)
+{
+/* skip initial delims */
+s += strspn (s, delim);
+
+/* length of token */
+*len = strcspn (s, delim);
+
+return *len ? s : NULL;
+}
+
+/* Tag messages according to 'input', which must consist of lines of
+ * the format:
+ *
+ * +tag|-tag [...] [--] search-terms
+ *
+ * Each line is interpreted similarly to notmuch tag command line
+ * arguments. The delimiter is one or more spaces ' '. Any characters
+ * in tag and search-terms MAY be hex encoded with %NN where NN is
+ * the hexadecimal value of the character. Any ' ' and '%' characters
+ * in tag and search-terms MUST be hex encoded (using %20 and %25,
+ * respectively). Any characters that are not part of tag or
+ * search-terms MUST NOT be hex encoded.
+ *
+ * Leading and trailing space ' ' is ignored. Empty lines and lines
+ * beginning with '#' are ignored.
+ */
+int
+parse_tag_line (void *ctx, char *line,
+   char **query_string,
+   tag_op_list_t *tag_ops)
+{
+char *tok = line;
+size_t tok_len = 0;
+
+chomp_newline (line);
+tag_op_list_reset (tag_ops);
+
+/* Parse tags. */
+while ((tok = strtok_len (tok + tok_len,  , tok_len)) != NULL) {
+   notmuch_bool_t remove;
+   char *tag;
+
+   /* Optional explicit end of tags marker. */
+   if (strncmp (tok, --, tok_len) == 0) {
+   tok = strtok_len (tok + tok_len,  , tok_len);
+   break;
+   }
+
+   /* Implicit end of tags. */
+   if (*tok != '-'  *tok != '+')
+   break;
+
+   /* If tag is terminated by NUL, there's no query string. */
+   if (*(tok + tok_len) == '\0') {
+   tok = NULL;
+   break;
+   }
+
+   /* Terminate, and start next token after terminator. */
+   *(tok + tok_len++) = '\0';
+
+   remove = (*tok == '-');
+   tag = tok + 1;
+
+   /* Refuse empty tags. */
+   if (*tag == '\0') {
+   tok = NULL;
+   break;
+   }
+
+   /* Decode tag. */
+   if (hex_decode_inplace (tag) != HEX_SUCCESS) {
+   tok = NULL;
+   break;
+   }
+
+   if (tag_op_list_append (ctx, tag_ops, tag, remove))
+   return -1;
+}
+
+if (tok == NULL || tag_ops-count == 0) {
+   /* FIXME: line has been modified! */
+   fprintf (stderr, Warning: Ignoring invalid input line: %s\n,
+line);
+   return 1;
+}
+
+/* tok now points to the query string */
+if (hex_decode_inplace (tok) != HEX_SUCCESS) {
+   /* FIXME: line has been modified! */
+   fprintf (stderr, Warning: Ignoring invalid input line: %s\n,
+line);
+   return 1;
+}
+
+*query_string = tok;
+
+return 0;
+}
+
+static inline void
+message_error (notmuch_message_t *message,
+  notmuch_status_t status,
+  const char *format, ...)
+{
+va_list va_args;
+
+va_start (va_args, format);
+
+vfprintf (stderr, format, va_args);
+fprintf (stderr, Message-ID: %s\n, notmuch_message_get_message_id 
(message));
+fprintf (stderr, Status: %s\n, notmuch_status_to_string (status));
+}
+
+notmuch_status_t
+tag_op_list_apply (notmuch_message_t *message,
+  tag_op_list_t *list,
+  tag_op_flag_t flags)
+{
+int i;
+
+notmuch_status_t status = 0;
+tag_operation_t *tag_ops = list-ops;
+
+status = notmuch_message_freeze (message);
+if (status) {
+   message_error (message, status, freezing message);
+   return status;
+}
+
+if (flags  TAG_FLAG_REMOVE_ALL) {
+   status = notmuch_message_remove_all_tags (message);
+   if (status) {
+   message_error (message, status, removing all tags );
+   return status;
+   }
+}
+
+for (i = 0; i  list-count; i++) {
+   if (tag_ops[i].remove) {
+   status = 

[PATCH 11/16] notmuch-restore: add support for input format 'batch-tag'

2012-11-18 Thread david
From: David Bremner brem...@debian.org

This is the same as the batch input for notmuch tag, except by default
it removes all tags before modifying a given message id and only id:
is supported.
---
 notmuch-restore.c |  180 +++--
 1 file changed, 105 insertions(+), 75 deletions(-)

diff --git a/notmuch-restore.c b/notmuch-restore.c
index f03dcac..1776e99 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -19,18 +19,21 @@
  */
 
 #include notmuch-client.h
+#include dump-restore-private.h
+#include tag-util.h
+
+static volatile sig_atomic_t interrupted;
+static regex_t regex;
 
 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)
+tag_message (unused (void *ctx),
+notmuch_database_t *notmuch,
+const char *message_id,
+tag_op_list_t *tag_ops,
+tag_op_flag_t 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);
@@ -44,55 +47,46 @@ tag_message (notmuch_database_t *notmuch, const char 
*message_id,
 
 /* 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 ( (flags  TAG_FLAG_REMOVE_ALL) || ! (tag_op_list_empty (tag_ops)))
+   tag_op_list_apply (message, tag_ops, flags);
 
-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;
+if (message)
+   notmuch_message_destroy (message);
 
-notmuch_message_freeze (message);
+return ret;
+}
 
-if (remove_all)
-   notmuch_message_remove_all_tags (message);
+static int
+parse_sup_line (void *ctx, char *line,
+   char **query_str, tag_op_list_t *tag_ops)
+{
 
-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;
-   }
-}
+regmatch_t match[3];
+char *file_tags;
+int rerr;
 
-notmuch_message_thaw (message);
+tag_op_list_reset (tag_ops);
 
-if (synchronize_flags)
-   notmuch_message_tags_to_maildir_flags (message);
+chomp_newline (line);
 
-  DONE:
-if (message)
-   notmuch_message_destroy (message);
+/* Silently ignore blank lines */
+if (line[0] == '\0') {
+   return 1;
+}
 
-return ret;
+rerr = xregexec (regex, line, 3, match, 0);
+if (rerr == REG_NOMATCH) {
+   fprintf (stderr, Warning: Ignoring invalid input line: %s\n,
+line);
+   return 1;
+}
+
+*query_str = talloc_strndup (ctx, line + match[1].rm_so,
+match[1].rm_eo - match[1].rm_so);
+file_tags = talloc_strndup (ctx, line + match[2].rm_so,
+   match[2].rm_eo - match[2].rm_so);
+
+return tag_op_list_from_string (ctx, file_tags, FALSE, tag_ops);
 }
 
 int
@@ -100,16 +94,19 @@ notmuch_restore_command (unused (void *ctx), int argc, 
char *argv[])
 {
 notmuch_config_t *config;
 notmuch_database_t *notmuch;
-notmuch_bool_t synchronize_flags;
 notmuch_bool_t accumulate = FALSE;
+tag_op_flag_t flags = 0;
+tag_op_list_t *tag_ops;
+
 char *input_file_name = NULL;
 FILE *input = stdin;
 char *line = NULL;
 size_t line_size;
 ssize_t line_len;
-regex_t regex;
-int rerr;
+
+int ret = 0;
 int opt_index;
+int input_format = DUMP_FORMAT_AUTO;
 
 config = notmuch_config_open (ctx, NULL, NULL);
 if (config == NULL)
@@ -119,9 +116,15 @@ notmuch_restore_command (unused (void *ctx), int argc, 
char *argv[])
   NOTMUCH_DATABASE_MODE_READ_WRITE, notmuch))
return 1;
 
-synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
+if (notmuch_config_get_maildir_synchronize_flags (config))
+   flags |= TAG_FLAG_MAILDIR_SYNC;

[PATCH 13/16] test: second set of dump/restore --format=batch-tag tests

2012-11-18 Thread david
From: David Bremner brem...@debian.org

These one need the completed functionality in notmuch-restore. Fairly
exotic tags are tested, but no weird message id's.

We test each possible input to autodetection, both explicit (with
--format=auto) and implicit (without --format).
---
 test/dump-restore |   73 +
 1 file changed, 73 insertions(+)

diff --git a/test/dump-restore b/test/dump-restore
index e08b656..7b67d03 100755
--- a/test/dump-restore
+++ b/test/dump-restore
@@ -85,6 +85,79 @@ test_begin_subtest dump --output=outfile -- from:cworth
 notmuch dump --output=dump-outfile-dash-inbox.actual -- from:cworth
 test_expect_equal_file dump-cworth.expected dump-outfile-dash-inbox.actual
 
+test_begin_subtest Check for a safe set of message-ids
+notmuch search --output=messages from:cworth  EXPECTED.$test_count
+notmuch search --output=messages from:cworth |\
+   $TEST_DIRECTORY/hex-xcode --direction=encode  OUTPUT.$test_count
+test_expect_equal_file OUTPUT.$test_count EXPECTED.$test_count
+
+test_begin_subtest format=batch-tag, # round-trip
+notmuch dump --format=sup | sort  EXPECTED.$test_count
+notmuch dump --format=batch-tag | notmuch restore --format=batch-tag
+notmuch dump --format=sup | sort  OUTPUT.$test_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+
+tag1='comic_swear=$^%$^%\\//-+$^%$'
+enc1=$($TEST_DIRECTORY/hex-xcode --direction=encode $tag1)
+
+tag2=$(printf 'this\n tag\t has\n spaces')
+enc2=$($TEST_DIRECTORY/hex-xcode --direction=encode $tag2)
+
+enc3='%c3%91%c3%a5%c3%b0%c3%a3%c3%a5%c3%a9-%c3%8f%c3%8a'
+tag3=$($TEST_DIRECTORY/hex-xcode --direction=decode $enc3)
+
+notmuch dump --format=batch-tag  BACKUP
+
+notmuch tag +$tag1 +$tag2 +$tag3 -inbox -unread *
+
+test_begin_subtest 'format=batch-tag, round trip with strange tags'
+notmuch dump --format=batch-tag  EXPECTED.$test_count
+notmuch dump --format=batch-tag | notmuch restore --format=batch-tag
+notmuch dump --format=batch-tag  OUTPUT.$test_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+
+test_begin_subtest 'format=batch-tag, checking encoded output'
+cp /dev/null EXPECTED.$test_count
+notmuch dump --format=batch-tag -- from:cworth |\
+awk { print \+$enc1 +$enc2 +$enc3 -- \ \$5 }  EXPECTED.$test_count
+
+notmuch dump --format=batch-tag -- from:cworth   OUTPUT.$test_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+
+test_begin_subtest 'restoring sane tags'
+notmuch restore --format=batch-tag  BACKUP
+notmuch dump --format=batch-tag  OUTPUT.$test_count
+test_expect_equal_file BACKUP OUTPUT.$test_count
+
+test_begin_subtest 'format=batch-tag, restore=auto'
+notmuch dump --format=batch-tag  EXPECTED.$test_count
+notmuch tag -inbox -unread *
+notmuch restore --format=auto  EXPECTED.$test_count
+notmuch dump --format=batch-tag  OUTPUT.$test_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+
+test_begin_subtest 'format=sup, restore=auto'
+notmuch dump --format=sup  EXPECTED.$test_count
+notmuch tag -inbox -unread *
+notmuch restore --format=auto  EXPECTED.$test_count
+notmuch dump --format=sup  OUTPUT.$test_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+
+test_begin_subtest 'format=batch-tag, restore=default'
+notmuch dump --format=batch-tag  EXPECTED.$test_count
+notmuch tag -inbox -unread *
+notmuch restore  EXPECTED.$test_count
+notmuch dump --format=batch-tag  OUTPUT.$test_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+
+test_begin_subtest 'format=sup, restore=default'
+notmuch dump --format=sup  EXPECTED.$test_count
+notmuch tag -inbox -unread *
+notmuch restore  EXPECTED.$test_count
+notmuch dump --format=sup  OUTPUT.$test_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+
+notmuch dump --format=batch-tag --output=save.tags
 
 test_expect_success 'roundtripping random message-ids and tags' \
 '${TEST_DIRECTORY}/random-corpus --num-messages=100 
--config-path=${NOTMUCH_CONFIG} 
-- 
1.7.10.4

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


[PATCH 10/16] test: add test for notmuch tag --batch option

2012-11-18 Thread david
From: Jani Nikula j...@nikula.org

Basic test of functionality, along with all combinations of options.
---
 test/tagging |   35 +++
 1 file changed, 35 insertions(+)

diff --git a/test/tagging b/test/tagging
index 980ff92..574bc10 100755
--- a/test/tagging
+++ b/test/tagging
@@ -46,6 +46,41 @@ 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 tag4 unread)
 
+test_begin_subtest --batch
+notmuch tag --batch EOF
+# %20 is a space in tag
+-:%20 -tag1 +tag5 +tag6 -- One
++tag1 -tag1 -tag4 +tag4 -- Two
+-tag6 One
++tag5 Two
+EOF
+output=$(notmuch search \* | notmuch_search_sanitize)
+test_expect_equal $output \
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (inbox tag5 unread)
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag4 tag5 unread)
+
+ batch.in  EOF
+# %20 is a space in tag
+-:%20 -tag1 +tag5 +tag6 -- One
++tag1 -tag1 -tag4 +tag4 -- Two
+-tag6 One
++tag5 Two
+EOF
+
+test_begin_subtest --input
+notmuch tag --input=batch.in
+output=$(notmuch search \* | notmuch_search_sanitize)
+test_expect_equal $output \
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (inbox tag5 unread)
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag4 tag5 unread)
+
+test_begin_subtest --batch --input
+notmuch tag --batch --input=batch.in
+output=$(notmuch search \* | notmuch_search_sanitize)
+test_expect_equal $output \
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (inbox tag5 unread)
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag4 tag5 unread)
+
 test_expect_code 1 Empty tag names 'notmuch tag + One'
 
 test_expect_code 1 Tag name beginning with - 'notmuch tag +- One'
-- 
1.7.10.4

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


[PATCH 14/16] tag-util: optimization of tag application

2012-11-18 Thread david
From: David Bremner brem...@debian.org

The idea is not to bother with restore operations if they don't change
the set of tags. This is actually a relatively common case.

In order to avoid fancy datastructures, this method is quadratic in
the number of tags; at least on my mail database this doesn't seem to
be a big problem.
---
 notmuch-tag.c |2 +-
 tag-util.c|   59 +
 tag-util.h|3 ++-
 3 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/notmuch-tag.c b/notmuch-tag.c
index ca120d5..33cf78d 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -140,7 +140,7 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const 
char *query_string,
 notmuch_messages_valid (messages)  ! interrupted;
 notmuch_messages_move_to_next (messages)) {
message = notmuch_messages_get (messages);
-   tag_op_list_apply (message, tag_ops, flags);
+   tag_op_list_apply (message, tag_ops, flags | TAG_FLAG_PRE_OPTIMIZED);
notmuch_message_destroy (message);
 }
 
diff --git a/tag-util.c b/tag-util.c
index 5329b1f..de0928b 100644
--- a/tag-util.c
+++ b/tag-util.c
@@ -117,6 +117,62 @@ message_error (notmuch_message_t *message,
 fprintf (stderr, Status: %s\n, notmuch_status_to_string (status));
 }
 
+static int
+makes_changes (notmuch_message_t *message,
+  tag_op_list_t *list,
+  tag_op_flag_t flags)
+{
+
+int i;
+
+notmuch_tags_t *tags;
+notmuch_bool_t changes = FALSE;
+
+/* First, do we delete an existing tag? */
+changes = FALSE;
+for (tags = notmuch_message_get_tags (message);
+! changes  notmuch_tags_valid (tags);
+notmuch_tags_move_to_next (tags)) {
+   const char *cur_tag = notmuch_tags_get (tags);
+   int last_op =  (flags  TAG_FLAG_REMOVE_ALL) ? -1 : 0;
+
+   for (i = 0; i  list-count; i++) {
+   if (strcmp (cur_tag, list-ops[i].tag) == 0) {
+   last_op = list-ops[i].remove ? -1 : 1;
+   }
+   }
+
+   changes = (last_op == -1);
+}
+notmuch_tags_destroy (tags);
+
+if (changes)
+   return TRUE;
+
+/* Now check for adding new tags */
+for (i = 0; i  list-count; i++) {
+   notmuch_bool_t exists = FALSE;
+
+   for (tags = notmuch_message_get_tags (message);
+notmuch_tags_valid (tags);
+notmuch_tags_move_to_next (tags)) {
+   const char *cur_tag = notmuch_tags_get (tags);
+   if (strcmp (cur_tag, list-ops[i].tag) == 0) {
+   exists = TRUE;
+   break;
+   }
+   }
+   notmuch_tags_destroy (tags);
+
+   /* the following test is conservative, it's ok to think we
+* make changes when we don't */
+   if ( ! exists  ! list-ops[i].remove )
+   return TRUE;
+}
+return FALSE;
+
+}
+
 notmuch_status_t
 tag_op_list_apply (notmuch_message_t *message,
   tag_op_list_t *list,
@@ -127,6 +183,9 @@ tag_op_list_apply (notmuch_message_t *message,
 notmuch_status_t status = 0;
 tag_operation_t *tag_ops = list-ops;
 
+if (! (flags  TAG_FLAG_PRE_OPTIMIZED)  ! makes_changes (message, list, 
flags))
+   return NOTMUCH_STATUS_SUCCESS;
+
 status = notmuch_message_freeze (message);
 if (status) {
message_error (message, status, freezing message);
diff --git a/tag-util.h b/tag-util.h
index b381b8e..998c1b9 100644
--- a/tag-util.h
+++ b/tag-util.h
@@ -19,7 +19,8 @@ typedef struct {
 /* Use powers of 2 */
 typedef enum  { TAG_FLAG_NONE = 0,
TAG_FLAG_MAILDIR_SYNC = 1,
-   TAG_FLAG_REMOVE_ALL = 2 } tag_op_flag_t;
+   TAG_FLAG_REMOVE_ALL = 2,
+   TAG_FLAG_PRE_OPTIMIZED = 4 } tag_op_flag_t;
 
 
 typedef int (*tag_callback_t)(void *ctx,
-- 
1.7.10.4

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


[PATCH 15/16] man: document notmuch tag --batch, --input options

2012-11-18 Thread david
From: Jani Nikula j...@nikula.org

---
 man/man1/notmuch-tag.1 |   52 +++-
 1 file changed, 51 insertions(+), 1 deletion(-)

diff --git a/man/man1/notmuch-tag.1 b/man/man1/notmuch-tag.1
index 0f86582..751db7b 100644
--- a/man/man1/notmuch-tag.1
+++ b/man/man1/notmuch-tag.1
@@ -4,7 +4,12 @@ notmuch-tag \- add/remove tags for all messages matching the 
search terms
 
 .SH SYNOPSIS
 .B notmuch tag
-.RI  + tag |\- tag  [...] [\-\-]  search-term ...
+.RI + tag |\- tag  [...] [\-\-]  search-terms 
+
+.B notmuch tag
+.RI --batch
+.RI [ --input= filename  ]
+
 
 .SH DESCRIPTION
 
@@ -29,6 +34,51 @@ updates the maildir flags according to tag changes if the
 configuration option is enabled. See \fBnotmuch-config\fR(1) for
 details.
 
+Supported options for
+.B tag
+include
+.RS 4
+.TP 4
+.BR \-\-batch
+
+Read batch tagging operations from standard input. This is more
+efficient than repeated
+.B notmuch tag
+invocations. See
+.B TAG FILE FORMAT
+below for the input format. This option is not compatible with
+specifying tagging on the command line.
+.RE
+
+.RS 4
+.TP 4
+.BR \-\-input= filename
+
+Read input from given file, instead of from stdin. Implies
+.BR --batch .
+
+.SH TAG FILE FORMAT
+
+The input must consist of lines of the format:
+
+.RI T + tag |\- tag  [...] [\-\-]  search-terms 
+
+Each line is interpreted similarly to
+.B notmuch tag
+command line arguments. The delimiter is one or more spaces ' '. Any
+characters in tag and search-terms
+.B may
+be hex encoded with %NN where NN is the hexadecimal value of the
+character. Any ' ' and '%' characters in tag and search-terms
+.B must
+be hex encoded (using %20 and %25, respectively). Any characters that
+are not part of tag or search-terms
+.B must not
+be hex encoded.
+
+Leading and trailing space ' ' is ignored. Empty lines and lines
+beginning with '#' are ignored.
+
 .SH SEE ALSO
 
 \fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
-- 
1.7.10.4

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


[PATCH 16/16] notmuch-{dump,restore}.1: document new format options

2012-11-18 Thread david
From: David Bremner brem...@debian.org

More or less arbitrarily, notmuch-dump.1 gets the more detailed
description of the format.
---
 man/man1/notmuch-dump.1|   58 +++
 man/man1/notmuch-restore.1 |   59 +++-
 2 files changed, 111 insertions(+), 6 deletions(-)

diff --git a/man/man1/notmuch-dump.1 b/man/man1/notmuch-dump.1
index 230deec..b57c2c1 100644
--- a/man/man1/notmuch-dump.1
+++ b/man/man1/notmuch-dump.1
@@ -5,6 +5,7 @@ notmuch-dump \- creates a plain-text dump of the tags of each 
message
 .SH SYNOPSIS
 
 .B notmuch dump
+.RB  [ \-\-format=(sup|batch-tag)  ] [--]
 .RI [ --output= filename  ] [--]
 .RI [  search-term ...]
 
@@ -19,6 +20,63 @@ recreated from the messages themselves.  The output of 
notmuch dump is
 therefore the only critical thing to backup (and much more friendly to
 incremental backup than the native database files.)
 
+.TP 4
+.B \-\-format=(sup|batch-tag)
+
+Notmuch restore supports two plain text dump formats, both with one message-id
+per line, followed by a list of tags.
+
+.RS 4
+.TP 4
+.B sup
+
+The
+.B sup
+dump file format is specifically chosen to be
+compatible with the format of files produced by sup-dump.
+So if you've previously been using sup for mail, then the
+.B notmuch restore
+command provides you a way to import all of your tags (or labels as
+sup calls them).
+Each line has the following form
+
+.RS 4
+.RI  message-id 
+.B (
+.RI  tag  ...
+.B )
+
+with zero or more tags are separated by spaces. Note that (malformed)
+message-ids may contained arbitrary non-null characters. Note also
+that tags with spaces will not be correctly restored with this format.
+
+.RE
+
+.RE
+.RS 4
+.TP 4
+.B batch-tag
+
+The
+.B batch-tag
+dump format is intended to more robust against malformed message-ids
+and tags containing whitespace or non-\fBascii\fR(7) characters.
+Each line has the form
+
+.RS 4
+.RI + encoded-tag+ encoded-tag  ... -- 
encoded-message-id 
+
+where encoded means that every byte not matching the regex
+.B [A-Za-z0-9+-_@=.:,]
+is replace by
+.B %nn
+where nn is the two digit hex encoding.
+The astute reader will notice this is a special case of the batch input
+format for \fBnotmuch-tag\fR(1).
+
+.RE
+
+
 With no search terms, a dump of all messages in the database will be
 generated.  A -- argument instructs notmuch that the
 remaining arguments are search terms.
diff --git a/man/man1/notmuch-restore.1 b/man/man1/notmuch-restore.1
index 2fa8733..3860829 100644
--- a/man/man1/notmuch-restore.1
+++ b/man/man1/notmuch-restore.1
@@ -6,6 +6,7 @@ notmuch-restore \- restores the tags from the given file (see 
notmuch dump)
 
 .B notmuch restore
 .RB [ --accumulate ]
+.RB [ --format=(auto|batch-tag|sup) ]
 .RI [ --input= filename  ]
 
 .SH DESCRIPTION
@@ -15,19 +16,51 @@ Restores the tags from the given file (see
 
 The input is read from the given filename, if any, or from stdin.
 
-Note: The dump file format is specifically chosen to be
+
+Supported options for
+.B restore
+include
+.RS 4
+.TP 4
+.B \-\-accumulate
+
+The union of the existing and new tags is applied, instead of
+replacing each message's tags as they are read in from the dump file.
+
+.RE
+.RS 4
+.TP 4
+.B \-\-format=(sup|batch-tag|auto)
+
+Notmuch restore supports two plain text dump formats, with one message-id
+per line, and a list of tags.
+For details of the actual formats, see \fBnotmuch-dump\fR(1).
+
+.RS 4
+.TP 4
+.B sup
+
+The
+.B sup
+dump file format is specifically chosen to be
 compatible with the format of files produced by sup-dump.
 So if you've previously been using sup for mail, then the
 .B notmuch restore
 command provides you a way to import all of your tags (or labels as
 sup calls them).
 
-The --accumulate switch causes the union of the existing and new tags to be
-applied, instead of replacing each message's tags as they are read in from the
-dump file.
+.RE
+.RS 4
+.TP 4
+.B batch-tag
 
-See \fBnotmuch-search-terms\fR(7)
-for details of the supported syntax for search-terms.
+The
+.B batch-tag
+dump format is intended to more robust against malformed message-ids
+and tags containing whitespace or non-\fBascii\fR(7) characters.  This
+format hex-escapes all characters those outside of a small character
+set, intended to be suitable for e.g. pathnames in most UNIX-like
+systems.
 
 .B notmuch restore
 updates the maildir flags according to tag changes if the
@@ -36,6 +69,20 @@ configuration option is enabled. See \fBnotmuch-config\fR(1) 
for
 details.
 
 .RE
+
+.RS 4
+.TP 4
+.B auto
+
+This option (the default) tries to guess the format from the
+input. For correctly formed input in either supported format, this
+heuristic, based the fact that batch-tag format contains no parentheses,
+should be accurate.
+
+.RE
+
+.RE
+
 .SH SEE ALSO
 
 \fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
-- 
1.7.10.4

___
notmuch mailing 

[PATCH v2] emacs: display tags in notmuch-show with links

2012-11-18 Thread Damien Cassou
This patch obsoletes
id:1352565719-12397-1-git-send-email-damien.cas...@gmail.com

[PATCH 1/4] emacs: Add a thread's tags to emacs header-line
[PATCH 2/4] emacs: Make tags in header-line clickable
[PATCH 3/4] emacs: Make tags that appear in `notmuch-show' clickable
[PATCH 4/4] emacs: Add unit-tests for clickable tags in notmuch-show

This patch makes clickable all tags that appear in notmuch-show
buffers. Each tag is a link to open a new notmuch-search buffer for
this tag. Additionally, the buffer's header-line now shows the
thread's tags (clickable only if the `header-button' library is loaded
or loadable).

This patch is the first one of an upcoming series whose goal is to
integrate notmuch-labeler into notmuch. See the following for more
details:
https://github.com/DamienCassou/notmuch-labeler

With respect to v2, I took care of the comments you made:
- The header-button library is not attached to the patch anymore. The
  tags in the header-line are always displayed but made clickable only
  if the library is loaded or loadable.
- The large patch is separated into a series of 4 smaller patches.

You can follow this patch series on
https://github.com/DamienCassou/notmuch/tree/labeler-integration-slow
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 2/4] Make tags in header-line clickable

2012-11-18 Thread Damien Cassou
Signed-off-by: Damien Cassou damien.cas...@gmail.com
---
 emacs/notmuch-tagger.el |   54 ++-
 1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/emacs/notmuch-tagger.el b/emacs/notmuch-tagger.el
index 5ca190e..19a6c7e 100644
--- a/emacs/notmuch-tagger.el
+++ b/emacs/notmuch-tagger.el
@@ -32,11 +32,63 @@
   (setq first nil)
   (push elt res
 
+(defun notmuch-tagger-header-button-present-p ()
+  Check if `header-button' can be loaded or is already loaded.
+
+`header-button' is a third-party library which facilitates the
+creation of links in emacs header-line. This function tries to
+`require' `header-button' and returns nil if and only if this
+fails.
+  (require 'header-button nil t))
+
+(defun notmuch-tagger-goto-target (tag)
+  Show a `notmuch-search' buffer for the TAG.
+  (notmuch-search (concat tag: tag)))
+
+(defun notmuch-tagger-header-button-action (button)
+  Open `notmuch-search' for the tag referenced by BUTTON.
+This function depends on the presence of the `header-button'
+library. Please call `notmuch-tagger-header-button-present-p' to
+test if the library is present before calling this function.
+  (let ((tag (header-button-get button 'notmuch-tagger-tag)))
+(notmuch-tagger-goto-target tag)))
+
+(eval-after-load header-button
+  '(define-button-type 'notmuch-tagger-header-button-type
+ 'supertype 'header
+ 'action#'notmuch-tagger-header-button-action
+ 'follow-link t))
+
+(defun notmuch-tagger-really-make-header-link (tag)
+   Return a property list that presents a link to TAG.
+
+The returned property list will only work in the header-line.
+Additionally, this function depends on the presence of the
+`header-button' library. Please call
+`notmuch-tagger-header-button-present-p' to test if library is
+present before calling this function.
+   (header-button-format
+tag
+:type 'notmuch-tagger-header-button-type
+'notmuch-tagger-tag tag
+'help-echo (format %s: Search other messages like this tag)))
+
+(defun notmuch-tagger-make-header-link (tag)
+  Return a property list to present TAG as a link to search.
+
+This only works if `header-button' is loaded. Simply returns tag
+if not.
+  (if (notmuch-tagger-header-button-present-p)
+  (notmuch-tagger-really-make-header-link tag)
+tag))
+
 (defun notmuch-tagger-present-tags-header-line (tags)
   Return a property list to present TAGS in emacs header-line.
   (list
(
-   (notmuch-tagger-separate-elems tags  )
+   (notmuch-tagger-separate-elems
+(mapcar #'notmuch-tagger-make-header-link tags)
+ )
)))
 
 
-- 
1.7.10.4

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


[PATCH 1/4] Add a thread's tags to emacs header-line

2012-11-18 Thread Damien Cassou
Signed-off-by: Damien Cassou damien.cas...@gmail.com
---
 emacs/notmuch-show.el   |   24 +---
 emacs/notmuch-tagger.el |   44 
 2 files changed, 65 insertions(+), 3 deletions(-)
 create mode 100644 emacs/notmuch-tagger.el

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 5b3e70e..988e27c 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -36,6 +36,7 @@
 (require 'notmuch-mua)
 (require 'notmuch-crypto)
 (require 'notmuch-print)
+(require 'notmuch-tagger)
 
 (declare-function notmuch-call-notmuch-process notmuch (rest args))
 (declare-function notmuch-fontify-headers notmuch nil)
@@ -1121,11 +1122,28 @@ function is used.
 
   (jit-lock-register #'notmuch-show-buttonise-links)
 
-  ;; Set the header line to the subject of the first message.
-  (setq header-line-format (notmuch-show-strip-re 
(notmuch-show-get-subject)))
-
+  (notmuch-show-update-header-line)
   (run-hooks 'notmuch-show-hook
 
+(defun notmuch-show-thread-tags ()
+  Return the list of tags for the current thread.
+  (let ((tags (list)))
+(notmuch-show-mapc (lambda ()
+(mapcar (lambda (elt)
+  ;; Avoid adding duplicate tags
+  (add-to-list 'tags elt))
+(notmuch-show-get-tags
+tags))
+
+(defun notmuch-show-update-header-line ()
+  Make the header-line show the thread's subject and tags.
+  (let ((thread-subject (notmuch-show-strip-re (notmuch-show-get-subject
+(setq header-line-format
+ (list
+  thread-subject
+   
+  (notmuch-tagger-present-tags-header-line 
(notmuch-show-thread-tags))
+
 (defun notmuch-show-capture-state ()
   Capture the state of the current buffer.
 
diff --git a/emacs/notmuch-tagger.el b/emacs/notmuch-tagger.el
new file mode 100644
index 000..5ca190e
--- /dev/null
+++ b/emacs/notmuch-tagger.el
@@ -0,0 +1,44 @@
+;; notmuch-tagger.el --- Library to improve the way tags are displayed
+;;
+;; Copyright © Damien Cassou
+;;
+;; This file is part of Notmuch.
+;;
+;; Notmuch is free software: you can redistribute it and/or modify it
+;; under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+;;
+;; Notmuch is distributed in the hope that it will be useful, but
+;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+;; General Public License for more details.
+;;
+;; You should have received a copy of the GNU General Public License
+;; along with Notmuch.  If not, see http://www.gnu.org/licenses/.
+;;
+;; Authors: Damien Cassou damien.cas...@gmail.com
+;;; Commentary:
+;;
+;;; Code:
+;;
+(defun notmuch-tagger-separate-elems (list sep)
+  Return a list with all elements of LIST separated by SEP.
+  (let ((first t)
+(res nil))
+(dolist (elt (reverse list) res)
+  (unless first
+(push sep res))
+  (setq first nil)
+  (push elt res
+
+(defun notmuch-tagger-present-tags-header-line (tags)
+  Return a property list to present TAGS in emacs header-line.
+  (list
+   (
+   (notmuch-tagger-separate-elems tags  )
+   )))
+
+
+(provide 'notmuch-tagger)
+;;; notmuch-tagger.el ends here
-- 
1.7.10.4

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


[PATCH 4/4] emacs: Add unit-tests for clickable tags in notmuch-show

2012-11-18 Thread Damien Cassou
Signed-off-by: Damien Cassou damien.cas...@gmail.com
---
 test/emacs |   62 
 1 file changed, 62 insertions(+)

diff --git a/test/emacs b/test/emacs
index 77265b0..2335bc4 100755
--- a/test/emacs
+++ b/test/emacs
@@ -813,5 +813,67 @@ test_emacs (let ((mm-text-html-renderer
 test_expect_success Rendering HTML mail with images \
 'grep -q smiley OUTPUT'
 
+test_begin_subtest Extracting all tags from a thread
+add_message \
+'[subject]=Extracting all tags from a thread' \
+'[body]=body 1'
+parent=${gen_msg_id}
+add_message \
+'[subject]=Extracting all tags from a thread' \
+'[body]=body 2' \
+[in-reply-to]=\$parent\
+add_message \
+'[subject]=Extracting all tags from a thread' \
+'[body]=body 3' \
+[in-reply-to]=\$parent\
+latest=${gen_msg_id}
+# Extract the thread-id from one of the emails
+thread_id=$(notmuch search --output=threads id:${latest})
+echo THREAD ID: ''$thread_id''
+# Add tag mytagfoo to one of the emails
+notmuch tag +mytagfoo id:${latest}
+test_emacs_expect_t \
+(notmuch-show \${thread_id}\)
+ (let ((output (notmuch-show-thread-tags))
+   (expected '(\inbox\ \mytagfoo\ \unread\)))
+  (notmuch-test-expect-equal
+ (sort output #'string)
+ (sort expected #'string)))
+
+test_begin_subtest The tags appear in the header-line of notmuch-show
+add_message \
+'[subject]=foo bar' \
+'[body]=body 1'
+parent=${gen_msg_id}
+# Add tag mytagfoo to one of the emails
+notmuch tag +mytagfoo id:${parent}
+# Extract the thread-id from one of the emails
+thread_id=$(notmuch search --output=threads id:${latest})
+test_emacs_expect_t \
+(notmuch-show \${thread_id}\)
+ (if (string-match-p \mytagfoo\ (format-mode-line header-line-format))
+ t
+   \The tag mytagfoo was not in the header-line-format\)
+
+test_begin_subtest The tags of notmuch-show emails are clickable
+add_message \
+'[subject]=foo bar' \
+'[body]=body 1'
+parent=${gen_msg_id}
+# Add tag mytagfoo to one of the emails
+notmuch tag +mytagfoo id:${parent}
+# Extract the thread-id from one of the emails
+thread_id=$(notmuch search --output=threads id:${latest})
+test_emacs_expect_t \
+(notmuch-show \${thread_id}\)
+(goto-char (point-min))
+(re-search-forward \mytagfoo\)
+(backward-char) ;; to be 'in' the tag
+(unless (eq major-mode 'notmuch-show-mode)
+   (error \We must be in notmuch-show at this point but we are in %s.\ 
major-mode))
+(push-button) ;; simulate a press on the RET key
+(if (eq major-mode 'notmuch-search-mode)
+t
+   (format \We must be in notmuch-search at this point but we are in 
%s.\ major-mode))
 
 test_done
-- 
1.7.10.4

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


[PATCH 2/2] emacs: less guessing of character set in messages

2012-11-18 Thread Tomi Ollila
The macro with-current-notmuch-show-message executes command
`notmuch show --format=raw id:...` which just outputs the contents
of the mail file verbatim (into temporary buffer). In case e.g. utf-8
locale is used the temporary buffer has buffer-file-coding-system as
utf-8. In this case Emacs converts the data to multibyte format, guessing
that input is in utf-8.
However, the raw (MIME) message may contain octet data in any other
8bit format, and as no (MIME-)content spesific handling to the message
is done at this point, conversion to other formats may lose information.
By setting coding-system-for-read 'no-conversion drops the conversion part
and makes this handle input as notmuch-get-bodypart-internal() does.
This marks the broken test in previous change fixed.
---
 emacs/notmuch-show.el | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 5b3e70e..44b6f35 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -203,9 +203,10 @@ For example, if you wanted to remove an \unread\ tag and 
add a
  (let ((id (notmuch-show-get-message-id)))
(let ((buf (generate-new-buffer (concat *notmuch-msg- id *
  (with-current-buffer buf
-   (call-process notmuch-command nil t nil show --format=raw id)
-   ,@body)
-(kill-buffer buf)
+  (let ((coding-system-for-read 'no-conversion))
+(call-process notmuch-command nil t nil show --format=raw id)
+,@body)
+  (kill-buffer buf))
 
 (defun notmuch-show-turn-on-visual-line-mode ()
   Enable Visual Line mode.
-- 
1.8.0

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


[PATCH 1/2] test/emacs: test saving of attachment containing 8bit octets

2012-11-18 Thread Tomi Ollila
This test catches the case 8bit octets in an attachment gets converted
or lost when saving attachment to the file.
This test is marked known broken.
---
 test/emacs | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/test/emacs b/test/emacs
index 77265b0..c8db7d0 100755
--- a/test/emacs
+++ b/test/emacs
@@ -516,6 +516,33 @@ test_emacs '(let ((standard-input \attachment2.gz\))
  (notmuch-show-save-part 
id:cf0c4d610911171136h1713aa59w9cf9aa31f052a...@mail.gmail.com 5))'
 test_expect_equal_file attachment2.gz $EXPECTED/attachment
 
+test_begin_subtest Save 8bit attachment from within emacs using 
notmuch-show-save-attachments
+test_subtest_known_broken
+
+add_message '[subject]=Attachment with 8bit chars' \
+   '[header]=MIME-Version: 1.0' \
+   '[content-type]=multipart/mixed; boundary=\abcd\' \
+   '[body]=--abcd
+Content-Type: text/plain
+
+Attachment follows:
+
+--abcd
+Content-Type: application/octet-stream; name=\sample\
+Content-Transfer-Encoding: 8bit
+Content-Disposition: attachment; filename=\sample\
+
+“¡ Hey ! It compiles ¡ Ship it !”
+
+--abcd--
+'
+test_emacs '(notmuch-show id:'${gen_msg_id}')
+   (delete-file OUTPUT)
+   (let ((standard-input \OUTPUT\))
+ (notmuch-show-save-attachments))'
+
+test_expect_equal $(cat OUTPUT) '“¡ Hey ! It compiles ¡ Ship it !”'
+
 test_begin_subtest View raw message within emacs
 test_emacs '(notmuch-show 
id:cf0c4d610911171136h1713aa59w9cf9aa31f052a...@mail.gmail.com)
(notmuch-show-view-raw-message)
-- 
1.8.0

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


Re: Add new dump/restore format and batch tagging.

2012-11-18 Thread Ethan Glasser-Camp
da...@tethera.net writes:

 which was revied by Tomi and Ethan. I think I implemented their
 suggestions.

Actually, I don't think you implemented all of mine.

- Patch 4 still has a subject line that ends in a period. I don't think
  this is mandatory for everyone but some people consider it best
  practice. You still have the spelling seperate (also, patch 7 has
  seperated).

- In patch 4, I still think this would look better if you switched the
  branches.

+if (private_status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) {
+   _notmuch_message_add_term (message, type, mail);
+} else {
+   return NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID;
+}

- In patch 5, I still think this looks funny:

+   int num_tags = random () % (max_tags + 1);
+   int this_mid_len = random () % message_id_len + 1;

Additionally, I would like a check that message_id_len is reasonable
(more than 1, say).

- Patch 8:

+int
+parse_tag_stream (void *ctx,
+ notmuch_database_t *notmuch,
+ FILE *input,
+ tag_callback_t callback,
+ tag_op_flag_t flags,
+ volatile sig_atomic_t *interrupted);

Am I going crazy, or does this function not get implemented?

- Patch 11: DUMP_FORMAT can change from DUMP_FORMAT_BATCH_TAG to
  DUMP_FORMAT_SUP if a paren is anywhere on the first line. I'd prefer
  this only happens if we have DUMP_FORMAT_AUTO.

You probably want to move the comment Dump output is... closer to the
regex.

I don't see why it's necessary to have this:

+   query_string = query_string + 3;

- Patch 13:

+cp /dev/null EXPECTED.$test_count
+notmuch dump --format=batch-tag -- from:cworth |\
+awk { print \+$enc1 +$enc2 +$enc3 -- \ \$5 }  EXPECTED.$test_count

What's the purpose of the CP here? It just creates an empty file. You
could do it with touch. Why even bother since you're going to fill it
with stuff in a second? Actually, care to explain the dump and sed call?
It looks like you're using this dump to encode the message IDs. If
format=batch-tag skips a message for some reason or terminates early,
the test won't fail.

- Patch 16:

+message-ids may contained arbitrary non-null characters. Note also

I think this should be may contain, or something else entirely if
you're trying to describe past behavior of sup?

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


Re: Add new dump/restore format and batch tagging.

2012-11-18 Thread Ethan Glasser-Camp
Ethan Glasser-Camp ethan.glasser.c...@gmail.com writes:

 - Patch 4 still has a subject line that ends in a period. I don't think
   this is mandatory for everyone but some people consider it best
   practice.

Best practice, of course, would be to remove the period at the end of
the subject line. Patch 12 also has one.

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


Re: [PATCH v2] emacs: display tags in notmuch-show with links

2012-11-18 Thread Ethan Glasser-Camp
Damien Cassou damien.cas...@gmail.com writes:

 This patch obsoletes
 id:1352565719-12397-1-git-send-email-damien.cas...@gmail.com

 [PATCH 1/4] emacs: Add a thread's tags to emacs header-line
 [PATCH 2/4] emacs: Make tags in header-line clickable
 [PATCH 3/4] emacs: Make tags that appear in `notmuch-show' clickable
 [PATCH 4/4] emacs: Add unit-tests for clickable tags in notmuch-show

 This patch makes clickable all tags that appear in notmuch-show
 buffers. Each tag is a link to open a new notmuch-search buffer for
 this tag. Additionally, the buffer's header-line now shows the
 thread's tags (clickable only if the `header-button' library is loaded
 or loadable).

Looks fine to me. Let me just get the notes from my bikeshed, in case
you get asked to roll another version :)

- You might want to use #' on lambdas.

- It bothers me how similar notmuch-tagger-{body,header}-button-action
  are. I thought it might be better to unify them by seeing what type
  the button argument was. Here's my (untested) approach which you might
  find prettier or uglier.

(notmuch-tagger-all-button-get (button attrib)
  Utility function to do button-get on different kinds of buttons.
  (cond
((integer-or-marker-p button)
  (button-get button attrib))
((and (featurep 'header-button)
  (listp button))
  (header-button-get button attrib))
(t (error unknown type of button %s button))

- The comment for notmuch-tagger-make-body-link reads that it will work
  everywhere except in the header-line. Does this mean mode-line, menu
  bar, or what? How about just won't work in the header-line?

- In patch 3:

 +If tags the result of this function is to be used within the

I think this should just read If the result.

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


Re: Notmuch scripts (again), now with more usenet

2012-11-18 Thread Ethan Glasser-Camp
Jan Pobrislo c...@webprojekty.cz writes:

 Hi! I was having some hardware issues and had to migrate the site. It should
 be all up again and ready for inclusion.

 Added stuff from last time:
 * source function  actions for zaw (https://github.com/zsh-users/zaw)
 * LICENSE (CC0)

Hi! Sorry for the delay, real life got in the way.

In previous emails, David Bremner said he'd like someone with Python
expertise to look at notmuch-new.py. He notes that pylint goes
ballistic, but most of these errors don't bother me much (complaints
about variable naming and missing docstrings).

As for the rest, I'm not a zsh expert, but I looked at zmuch and
notmuch-colorize and they looked fine.

If Bremner is willing to put this package in contrib, I think he should
do so.

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


Re: [PATCH v2] emacs: display tags in notmuch-show with links

2012-11-18 Thread Aaron Ecay
2012ko azaroak 18an, Ethan Glasser-Camp-ek idatzi zuen:
 
 - You might want to use #' on lambdas.

This is actually unnecessary – as the info node (elisp) Anonymous
Functions says, the forms with and without #' are equivalent.  The
current notmuch style is not to have #' on lambdas (that is, there are 0
instances of #'(lambda ...) in the code base).  IMO that’s correct:
the unnecessary #' is just line-noise-ish.

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


emacs's function special form

2012-11-18 Thread Ethan
On Sun, Nov 18, 2012 at 7:10 PM, Aaron Ecay aarone...@gmail.com wrote:

 2012ko azaroak 18an, Ethan Glasser-Camp-ek idatzi zuen:
 
  - You might want to use #' on lambdas.

 This is actually unnecessary – as the info node (elisp) Anonymous
 Functions says, the forms with and without #' are equivalent.  The
 current notmuch style is not to have #' on lambdas (that is, there are 0
 instances of #'(lambda ...) in the code base).  IMO that’s correct:
 the unnecessary #' is just line-noise-ish.


OK, I think I understand. Thanks for the clarification. I found that info
node very confusing.

It says that #' has effects assuming function-object is a valid lambda
expression. So why put #' on variables that are the names of functions?

In fact, outside names of functions, when are you supposed to use #' at all?

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


Re: emacs's function special form

2012-11-18 Thread Austin Clements
On Sun, 18 Nov 2012, Ethan ethan.glasser.c...@gmail.com wrote:
 On Sun, Nov 18, 2012 at 7:10 PM, Aaron Ecay aarone...@gmail.com wrote:

 2012ko azaroak 18an, Ethan Glasser-Camp-ek idatzi zuen:
 
  - You might want to use #' on lambdas.

 This is actually unnecessary – as the info node (elisp) Anonymous
 Functions says, the forms with and without #' are equivalent.  The
 current notmuch style is not to have #' on lambdas (that is, there are 0
 instances of #'(lambda ...) in the code base).  IMO that’s correct:
 the unnecessary #' is just line-noise-ish.


 OK, I think I understand. Thanks for the clarification. I found that info
 node very confusing.

 It says that #' has effects assuming function-object is a valid lambda
 expression. So why put #' on variables that are the names of functions?

 In fact, outside names of functions, when are you supposed to use #' at all?

You don't need to use #' on a lambda form because lambda is actually a
macro (implemented in Elisp!) that expands to
  #'(lambda ...)
so using #' would be redundant.  But you should use it on function
symbols to hint the byte code compiler that you're referring to a
function rather than a variable.  Using #' on symbols affects the
optimizer (and may improve warnings?), even though the info node doesn't
explicitly say how.  Plus, #' makes code more self-documenting.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch