[PATCH 6/7] emacs: Allow custom tags formatting

2012-07-12 Thread Austin Clements
Previously we ignored any notmuch-search-result-format customizations
for tag formatting because we needed to be able to parse back in the
result line and update the tags in place.  We no longer do either of
these things, so we can allow customization of this format.

(Coincidentally, previously we still allowed too much customization of
the tags format, since moving it earlier on the line or removing it
from the line would interfere with the tagging mechanism.  There is
now no problem with doing such things.)
---
 emacs/notmuch.el |8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index a5cf0dc..f32cfb0 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -755,11 +755,9 @@ non-authors is found, assume that all of the authors 
match."
 (notmuch-search-insert-authors format-string (plist-get result :authors)))

((string-equal field "tags")
-;; Ignore format-string here because notmuch-search-set-tags
-;; depends on the format of this
-(insert (concat "(" (propertize
-(mapconcat 'identity (plist-get result :tags) " ")
-'font-lock-face 'notmuch-tag-face) ")")
+(let ((tags-str (mapconcat 'identity (plist-get result :tags) " ")))
+  (insert (propertize (format format-string tags-str)
+ 'face 'notmuch-tag-face))

 (defun notmuch-search-show-result (result  pos)
   ;; Ignore excluded matches
-- 
1.7.10



[PATCH 5/7] emacs: Replace other search text properties with result property

2012-07-12 Thread Austin Clements
Since the result object contains everything that the other text
properties recorded, we can remove the other text properties and
simply look in the plist of the appropriate result object.
---
 emacs/notmuch.el |   24 ++--
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 3f72427..a5cf0dc 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -452,16 +452,18 @@ current result."
 (defun notmuch-search-properties-in-region (property beg end)
   (let (output)
 (notmuch-search-do-results beg end pos
-  (push (get-text-property pos property) output))
+  (push (plist-get (notmuch-search-get-result pos) property) output))
 output))

 (defun notmuch-search-find-thread-id ()
   "Return the thread for the current thread"
-  (get-text-property (point) 'notmuch-search-thread-id))
+  (let ((thread (plist-get (notmuch-search-get-result) :thread)))
+(when thread (concat "thread:" thread

 (defun notmuch-search-find-thread-id-region (beg end)
   "Return a list of threads for the current region"
-  (notmuch-search-properties-in-region 'notmuch-search-thread-id beg end))
+  (mapcar (lambda (thread) (concat "thread:" thread))
+ (notmuch-search-properties-in-region :thread beg end)))

 (defun notmuch-search-find-thread-id-region-search (beg end)
   "Return a search string for threads for the current region"
@@ -469,19 +471,19 @@ current result."

 (defun notmuch-search-find-authors ()
   "Return the authors for the current thread"
-  (get-text-property (point) 'notmuch-search-authors))
+  (plist-get (notmuch-search-get-result) :authors))

 (defun notmuch-search-find-authors-region (beg end)
   "Return a list of authors for the current region"
-  (notmuch-search-properties-in-region 'notmuch-search-authors beg end))
+  (notmuch-search-properties-in-region :authors beg end))

 (defun notmuch-search-find-subject ()
   "Return the subject for the current thread"
-  (get-text-property (point) 'notmuch-search-subject))
+  (plist-get (notmuch-search-get-result) :subject))

 (defun notmuch-search-find-subject-region (beg end)
   "Return a list of authors for the current region"
-  (notmuch-search-properties-in-region 'notmuch-search-subject beg end))
+  (notmuch-search-properties-in-region :subject beg end))

 (defun notmuch-search-show-thread ()
   "Display the currently selected thread."
@@ -769,13 +771,7 @@ non-authors is found, assume that all of the authors 
match."
  (notmuch-search-insert-field (car spec) (cdr spec) result))
(insert "\n")
(notmuch-search-color-line beg (point) (plist-get result :tags))
-   (put-text-property beg (point) 'notmuch-search-result result)
-   (put-text-property beg (point) 'notmuch-search-thread-id
-  (concat "thread:" (plist-get result :thread)))
-   (put-text-property beg (point) 'notmuch-search-authors
-  (plist-get result :authors))
-   (put-text-property beg (point) 'notmuch-search-subject
-  (plist-get result :subject)))
+   (put-text-property beg (point) 'notmuch-search-result result))
   (when (string= (plist-get result :thread) notmuch-search-target-thread)
(setq notmuch-search-target-thread "found")
(goto-char beg)
-- 
1.7.10



[PATCH 4/7] emacs: Use result text properties for search result iteration

2012-07-12 Thread Austin Clements
This simplifies the traversal of regions of results and eliminates the
need for save-excursions (which tend to get in the way of maintaining
point when we make changes to the buffer).  It also fixes some strange
corner cases in the old line-based code where results that bordered
the region but were not included in it could be affected by region
commands.  Coincidentally, this also essentially enables multi-line
search result formats; the only remaining non-multi-line-capable
functions are notmuch-search-{next,previous}-thread, which are only
used interactively.
---
 emacs/notmuch.el |   61 ++
 1 file changed, 34 insertions(+), 27 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 2f83425..3f72427 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -427,17 +427,33 @@ returns nil"
 (next-single-property-change (or pos (point)) 'notmuch-search-result
 nil (point-max

+(defmacro notmuch-search-do-results (beg end pos-sym  body)
+  "Invoke BODY for each result between BEG and END.
+
+POS-SYM will be bound to the point at the beginning of the
+current result."
+  (declare (indent 3))
+  (let ((end-sym (make-symbol "end"))
+   (first-sym (make-symbol "first")))
+`(let ((,pos-sym (notmuch-search-result-beginning ,beg))
+  ;; End must be a marker in case body changes the text
+  (,end-sym (copy-marker ,end))
+  ;; Make sure we examine one result, even if (= beg end)
+  (,first-sym t))
+   ;; We have to be careful if the region extends beyond the
+   ;; results.  In this case, pos could be null or there could be
+   ;; no result at pos.
+   (while (and ,pos-sym (or (< ,pos-sym ,end-sym) ,first-sym))
+(when (notmuch-search-get-result ,pos-sym)
+  , at body)
+(setq ,pos-sym (notmuch-search-result-end ,pos-sym)
+  ,first-sym nil)
+
 (defun notmuch-search-properties-in-region (property beg end)
-  (save-excursion
-(let ((output nil)
- (last-line (line-number-at-pos end))
- (max-line (- (line-number-at-pos (point-max)) 2)))
-  (goto-char beg)
-  (beginning-of-line)
-  (while (<= (line-number-at-pos) (min last-line max-line))
-   (setq output (cons (get-text-property (point) property) output))
-   (forward-line 1))
-  output)))
+  (let (output)
+(notmuch-search-do-results beg end pos
+  (push (get-text-property pos property) output))
+output))

 (defun notmuch-search-find-thread-id ()
   "Return the thread for the current thread"
@@ -517,28 +533,19 @@ and will also appear in a buffer named \"*Notmuch 
errors*\"."
   (plist-get (notmuch-search-get-result pos) :tags))

 (defun notmuch-search-get-tags-region (beg end)
-  (save-excursion
-(let ((output nil)
- (last-line (line-number-at-pos end))
- (max-line (- (line-number-at-pos (point-max)) 2)))
-  (goto-char beg)
-  (while (<= (line-number-at-pos) (min last-line max-line))
-   (setq output (append output (notmuch-search-get-tags)))
-   (forward-line 1))
-  output)))
+  (let (output)
+(notmuch-search-do-results beg end pos
+  (setq output (append output (notmuch-search-get-tags pos
+output))

 (defun notmuch-search-tag-region (beg end  tag-changes)
   "Change tags for threads in the given region."
   (let ((search-string (notmuch-search-find-thread-id-region-search beg end)))
 (setq tag-changes (funcall 'notmuch-tag search-string tag-changes))
-(save-excursion
-  (let ((last-line (line-number-at-pos end))
-   (max-line (- (line-number-at-pos (point-max)) 2)))
-   (goto-char beg)
-   (while (<= (line-number-at-pos) (min last-line max-line))
- (notmuch-search-set-tags
-  (notmuch-update-tags (notmuch-search-get-tags) tag-changes))
- (forward-line))
+(notmuch-search-do-results beg end pos
+  (notmuch-search-set-tags
+   (notmuch-update-tags (notmuch-search-get-tags pos) tag-changes)
+   pos

 (defun notmuch-search-tag ( tag-changes)
   "Change tags for the currently selected thread or region.
-- 
1.7.10



[PATCH 3/7] emacs: Update tags by rewriting the search result line in place

2012-07-12 Thread Austin Clements
Now that we keep the full thread result object, we can refresh a
result after any changes by simply deleting and reconstructing the
result line from scratch.

A convenient side-effect of this wholesale replacement is that search
now re-applies notmuch-search-line-faces when tags change.
---
 emacs/notmuch.el |   55 ++
 1 file changed, 31 insertions(+), 24 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 82c148d..2f83425 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -509,28 +509,12 @@ and will also appear in a buffer named \"*Notmuch 
errors*\"."
(error (buffer-substring beg end))
))

-(defun notmuch-search-set-tags (tags)
-  (save-excursion
-(end-of-line)
-(re-search-backward "(")
-(forward-char)
-(let ((beg (point))
- (inhibit-read-only t))
-  (re-search-forward ")")
-  (backward-char)
-  (let ((end (point)))
-   (delete-region beg end)
-   (insert (propertize (mapconcat  'identity tags " ")
-   'face 'notmuch-tag-face))
-
-(defun notmuch-search-get-tags ()
-  (save-excursion
-(end-of-line)
-(re-search-backward "(")
-(let ((beg (+ (point) 1)))
-  (re-search-forward ")")
-  (let ((end (- (point) 1)))
-   (split-string (buffer-substring-no-properties beg end))
+(defun notmuch-search-set-tags (tags  pos)
+  (let ((new-result (plist-put (notmuch-search-get-result pos) :tags tags)))
+(notmuch-search-update-result new-result pos)))
+
+(defun notmuch-search-get-tags ( pos)
+  (plist-get (notmuch-search-get-result pos) :tags))

 (defun notmuch-search-get-tags-region (beg end)
   (save-excursion
@@ -583,6 +567,29 @@ This function advances the next thread when finished."
   (notmuch-search-tag '("-inbox"))
   (notmuch-search-next-thread))

+(defun notmuch-search-update-result (result  pos)
+  "Update the result object of the current thread and redraw it."
+  (let ((start (notmuch-search-result-beginning pos))
+   (end (notmuch-search-result-end pos))
+   (init-point (point))
+   (inhibit-read-only t))
+;; Delete the current thread
+(delete-region start end)
+;; Insert the updated thread
+(notmuch-search-show-result result start)
+;; There may have been markers pointing into the text we just
+;; replaced.  For the most part, there's nothing we can do about
+;; this, but we can fix markers that were at point (which includes
+;; point itself and any save-excursions for which point hasn't
+;; moved) by re-inserting the text that should come before point
+;; before markers.
+(when (and (>= init-point start) (<= init-point end))
+  (let* ((new-end (notmuch-search-result-end start))
+(new-point (if (= init-point end)
+   new-end
+ (min init-point (- new-end 1)
+   (insert-before-markers (delete-and-extract-region start new-point))
+
 (defun notmuch-search-process-sentinel (proc msg)
   "Add a message to let user know when \"notmuch search\" exits"
   (let ((buffer (process-buffer proc))
@@ -745,10 +752,10 @@ non-authors is found, assume that all of the authors 
match."
 (mapconcat 'identity (plist-get result :tags) " ")
 'font-lock-face 'notmuch-tag-face) ")")

-(defun notmuch-search-show-result (result)
+(defun notmuch-search-show-result (result  pos)
   ;; Ignore excluded matches
   (unless (= (plist-get result :matched) 0)
-(let ((beg (point-max)))
+(let ((beg (or pos (point-max
   (save-excursion
(goto-char beg)
(dolist (spec notmuch-search-result-format)
-- 
1.7.10



[PATCH 2/7] emacs: Use text properties instead of overlays for tag coloring

2012-07-12 Thread Austin Clements
Previously, tag-based search result highlighting was done by creating
an overlay over each search result.  However, overlays have annoying
front- and rear-advancement semantics that make it difficult to
manipulate text at their boundaries, which the next patch will do.
They also have performance problems (creating an overlay is linear in
the number of overlays between point and the new overlay, making
highlighting a search buffer quadratic in the number of results).

Text properties have neither problem.  However, text properties make
it more difficult to apply multiple faces since, unlike with overlays,
a given character can only have a single 'face text property.  Hence,
we introduce a utility function that combines faces into any existing
'face text properties.

Using this utility function, it's straightforward to apply all of the
appropriate tag faces in notmuch-search-color-line.
---
 emacs/notmuch-lib.el |   15 +++
 emacs/notmuch.el |   21 +++--
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index aa25513..30db58f 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -269,6 +269,21 @@ current buffer, if possible."
   (loop for (key value . rest) on plist by #'cddr
collect (cons (intern (substring (symbol-name key) 1)) value)))

+(defun notmuch-combine-face-text-property (start end face)
+  "Combine FACE into the 'face text property between START and END.
+
+This function combines FACE with any existing faces between START
+and END.  Attributes specified by FACE take precedence over
+existing attributes.  FACE must be a face name (a symbol or
+string), a property list of face attributes, or a list of these."
+
+  (let ((pos start))
+(while (< pos end)
+  (let ((cur (get-text-property pos 'face))
+   (next (next-single-property-change pos 'face nil end)))
+   (put-text-property pos next 'face (cons face cur))
+   (setq pos next)
+
 ;; Compatibility functions for versions of emacs before emacs 23.
 ;;
 ;; Both functions here were copied from emacs 23 with the following copyright:
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index ef18927..82c148d 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -633,20 +633,13 @@ foreground and blue background."

 (defun notmuch-search-color-line (start end line-tag-list)
   "Colorize lines in `notmuch-show' based on tags."
-  ;; Create the overlay only if the message has tags which match one
-  ;; of those specified in `notmuch-search-line-faces'.
-  (let (overlay)
-(mapc (lambda (elem)
-   (let ((tag (car elem))
- (attributes (cdr elem)))
- (when (member tag line-tag-list)
-   (when (not overlay)
- (setq overlay (make-overlay start end)))
-   ;; Merge the specified properties with any already
-   ;; applied from an earlier match.
-   (overlay-put overlay 'face
-(append (overlay-get overlay 'face) attributes)
- notmuch-search-line-faces)))
+  (mapc (lambda (elem)
+ (let ((tag (car elem))
+   (attributes (cdr elem)))
+   (when (member tag line-tag-list)
+ (notmuch-combine-face-text-property start end attributes
+   ;; Reverse the list so earlier entries take precedence
+   (reverse notmuch-search-line-faces)))

 (defun notmuch-search-author-propertize (authors)
   "Split `authors' into matching and non-matching authors and
-- 
1.7.10



[PATCH 1/7] emacs: Record thread search result object in a text property

2012-07-12 Thread Austin Clements
This also provides utility functions for working with this text
property that get its value, find its start, and find its end.
---
 emacs/notmuch.el |   27 +++
 1 file changed, 27 insertions(+)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index fabb7c0..ef18927 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -401,6 +401,32 @@ Complete list of currently available key bindings:
mode-name "notmuch-search")
   (setq buffer-read-only t))

+(defun notmuch-search-get-result ( pos)
+  "Return the result object for the thread at POS (or point).
+
+If there is no thread at POS (or point), returns nil."
+  (get-text-property (or pos (point)) 'notmuch-search-result))
+
+(defun notmuch-search-result-beginning ( pos)
+  "Return the point at the beginning of the thread at POS (or point).
+
+If there is no thread at POS (or point), returns nil."
+  (when (notmuch-search-get-result pos)
+;; We pass 1+point because previous-single-property-change starts
+;; searching one before the position we give it.
+(previous-single-property-change (1+ (or pos (point)))
+'notmuch-search-result nil (point-min
+
+(defun notmuch-search-result-end ( pos)
+  "Return the point at the end of the thread at POS (or point).
+
+The returned point will be just after the newline character that
+ends the result line.  If there is no thread at POS (or point),
+returns nil"
+  (when (notmuch-search-get-result pos)
+(next-single-property-change (or pos (point)) 'notmuch-search-result
+nil (point-max
+
 (defun notmuch-search-properties-in-region (property beg end)
   (save-excursion
 (let ((output nil)
@@ -736,6 +762,7 @@ non-authors is found, assume that all of the authors match."
  (notmuch-search-insert-field (car spec) (cdr spec) result))
(insert "\n")
(notmuch-search-color-line beg (point) (plist-get result :tags))
+   (put-text-property beg (point) 'notmuch-search-result result)
(put-text-property beg (point) 'notmuch-search-thread-id
   (concat "thread:" (plist-get result :thread)))
(put-text-property beg (point) 'notmuch-search-authors
-- 
1.7.10



[PATCH v4 3/3] Use the structured format printer for JSON in notmuch search.

2012-07-12 Thread Austin Clements
Quoth myself on Jul 12 at  8:02 pm:
> This is fantastic.  It simplifies the code a lot, and I think it opens
> up opportunities to simplify it even further.
> 
> Detailed comments are below, but first one general comment.  For the
> text format, I wonder if most of the special case code would go away
> with a stub sprinter that did nothing for most operations and for
> string printed the string followed by a newline (or maybe the newline
> would be printed by the "frame" method or whatever you end up calling
> it).  I believe this would unify all of the code in do_search_tags and
> do_search_files between the two formats.  For do_search_threads,
> you'll still need some special-casing to format the summary line, but
> I think it would unify all of the framing code.  (If this does work
> out, some of my comments below will be irrelevant.)

Oh, wait, the text format adds prefixes...

I think something along the lines of what I described above would
still simplify things.  I can think of two ways to do it.  You could
have a completely stub sprinter where everything is a no-op; that
would at least save the predication of calls like
format->begin_list ().  Or, you could have an additional function for
the text formatter that registers a prefix, and have the string method
first print that prefix and then the argument string.  This additional
function doesn't have to be a method of sprinter; it could just be a
regular function that stashes the prefix away somewhere (a global
variable's probably fine; if you want to be fancy you store it in an
sprinter_text extension of sprinter_t like the one JSON uses).  For
the JSON format, this would be a no-op, so you could call it
regardless of what format you've selected.


[PATCH v4 3/3] Use the structured format printer for JSON in notmuch search.

2012-07-12 Thread Austin Clements
This is fantastic.  It simplifies the code a lot, and I think it opens
up opportunities to simplify it even further.

Detailed comments are below, but first one general comment.  For the
text format, I wonder if most of the special case code would go away
with a stub sprinter that did nothing for most operations and for
string printed the string followed by a newline (or maybe the newline
would be printed by the "frame" method or whatever you end up calling
it).  I believe this would unify all of the code in do_search_tags and
do_search_files between the two formats.  For do_search_threads,
you'll still need some special-casing to format the summary line, but
I think it would unify all of the framing code.  (If this does work
out, some of my comments below will be irrelevant.)

Quoth craven at gmx.net on Jul 12 at  9:43 am:
> This patch switches from the current ad-hoc printer to the structured
> output formatter in sprinter.h.
> 
> It removes search_format_t, replaces it by sprinter_t and inlines the
> text printer where necessary.
> 
> The tests are changed (only whitespaces and regular expressions) in
> order to make them PASS for the new structured output formatter.
> ---
>  notmuch-search.c   | 349 
> ++---
>  test/json  |  20 +--
>  test/search-output | 270 +
>  3 files changed, 288 insertions(+), 351 deletions(-)
> 
> diff --git a/notmuch-search.c b/notmuch-search.c
> index 3be296d..b853f5f 100644
> --- a/notmuch-search.c
> +++ b/notmuch-search.c
> @@ -19,6 +19,7 @@
>   */
>  
>  #include "notmuch-client.h"
> +#include "sprinter.h"
>  
>  typedef enum {
>  OUTPUT_SUMMARY,
> @@ -28,91 +29,9 @@ typedef enum {
>  OUTPUT_TAGS
>  } output_t;
>  
> -typedef struct search_format {
> -const char *results_start;
> -const char *item_start;
> -void (*item_id) (const void *ctx,
> -  const char *item_type,
> -  const char *item_id);
> -void (*thread_summary) (const void *ctx,
> - const char *thread_id,
> - const time_t date,
> - const int matched,
> - const int total,
> - const char *authors,
> - const char *subject);
> -const char *tag_start;
> -const char *tag;
> -const char *tag_sep;
> -const char *tag_end;
> -const char *item_sep;
> -const char *item_end;
> -const char *results_end;
> -const char *results_null;
> -} search_format_t;
> -
> -static void
> -format_item_id_text (const void *ctx,
> -  const char *item_type,
> -  const char *item_id);
> -
> -static void
> -format_thread_text (const void *ctx,
> - const char *thread_id,
> - const time_t date,
> - const int matched,
> - const int total,
> - const char *authors,
> - const char *subject);
> -static const search_format_t format_text = {
> -"",
> - "",
> - format_item_id_text,
> - format_thread_text,
> - " (",
> - "%s", " ",
> - ")", "\n",
> - "",
> -"\n",
> -"",
> -};
> -
> -static void
> -format_item_id_json (const void *ctx,
> -  const char *item_type,
> -  const char *item_id);
> -
> -static void
> -format_thread_json (const void *ctx,
> - const char *thread_id,
> - const time_t date,
> - const int matched,
> - const int total,
> - const char *authors,
> - const char *subject);
> -
> -/* Any changes to the JSON format should be reflected in the file
> - * devel/schemata. */
> -static const search_format_t format_json = {
> -"[",
> - "{",
> - format_item_id_json,
> - format_thread_json,
> - "\"tags\": [",
> - "\"%s\"", ", ",
> - "]", ",\n",
> - "}",
> -"]\n",
> -"]\n",
> -};
> -
> -static void
> -format_item_id_text (unused (const void *ctx),
> -  const char *item_type,
> -  const char *item_id)
> -{
> -printf ("%s%s", item_type, item_id);
> -}
> +static const char * text_item_sep = "\n";
> +static const char * text_results_null = "";
> +static const char * text_results_end = "\n";

Given that you're special-casing the text format anyway, I think it's
actually more readable if you hard-code these in the appropriate
places below.

>  
>  static char *
>  sanitize_string (const void *ctx, const char *str)
> @@ -131,72 +50,8 @@ sanitize_string (const void *ctx, const char *str)
>  return out;
>  }
>  
> -static void
> -format_thread_text (const void *ctx,
> - const char *thread_id,
> - const time_t date,
> - const int matched,
> - const int total,
> -  

[RFC] emacs: use ido completing read for address completion in message mode

2012-07-12 Thread Mark Walters
This patch uses ido-completing-read for address completion in message
mode.  Although ido-completing-read is nominally a drop-in replacement
for completing-read `initial` and `default` behave rather differently
and it makes sense to use `default` rather than `initial` in the ido
case.
---

There was some interest on irc for using ido-completing-read for
address completion in message-mode so here is a trivial patch.

I am not sure I like the ido-completing-read behaviour as typing in
the minibuffer matches anywhere in the address not just at the start
of the address. I think there is a difference between this and setting
the users from address where it is likely that many of their addresses
start the same.

But it might be a starting point for experiments.

Best wishes

Mark


 emacs/notmuch-address.el |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
index 2bf762b..08e0e38 100644
--- a/emacs/notmuch-address.el
+++ b/emacs/notmuch-address.el
@@ -61,9 +61,9 @@ line."
  ((eq num-options 1)
   (car options))
  (t
-  (completing-read (format "Address (%s matches): " 
num-options)
-   (cdr options) nil nil (car options)
-   'notmuch-address-history)
+  (ido-completing-read (format "Address (%s matches): " 
num-options)
+   (cdr options) nil nil nil
+   'notmuch-address-history (car options))
 (if chosen
(progn
  (push chosen notmuch-address-history)
-- 
1.7.9.1



[PATCH v4 1/3] Add support for structured output formatters.

2012-07-12 Thread Austin Clements
Quoth craven at gmx.net on Jul 12 at  9:43 am:
> This patch adds a new type sprinter_t, which is used for structured
> formatting, e.g. JSON or S-Expressions. The structure printer is the
> code from Austin Clements (id:87d34hsdx8.fsf at awakening.csail.mit.edu).
> 
> The structure printer contains the following function pointers:
> 
> /* start a new map/dictionary structure.
>  */
> void (*begin_map) (struct sprinter *);
> 
> /* start a new list/array structure
>  */
> void (*begin_list) (struct sprinter *);
> 
> /* end the last opened list or map structure
>  */
> void (*end) (struct sprinter *);
> 
> /* print one string/integer/boolean/null element (possibly inside a
>  * list or map
>  */
> void (*string) (struct sprinter *, const char *);
> void (*integer) (struct sprinter *, int);
> void (*boolean) (struct sprinter *, notmuch_bool_t);
> void (*null) (struct sprinter *);
> 
> /* print the key of a map's key/value pair.
>  */
> void (*map_key) (struct sprinter *, const char *);
> 
> /* print a frame delimiter (such as a newline or extra whitespace)
>  */
> void (*frame) (struct sprinter *);
> 
> The printer can (and should) use internal state to insert delimiters and
> syntax at the correct places.
> 
> Example:
> 
> format->begin_map(format);
> format->map_key(format, "foo");
> format->begin_list(format);
> format->integer(format, 1);
> format->integer(format, 2);
> format->integer(format, 3);
> format->end(format);
> format->map_key(format, "bar");
> format->begin_map(format);
> format->map_key(format, "baaz");
> format->string(format, "hello world");
> format->end(format);
> format->end(format);
> 
> would output JSON as follows:
> 
> {"foo": [1, 2, 3], "bar": { "baaz": "hello world"}}
> ---
>  sprinter.h | 49 +
>  1 file changed, 49 insertions(+)
>  create mode 100644 sprinter.h
> 
> diff --git a/sprinter.h b/sprinter.h
> new file mode 100644
> index 000..1dad9a0
> --- /dev/null
> +++ b/sprinter.h
> @@ -0,0 +1,49 @@
> +#ifndef NOTMUCH_SPRINTER_H
> +#define NOTMUCH_SPRINTER_H
> +
> +/* for notmuch_bool_t */
> +#include "notmuch-client.h"
> +
> +/* Structure printer interface */
> +typedef struct sprinter
> +{
> +/* start a new map/dictionary structure.
> + */
> +void (*begin_map) (struct sprinter *);
> +
> +/* start a new list/array structure
> + */
> +void (*begin_list) (struct sprinter *);
> +
> +/* end the last opened list or map structure
> + */
> +void (*end) (struct sprinter *);
> +
> +/* print one string/integer/boolean/null element (possibly inside a
> + * list or map
> + */
> +void (*string) (struct sprinter *, const char *);

Oh, also, the documentation for string should mention the string
argument should be UTF-8 encoded.

> +void (*integer) (struct sprinter *, int);
> +void (*boolean) (struct sprinter *, notmuch_bool_t);
> +void (*null) (struct sprinter *);
> +
> +/* print the key of a map's key/value pair.
> + */
> +void (*map_key) (struct sprinter *, const char *);
> +
> +/* print a frame delimiter (such as a newline or extra whitespace)
> + */
> +void (*frame) (struct sprinter *);
> +} sprinter_t;
> +
> +/* Create a new structure printer that emits JSON */
> +struct sprinter *
> +sprinter_json_new(const void *ctx, FILE *stream);
> +
> +/* A dummy structure printer that signifies that standard text output is
> + * to be used instead of any structured format.
> + */
> +struct sprinter *
> +sprinter_text;
> +
> +#endif // NOTMUCH_SPRINTER_H


[PATCH v4 2/3] Add structured output formatter for JSON.

2012-07-12 Thread Austin Clements
Quoth craven at gmx.net on Jul 12 at  9:43 am:
> Using the new structured printer support in sprinter.h, implement
> sprinter_json_new, which returns a new JSON structured output
> formatter.
> 
> The formatter prints output similar to the existing JSON, but with
> differences in whitespace (mostly newlines).
> ---
>  Makefile.local |   1 +
>  sprinter.c | 172 
> +
>  2 files changed, 173 insertions(+)
>  create mode 100644 sprinter.c
> 
> diff --git a/Makefile.local b/Makefile.local
> index a890df2..8baf0c2 100644
> --- a/Makefile.local
> +++ b/Makefile.local
> @@ -290,6 +290,7 @@ notmuch_client_srcs = \
>   notmuch-show.c  \
>   notmuch-tag.c   \
>   notmuch-time.c  \
> + sprinter.c  \
>   query-string.c  \
>   mime-node.c \
>   crypto.c\
> diff --git a/sprinter.c b/sprinter.c
> new file mode 100644
> index 000..649f79a
> --- /dev/null
> +++ b/sprinter.c
> @@ -0,0 +1,172 @@
> +#include 
> +#include 
> +#include 
> +#include "sprinter.h"
> +
> +#define ARRAY_SIZE(arr) (sizeof (arr) / sizeof (arr[0]))
> +
> +struct sprinter *
> +sprinter_text = NULL;
> +
> +/*
> + * Every below here is the implementation of the JSON printer.

s/Every/Everything/

Or this can be shortened to just

/*
 * JSON printer
 */

If we wind up with separate files for the JSON and S-exp printers, I
don't think this comment is necessary at all.

> + */
> +
> +struct sprinter_json
> +{
> +struct sprinter vtable;
> +FILE *stream;
> +/* Top of the state stack, or NULL if the printer is not currently
> + * inside any aggregate types. */
> +struct json_state *state;
> +};
> +
> +struct json_state
> +{
> +struct json_state *parent;
> +/* True if nothing has been printed in this aggregate yet.
> + * Suppresses the comma before a value. */
> +notmuch_bool_t first;
> +/* The character that closes the current aggregate. */
> +char close;
> +};
> +
> +/* Helper function to set up the stream to print a value.  If this
> + * value follows another value, prints a comma. */
> +static struct sprinter_json *
> +json_begin_value(struct sprinter *sp)

As Tomi pointed out, there should be spaces before the parameter
lists (sorry).

> +{
> +struct sprinter_json *spj = (struct sprinter_json*)sp;
> +if (spj->state) {
> + if (!spj->state->first)
> + fputs (", ", spj->stream);
> + else
> + spj->state->first = false;
> +}
> +return spj;
> +}
> +
> +/* Helper function to begin an aggregate type.  Prints the open
> + * character and pushes a new state frame. */
> +static void
> +json_begin_aggregate(struct sprinter *sp, char open, char close)
> +{
> +struct sprinter_json *spj = json_begin_value (sp);
> +struct json_state *state = talloc (spj, struct json_state);
> +
> +fputc (open, spj->stream);
> +state->parent = spj->state;
> +state->first = true;
> +state->close = close;
> +spj->state = state;
> +}
> +
> +static void
> +json_begin_map(struct sprinter *sp)
> +{
> +json_begin_aggregate (sp, '{', '}');
> +}
> +
> +static void
> +json_begin_list(struct sprinter *sp)
> +{
> +json_begin_aggregate (sp, '[', ']');
> +}
> +
> +static void
> +json_end(struct sprinter *sp)
> +{
> +struct sprinter_json *spj = (struct sprinter_json*)sp;
> +struct json_state *state = spj->state;
> +
> +fputc (spj->state->close, spj->stream);
> +spj->state = state->parent;
> +talloc_free (state);
> +if(spj->state == NULL)

Ah, another missing space.

> + fputc ('\n', spj->stream);
> +}
> +
> +static void
> +json_string(struct sprinter *sp, const char *val)
> +{
> +static const char * const escapes[] = {
> + ['\"'] = "\\\"", ['\\'] = "", ['\b'] = "\\b",
> + ['\f'] = "\\f",  ['\n'] = "\\n",  ['\t'] = "\\t"
> +};
> +struct sprinter_json *spj = json_begin_value (sp);
> +fputc ('"', spj->stream);
> +for (; *val; ++val) {
> + unsigned char ch = *val;
> + if (ch < ARRAY_SIZE(escapes) && escapes[ch])
> + fputs (escapes[ch], spj->stream);
> + else if (ch >= 32)
> + fputc (ch, spj->stream);
> + else
> + fprintf (spj->stream, "\\u%04x", ch);
> +}
> +fputc ('"', spj->stream);
> +}
> +
> +static void
> +json_integer(struct sprinter *sp, int val)
> +{
> +struct sprinter_json *spj = json_begin_value (sp);
> +fprintf (spj->stream, "%d", val);
> +}
> +
> +static void
> +json_boolean(struct sprinter *sp, notmuch_bool_t val)
> +{
> +struct sprinter_json *spj = json_begin_value (sp);
> +fputs (val ? "true" : "false", spj->stream);
> +}
> +
> +static void
> +json_null(struct sprinter *sp)
> +{
> +struct sprinter_json *spj = json_begin_value (sp);
> +fputs ("null", spj->stream);
> +}
> +
> +static void
> +json_map_key(struct sprinter *sp, const char *key)
> +{
> +struct sprinter_json 

[PATCH v4 2/3] Add structured output formatter for JSON.

2012-07-12 Thread Austin Clements
Quoth Tomi Ollila on Jul 12 at  1:10 pm:
> On Thu, Jul 12 2012, craven at gmx.net wrote:
> 
> > Using the new structured printer support in sprinter.h, implement
> > sprinter_json_new, which returns a new JSON structured output
> > formatter.
> >
> > The formatter prints output similar to the existing JSON, but with
> > differences in whitespace (mostly newlines).
> > ---
> >  Makefile.local |   1 +
> >  sprinter.c | 172 
> > +
> >  2 files changed, 173 insertions(+)
> >  create mode 100644 sprinter.c
> >
> > diff --git a/Makefile.local b/Makefile.local
> > index a890df2..8baf0c2 100644
> > --- a/Makefile.local
> > +++ b/Makefile.local
> > @@ -290,6 +290,7 @@ notmuch_client_srcs =   \
> > notmuch-show.c  \
> > notmuch-tag.c   \
> > notmuch-time.c  \
> > +   sprinter.c  \
> > query-string.c  \
> > mime-node.c \
> > crypto.c\
> > diff --git a/sprinter.c b/sprinter.c
> > new file mode 100644
> > index 000..649f79a
> > --- /dev/null
> > +++ b/sprinter.c
> > @@ -0,0 +1,172 @@
> > +#include 
> > +#include 
> > +#include 
> > +#include "sprinter.h"
> > +
> > +#define ARRAY_SIZE(arr) (sizeof (arr) / sizeof (arr[0]))
> 
> You're including sprinter.h which includes notmuch-client.h which
> defines ARRAY_SIZE (Interesting that you did not get error(/warning?)
> about this)
> 
> Rest looks good -- except the whitespace -- as I looked through Austin's
> code yesterday you're just replicated the same lines :D. Easiest
> to fix is probably just running 
> uncrustify -c devel/uncrustify.cfg --replace sprinter.c 

Oops.  Shame on me.  Is anything wrong besides the missing space
before the argument list of all of the function declarations?


[PATCH v4 1/3] Add support for structured output formatters.

2012-07-12 Thread Austin Clements
Quoth craven at gmx.net on Jul 12 at  9:43 am:
> This patch adds a new type sprinter_t, which is used for structured
> formatting, e.g. JSON or S-Expressions. The structure printer is the
> code from Austin Clements (id:87d34hsdx8.fsf at awakening.csail.mit.edu).
> 
> The structure printer contains the following function pointers:
> 
> /* start a new map/dictionary structure.
>  */
> void (*begin_map) (struct sprinter *);
> 
> /* start a new list/array structure
>  */
> void (*begin_list) (struct sprinter *);
> 
> /* end the last opened list or map structure
>  */
> void (*end) (struct sprinter *);
> 
> /* print one string/integer/boolean/null element (possibly inside a
>  * list or map
>  */
> void (*string) (struct sprinter *, const char *);
> void (*integer) (struct sprinter *, int);
> void (*boolean) (struct sprinter *, notmuch_bool_t);
> void (*null) (struct sprinter *);
> 
> /* print the key of a map's key/value pair.
>  */
> void (*map_key) (struct sprinter *, const char *);
> 
> /* print a frame delimiter (such as a newline or extra whitespace)
>  */
> void (*frame) (struct sprinter *);
> 
> The printer can (and should) use internal state to insert delimiters and
> syntax at the correct places.
> 
> Example:
> 
> format->begin_map(format);
> format->map_key(format, "foo");
> format->begin_list(format);
> format->integer(format, 1);
> format->integer(format, 2);
> format->integer(format, 3);
> format->end(format);
> format->map_key(format, "bar");
> format->begin_map(format);
> format->map_key(format, "baaz");
> format->string(format, "hello world");
> format->end(format);
> format->end(format);
> 
> would output JSON as follows:
> 
> {"foo": [1, 2, 3], "bar": { "baaz": "hello world"}}
> ---
>  sprinter.h | 49 +
>  1 file changed, 49 insertions(+)
>  create mode 100644 sprinter.h
> 
> diff --git a/sprinter.h b/sprinter.h
> new file mode 100644
> index 000..1dad9a0
> --- /dev/null
> +++ b/sprinter.h
> @@ -0,0 +1,49 @@
> +#ifndef NOTMUCH_SPRINTER_H
> +#define NOTMUCH_SPRINTER_H
> +
> +/* for notmuch_bool_t */

Style/consistency nit: all of the comments should start with a capital
letter and anything that's a sentence should end with a period (some
of your comments do, some of them don't).

> +#include "notmuch-client.h"
> +
> +/* Structure printer interface */
> +typedef struct sprinter
> +{
> +/* start a new map/dictionary structure.

This should probably mention how other functions should be called
within the map structure.  Perhaps,

/* Start a new map/dictionary structure.  This should be followed by a
 * sequence of alternating calls to map_key and one of the value
 * printing functions until the map is ended.
 */

(You had a comment to this effect in one of your earlier patches.)

> + */
> +void (*begin_map) (struct sprinter *);
> +
> +/* start a new list/array structure
> + */
> +void (*begin_list) (struct sprinter *);
> +
> +/* end the last opened list or map structure
> + */
> +void (*end) (struct sprinter *);
> +
> +/* print one string/integer/boolean/null element (possibly inside a
> + * list or map

Missing close paren.

> + */
> +void (*string) (struct sprinter *, const char *);
> +void (*integer) (struct sprinter *, int);
> +void (*boolean) (struct sprinter *, notmuch_bool_t);
> +void (*null) (struct sprinter *);
> +
> +/* print the key of a map's key/value pair.
> + */
> +void (*map_key) (struct sprinter *, const char *);
> +
> +/* print a frame delimiter (such as a newline or extra whitespace)
> + */
> +void (*frame) (struct sprinter *);

I wonder if frame is still necessary.  At the time, I was thinking
that we would use embedded newline framing to do streaming JSON
parsing, but I wound up going with a general incremental JSON parser,
eliminating the need for framing.

It's probably still useful to have a function that inserts a newline
purely for readability/debuggability purposes.  In practice, the
implementation would be the same as frame (though if it's for
readability, maybe you'd want to print the comma before the newline
instead of after), but since the intent would be different, it would
need different documentation and maybe a different name.  "Frame"
isn't a bad name for either intent.  We can't use "break".
"Separator" (or just "sep")?  "Space"?  "Split"?  The comment could be
something like

/* Insert a separator for improved readability without affecting the
 * abstract syntax of the structure being printed.  For JSON, this is
 * simply a line break.
 */

> +} sprinter_t;
> +
> +/* Create a new structure printer that emits JSON */
> +struct sprinter *
> +sprinter_json_new(const void *ctx, FILE *stream);

This should probably be called sprinter_json_create to be consistent
with the naming of other constructors in notmuch.

Also, missing space between the function name and parameter list
(sorry, my fault).

> +
> +/* A dummy structure 

[PATCH 1/2] contrib/nmbug/ nmbug-status: restored out['subject']... block level

2012-07-12 Thread David Bremner
On Wed, 11 Jul 2012 12:10:04 +0300, Tomi Ollila  wrote:
> In reformatting the line 111 accidentally indented to one indentation
> level too much (happens easily when interactively indenting python
> code using emacs). The line now has 4 spacess less indentation, thus
> restoring it to the block level it belongs.

Thanks Tomi, both pushed

d


[PATCH 1/2] contrib/nmbug: make nmbug a subdirectory

2012-07-12 Thread David Bremner
On Sat,  7 Jul 2012 13:35:53 -0600, david at tethera.net wrote:
> From: David Bremner 
> 
> I want to ship the status tool here as well, along with a sample
> config file.

Pushed this one, along with an (almost) cleaned up version of v3. 

d


[PATCH v4 1/3] Add support for structured output formatters.

2012-07-12 Thread cra...@gmx.net
> what is the advantage of having this as one function rather than end_map
> and end_list? Indeed, my choice (but I think most other people would
> disagree) would be to have both functions but still keep state as this
> currently does and then throw an error if the code closes the wrong
> thing.

There's probably no advantage, one way or the other is fine, I'd say.
I've thought about introducing checks into the formatter functions, to
raise errors for improper closing, map_key not inside a map and things
like that, I just wasn't sure that would be acceptable.

> A second question: do you have an implementation in this style for
> s-expressions. I find it hard to tell whether the interface is right
> with just a single example. Even a completely hacky not ready for review
> example would be helpful.

See the attached patch :)

Thanks for the suggestions!

Peter
-- next part --
A non-text attachment was scrubbed...
Name: 0001-Add-an-S-Expression-output-format.patch
Type: text/x-patch
Size: 6661 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20120712/44250143/attachment.bin>


[PATCH v4 2/3] Add structured output formatter for JSON.

2012-07-12 Thread Tomi Ollila
On Thu, Jul 12 2012, craven at gmx.net wrote:

> Using the new structured printer support in sprinter.h, implement
> sprinter_json_new, which returns a new JSON structured output
> formatter.
>
> The formatter prints output similar to the existing JSON, but with
> differences in whitespace (mostly newlines).
> ---
>  Makefile.local |   1 +
>  sprinter.c | 172 
> +
>  2 files changed, 173 insertions(+)
>  create mode 100644 sprinter.c
>
> diff --git a/Makefile.local b/Makefile.local
> index a890df2..8baf0c2 100644
> --- a/Makefile.local
> +++ b/Makefile.local
> @@ -290,6 +290,7 @@ notmuch_client_srcs = \
>   notmuch-show.c  \
>   notmuch-tag.c   \
>   notmuch-time.c  \
> + sprinter.c  \
>   query-string.c  \
>   mime-node.c \
>   crypto.c\
> diff --git a/sprinter.c b/sprinter.c
> new file mode 100644
> index 000..649f79a
> --- /dev/null
> +++ b/sprinter.c
> @@ -0,0 +1,172 @@
> +#include 
> +#include 
> +#include 
> +#include "sprinter.h"
> +
> +#define ARRAY_SIZE(arr) (sizeof (arr) / sizeof (arr[0]))

You're including sprinter.h which includes notmuch-client.h which
defines ARRAY_SIZE (Interesting that you did not get error(/warning?)
about this)

Rest looks good -- except the whitespace -- as I looked through Austin's
code yesterday you're just replicated the same lines :D. Easiest
to fix is probably just running 
uncrustify -c devel/uncrustify.cfg --replace sprinter.c 

Tomi

> +
> +struct sprinter *
> +sprinter_text = NULL;
> +
> +/*
> + * Every below here is the implementation of the JSON printer.
> + */
> +
> +struct sprinter_json
> +{
> +struct sprinter vtable;
> +FILE *stream;
> +/* Top of the state stack, or NULL if the printer is not currently
> + * inside any aggregate types. */
> +struct json_state *state;
> +};
> +
> +struct json_state
> +{
> +struct json_state *parent;
> +/* True if nothing has been printed in this aggregate yet.
> + * Suppresses the comma before a value. */
> +notmuch_bool_t first;
> +/* The character that closes the current aggregate. */
> +char close;
> +};
> +
> +/* Helper function to set up the stream to print a value.  If this
> + * value follows another value, prints a comma. */
> +static struct sprinter_json *
> +json_begin_value(struct sprinter *sp)
> +{
> +struct sprinter_json *spj = (struct sprinter_json*)sp;
> +if (spj->state) {
> + if (!spj->state->first)
> + fputs (", ", spj->stream);
> + else
> + spj->state->first = false;
> +}
> +return spj;
> +}
> +
> +/* Helper function to begin an aggregate type.  Prints the open
> + * character and pushes a new state frame. */
> +static void
> +json_begin_aggregate(struct sprinter *sp, char open, char close)
> +{
> +struct sprinter_json *spj = json_begin_value (sp);
> +struct json_state *state = talloc (spj, struct json_state);
> +
> +fputc (open, spj->stream);
> +state->parent = spj->state;
> +state->first = true;
> +state->close = close;
> +spj->state = state;
> +}
> +
> +static void
> +json_begin_map(struct sprinter *sp)
> +{
> +json_begin_aggregate (sp, '{', '}');
> +}
> +
> +static void
> +json_begin_list(struct sprinter *sp)
> +{
> +json_begin_aggregate (sp, '[', ']');
> +}
> +
> +static void
> +json_end(struct sprinter *sp)
> +{
> +struct sprinter_json *spj = (struct sprinter_json*)sp;
> +struct json_state *state = spj->state;
> +
> +fputc (spj->state->close, spj->stream);
> +spj->state = state->parent;
> +talloc_free (state);
> +if(spj->state == NULL)
> + fputc ('\n', spj->stream);
> +}
> +
> +static void
> +json_string(struct sprinter *sp, const char *val)
> +{
> +static const char * const escapes[] = {
> + ['\"'] = "\\\"", ['\\'] = "", ['\b'] = "\\b",
> + ['\f'] = "\\f",  ['\n'] = "\\n",  ['\t'] = "\\t"
> +};
> +struct sprinter_json *spj = json_begin_value (sp);
> +fputc ('"', spj->stream);
> +for (; *val; ++val) {
> + unsigned char ch = *val;
> + if (ch < ARRAY_SIZE(escapes) && escapes[ch])
> + fputs (escapes[ch], spj->stream);
> + else if (ch >= 32)
> + fputc (ch, spj->stream);
> + else
> + fprintf (spj->stream, "\\u%04x", ch);
> +}
> +fputc ('"', spj->stream);
> +}
> +
> +static void
> +json_integer(struct sprinter *sp, int val)
> +{
> +struct sprinter_json *spj = json_begin_value (sp);
> +fprintf (spj->stream, "%d", val);
> +}
> +
> +static void
> +json_boolean(struct sprinter *sp, notmuch_bool_t val)
> +{
> +struct sprinter_json *spj = json_begin_value (sp);
> +fputs (val ? "true" : "false", spj->stream);
> +}
> +
> +static void
> +json_null(struct sprinter *sp)
> +{
> +struct sprinter_json *spj = json_begin_value (sp);
> +fputs ("null", spj->stream);
> +}
> +
> +static void
> 

[PATCH v4 1/3] Add support for structured output formatters.

2012-07-12 Thread Tomi Ollila
On Thu, Jul 12 2012, craven at gmx.net wrote:

> This patch adds a new type sprinter_t, which is used for structured
> formatting, e.g. JSON or S-Expressions. The structure printer is the
> code from Austin Clements (id:87d34hsdx8.fsf at awakening.csail.mit.edu).

...

> +
> +/* Create a new structure printer that emits JSON */
> +struct sprinter *
> +sprinter_json_new(const void *ctx, FILE *stream);
> +
> +/* A dummy structure printer that signifies that standard text output is
> + * to be used instead of any structured format.
> + */
> +struct sprinter *
> +sprinter_text;

Looks good... but this needs to be 'extern struct sprinter * sprinter_text;'

see id:"1340563714-3103-1-git-send-email-tomi.ollila at iki.fi"
for reference.


> +
> +#endif // NOTMUCH_SPRINTER_H
> -- 
> 1.7.11.1

Tomi


[PATCH v4 1/3] Add support for structured output formatters.

2012-07-12 Thread Mark Walters

A couple more things:

On Thu, 12 Jul 2012, craven at gmx.net wrote:
> This patch adds a new type sprinter_t, which is used for structured
> formatting, e.g. JSON or S-Expressions. The structure printer is the
> code from Austin Clements (id:87d34hsdx8.fsf at awakening.csail.mit.edu).
>
> The structure printer contains the following function pointers:
>
> /* start a new map/dictionary structure.
>  */
> void (*begin_map) (struct sprinter *);
>
> /* start a new list/array structure
>  */
> void (*begin_list) (struct sprinter *);
>
> /* end the last opened list or map structure
>  */
> void (*end) (struct sprinter *);

what is the advantage of having this as one function rather than end_map
and end_list? Indeed, my choice (but I think most other people would
disagree) would be to have both functions but still keep state as this
currently does and then throw an error if the code closes the wrong
thing.

A second question: do you have an implementation in this style for
s-expressions. I find it hard to tell whether the interface is right
with just a single example. Even a completely hacky not ready for review
example would be helpful.

Best wishes

Mark

> /* print one string/integer/boolean/null element (possibly inside a
>  * list or map
>  */
> void (*string) (struct sprinter *, const char *);
> void (*integer) (struct sprinter *, int);
> void (*boolean) (struct sprinter *, notmuch_bool_t);
> void (*null) (struct sprinter *);
>
> /* print the key of a map's key/value pair.
>  */
> void (*map_key) (struct sprinter *, const char *);
>
> /* print a frame delimiter (such as a newline or extra whitespace)
>  */
> void (*frame) (struct sprinter *);
>
> The printer can (and should) use internal state to insert delimiters and
> syntax at the correct places.
>
> Example:
>
> format->begin_map(format);
> format->map_key(format, "foo");
> format->begin_list(format);
> format->integer(format, 1);
> format->integer(format, 2);
> format->integer(format, 3);
> format->end(format);
> format->map_key(format, "bar");
> format->begin_map(format);
> format->map_key(format, "baaz");
> format->string(format, "hello world");
> format->end(format);
> format->end(format);
>
> would output JSON as follows:
>
> {"foo": [1, 2, 3], "bar": { "baaz": "hello world"}}
> ---
>  sprinter.h | 49 +
>  1 file changed, 49 insertions(+)
>  create mode 100644 sprinter.h
>
> diff --git a/sprinter.h b/sprinter.h
> new file mode 100644
> index 000..1dad9a0
> --- /dev/null
> +++ b/sprinter.h
> @@ -0,0 +1,49 @@
> +#ifndef NOTMUCH_SPRINTER_H
> +#define NOTMUCH_SPRINTER_H
> +
> +/* for notmuch_bool_t */
> +#include "notmuch-client.h"
> +
> +/* Structure printer interface */
> +typedef struct sprinter
> +{
> +/* start a new map/dictionary structure.
> + */
> +void (*begin_map) (struct sprinter *);
> +
> +/* start a new list/array structure
> + */
> +void (*begin_list) (struct sprinter *);
> +
> +/* end the last opened list or map structure
> + */
> +void (*end) (struct sprinter *);
> +
> +/* print one string/integer/boolean/null element (possibly inside a
> + * list or map
> + */
> +void (*string) (struct sprinter *, const char *);
> +void (*integer) (struct sprinter *, int);
> +void (*boolean) (struct sprinter *, notmuch_bool_t);
> +void (*null) (struct sprinter *);
> +
> +/* print the key of a map's key/value pair.
> + */
> +void (*map_key) (struct sprinter *, const char *);
> +
> +/* print a frame delimiter (such as a newline or extra whitespace)
> + */
> +void (*frame) (struct sprinter *);
> +} sprinter_t;
> +
> +/* Create a new structure printer that emits JSON */
> +struct sprinter *
> +sprinter_json_new(const void *ctx, FILE *stream);
> +
> +/* A dummy structure printer that signifies that standard text output is
> + * to be used instead of any structured format.
> + */
> +struct sprinter *
> +sprinter_text;
> +
> +#endif // NOTMUCH_SPRINTER_H
> -- 
> 1.7.11.1
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v4 1/3] Add support for structured output formatters.

2012-07-12 Thread Mark Walters

Hi

this definitely looks nice. I have a couple of suggestions for the
comments:

On Thu, 12 Jul 2012, craven at gmx.net wrote:
> This patch adds a new type sprinter_t, which is used for structured
> formatting, e.g. JSON or S-Expressions. The structure printer is the
> code from Austin Clements (id:87d34hsdx8.fsf at awakening.csail.mit.edu).
>
> The structure printer contains the following function pointers:
>
> /* start a new map/dictionary structure.
>  */
> void (*begin_map) (struct sprinter *);
>
> /* start a new list/array structure
>  */
> void (*begin_list) (struct sprinter *);
>
> /* end the last opened list or map structure
>  */
> void (*end) (struct sprinter *);
>
> /* print one string/integer/boolean/null element (possibly inside a
>  * list or map
>  */
> void (*string) (struct sprinter *, const char *);
> void (*integer) (struct sprinter *, int);
> void (*boolean) (struct sprinter *, notmuch_bool_t);
> void (*null) (struct sprinter *);
>
> /* print the key of a map's key/value pair.
>  */
> void (*map_key) (struct sprinter *, const char *);
>
> /* print a frame delimiter (such as a newline or extra whitespace)
>  */
> void (*frame) (struct sprinter *);
>
> The printer can (and should) use internal state to insert delimiters and
> syntax at the correct places.
>
> Example:
>
> format->begin_map(format);
> format->map_key(format, "foo");
> format->begin_list(format);
> format->integer(format, 1);
> format->integer(format, 2);
> format->integer(format, 3);
> format->end(format);
> format->map_key(format, "bar");
> format->begin_map(format);
> format->map_key(format, "baaz");
> format->string(format, "hello world");
> format->end(format);
> format->end(format);
>
> would output JSON as follows:
>
> {"foo": [1, 2, 3], "bar": { "baaz": "hello world"}}
> ---
>  sprinter.h | 49 +
>  1 file changed, 49 insertions(+)
>  create mode 100644 sprinter.h
>
> diff --git a/sprinter.h b/sprinter.h
> new file mode 100644
> index 000..1dad9a0
> --- /dev/null
> +++ b/sprinter.h
> @@ -0,0 +1,49 @@
> +#ifndef NOTMUCH_SPRINTER_H
> +#define NOTMUCH_SPRINTER_H
> +
> +/* for notmuch_bool_t */
> +#include "notmuch-client.h"
> +
> +/* Structure printer interface */
> +typedef struct sprinter
> +{
> +/* start a new map/dictionary structure.
> + */
> +void (*begin_map) (struct sprinter *);
> +
> +/* start a new list/array structure
> + */
> +void (*begin_list) (struct sprinter *);
> +
> +/* end the last opened list or map structure
> + */
> +void (*end) (struct sprinter *);
> +
> +/* print one string/integer/boolean/null element (possibly inside a
> + * list or map
> + */

I think this should say that it also prints any separator necessary.

> +void (*string) (struct sprinter *, const char *);
> +void (*integer) (struct sprinter *, int);
> +void (*boolean) (struct sprinter *, notmuch_bool_t);
> +void (*null) (struct sprinter *);
> +
> +/* print the key of a map's key/value pair.
> + */
> +void (*map_key) (struct sprinter *, const char *);
> +
> +/* print a frame delimiter (such as a newline or extra whitespace)
> + */
> +void (*frame) (struct sprinter *);

I think this should say that the intention is that this only prints
non-syntactic stuff.

Best wishes

Mark


> +} sprinter_t;
> +
> +/* Create a new structure printer that emits JSON */
> +struct sprinter *
> +sprinter_json_new(const void *ctx, FILE *stream);
> +
> +/* A dummy structure printer that signifies that standard text output is
> + * to be used instead of any structured format.
> + */
> +struct sprinter *
> +sprinter_text;
> +
> +#endif // NOTMUCH_SPRINTER_H
> -- 
> 1.7.11.1
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


notmuch-emacs and bbdb

2012-07-12 Thread Daniel Bergey
I hacked together the attached elisp yesterday.  It provides bindings to
put sender or recipients into bbdb.  (Recipients part needs more
testing.)  It also colors the from line green if the sender is in bbdb,
or orange otherwise.  When it's been through a bit more testing, I'll
submit at least the first part as a patch.

I prefer not to autocapture everything into bbdb, and for the same
reason, I don't want to use notmuch itself as my contacts DB.  Mostly,
this is because I read lots of email on lists, sent by people I'm
unlikely to write back to.  I don't like every John I've heard from
coming up on autocomplete.  Other reasons include sync to phone and
adding contact information from channels other than email.

bergey

-- next part --
A non-text attachment was scrubbed...
Name: bbdb-notmuch.el
Type: application/emacs-lisp
Size: 2689 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20120712/37beab9d/attachment-0001.bin>
-- next part --


Jameson Graef Rollins  writes:

> On Tue, Jul 10 2012, Daniel Bergey  wrote:
>> As far as I can tell, notmuch doesn't integrate as smoothly with bbdb as 
>> older
>> emacs mailclients.  I'm especially looking for a snarf function that
>> distinguishes sender from recipient.
>>
>> How do other people use bbdb with notmuch?
>>
>> Does anyone have lisp code like that which ships with bbdb for other
>> clients?
>>
>> If I were to find time to write such code, what would you like it to do?
>
> Hey, Bergey.  This is something that I think needs improvement as well.
> I've been just manually constructing the bbdb entries myself.
>
> But what I've really been meaning to get going is address
> auto-completion from the database, which I'm pretty sure could obviate
> my need for bbdb altogether:
>
> http://notmuchmail.org/emacstips/#index13h2
>
> Getting it working seems more difficult than it should be, though, and
> the existing solutions seem a bit slower than they need to be [0].  So I
> think there's also room for improvement here.
>
> For instance, I think it would be rad if notmuch provided this
> functionality natively, in the CLI, or even in the library [1].  I think
> it's definitely doable, and it would be a nice little project.
>
> The emacs integration could be a bit smoother as well.  A single config
> option should either turn the functionality on or off.  That would be
> very convenient [2].
>
> jamie.
>
> [0] id:"87r4xur3rv.fsf at plc.plecavalier.com"
> [1] For what it's worth, I would prefer a solution that didn't involve
> any caching of addresses outside of the database.
> [2] I also find it changes the behavior of the ido tab completion
> interface that I'm used to using in message mode.


[PATCH v4 3/3] Use the structured format printer for JSON in notmuch search.

2012-07-12 Thread cra...@gmx.net
This patch switches from the current ad-hoc printer to the structured
output formatter in sprinter.h.

It removes search_format_t, replaces it by sprinter_t and inlines the
text printer where necessary.

The tests are changed (only whitespaces and regular expressions) in
order to make them PASS for the new structured output formatter.
---
 notmuch-search.c   | 349 ++---
 test/json  |  20 +--
 test/search-output | 270 +
 3 files changed, 288 insertions(+), 351 deletions(-)

diff --git a/notmuch-search.c b/notmuch-search.c
index 3be296d..b853f5f 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -19,6 +19,7 @@
  */

 #include "notmuch-client.h"
+#include "sprinter.h"

 typedef enum {
 OUTPUT_SUMMARY,
@@ -28,91 +29,9 @@ typedef enum {
 OUTPUT_TAGS
 } output_t;

-typedef struct search_format {
-const char *results_start;
-const char *item_start;
-void (*item_id) (const void *ctx,
-const char *item_type,
-const char *item_id);
-void (*thread_summary) (const void *ctx,
-   const char *thread_id,
-   const time_t date,
-   const int matched,
-   const int total,
-   const char *authors,
-   const char *subject);
-const char *tag_start;
-const char *tag;
-const char *tag_sep;
-const char *tag_end;
-const char *item_sep;
-const char *item_end;
-const char *results_end;
-const char *results_null;
-} search_format_t;
-
-static void
-format_item_id_text (const void *ctx,
-const char *item_type,
-const char *item_id);
-
-static void
-format_thread_text (const void *ctx,
-   const char *thread_id,
-   const time_t date,
-   const int matched,
-   const int total,
-   const char *authors,
-   const char *subject);
-static const search_format_t format_text = {
-"",
-   "",
-   format_item_id_text,
-   format_thread_text,
-   " (",
-   "%s", " ",
-   ")", "\n",
-   "",
-"\n",
-"",
-};
-
-static void
-format_item_id_json (const void *ctx,
-const char *item_type,
-const char *item_id);
-
-static void
-format_thread_json (const void *ctx,
-   const char *thread_id,
-   const time_t date,
-   const int matched,
-   const int total,
-   const char *authors,
-   const char *subject);
-
-/* Any changes to the JSON format should be reflected in the file
- * devel/schemata. */
-static const search_format_t format_json = {
-"[",
-   "{",
-   format_item_id_json,
-   format_thread_json,
-   "\"tags\": [",
-   "\"%s\"", ", ",
-   "]", ",\n",
-   "}",
-"]\n",
-"]\n",
-};
-
-static void
-format_item_id_text (unused (const void *ctx),
-const char *item_type,
-const char *item_id)
-{
-printf ("%s%s", item_type, item_id);
-}
+static const char * text_item_sep = "\n";
+static const char * text_results_null = "";
+static const char * text_results_end = "\n";

 static char *
 sanitize_string (const void *ctx, const char *str)
@@ -131,72 +50,8 @@ sanitize_string (const void *ctx, const char *str)
 return out;
 }

-static void
-format_thread_text (const void *ctx,
-   const char *thread_id,
-   const time_t date,
-   const int matched,
-   const int total,
-   const char *authors,
-   const char *subject)
-{
-void *ctx_quote = talloc_new (ctx);
-
-printf ("thread:%s %12s [%d/%d] %s; %s",
-   thread_id,
-   notmuch_time_relative_date (ctx, date),
-   matched,
-   total,
-   sanitize_string (ctx_quote, authors),
-   sanitize_string (ctx_quote, subject));
-
-talloc_free (ctx_quote);
-}
-
-static void
-format_item_id_json (const void *ctx,
-unused (const char *item_type),
-const char *item_id)
-{
-void *ctx_quote = talloc_new (ctx);
-
-printf ("%s", json_quote_str (ctx_quote, item_id));
-
-talloc_free (ctx_quote);
-
-}
-
-static void
-format_thread_json (const void *ctx,
-   const char *thread_id,
-   const time_t date,
-   const int matched,
-   const int total,
-   const char *authors,
-   const char *subject)
-{
-void *ctx_quote = talloc_new (ctx);
-
-printf ("\"thread\": %s,\n"
-   "\"timestamp\": %ld,\n"
-   "\"date_relative\": \"%s\",\n"
-

[PATCH v4 2/3] Add structured output formatter for JSON.

2012-07-12 Thread cra...@gmx.net
Using the new structured printer support in sprinter.h, implement
sprinter_json_new, which returns a new JSON structured output
formatter.

The formatter prints output similar to the existing JSON, but with
differences in whitespace (mostly newlines).
---
 Makefile.local |   1 +
 sprinter.c | 172 +
 2 files changed, 173 insertions(+)
 create mode 100644 sprinter.c

diff --git a/Makefile.local b/Makefile.local
index a890df2..8baf0c2 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -290,6 +290,7 @@ notmuch_client_srcs =   \
notmuch-show.c  \
notmuch-tag.c   \
notmuch-time.c  \
+   sprinter.c  \
query-string.c  \
mime-node.c \
crypto.c\
diff --git a/sprinter.c b/sprinter.c
new file mode 100644
index 000..649f79a
--- /dev/null
+++ b/sprinter.c
@@ -0,0 +1,172 @@
+#include 
+#include 
+#include 
+#include "sprinter.h"
+
+#define ARRAY_SIZE(arr) (sizeof (arr) / sizeof (arr[0]))
+
+struct sprinter *
+sprinter_text = NULL;
+
+/*
+ * Every below here is the implementation of the JSON printer.
+ */
+
+struct sprinter_json
+{
+struct sprinter vtable;
+FILE *stream;
+/* Top of the state stack, or NULL if the printer is not currently
+ * inside any aggregate types. */
+struct json_state *state;
+};
+
+struct json_state
+{
+struct json_state *parent;
+/* True if nothing has been printed in this aggregate yet.
+ * Suppresses the comma before a value. */
+notmuch_bool_t first;
+/* The character that closes the current aggregate. */
+char close;
+};
+
+/* Helper function to set up the stream to print a value.  If this
+ * value follows another value, prints a comma. */
+static struct sprinter_json *
+json_begin_value(struct sprinter *sp)
+{
+struct sprinter_json *spj = (struct sprinter_json*)sp;
+if (spj->state) {
+   if (!spj->state->first)
+   fputs (", ", spj->stream);
+   else
+   spj->state->first = false;
+}
+return spj;
+}
+
+/* Helper function to begin an aggregate type.  Prints the open
+ * character and pushes a new state frame. */
+static void
+json_begin_aggregate(struct sprinter *sp, char open, char close)
+{
+struct sprinter_json *spj = json_begin_value (sp);
+struct json_state *state = talloc (spj, struct json_state);
+
+fputc (open, spj->stream);
+state->parent = spj->state;
+state->first = true;
+state->close = close;
+spj->state = state;
+}
+
+static void
+json_begin_map(struct sprinter *sp)
+{
+json_begin_aggregate (sp, '{', '}');
+}
+
+static void
+json_begin_list(struct sprinter *sp)
+{
+json_begin_aggregate (sp, '[', ']');
+}
+
+static void
+json_end(struct sprinter *sp)
+{
+struct sprinter_json *spj = (struct sprinter_json*)sp;
+struct json_state *state = spj->state;
+
+fputc (spj->state->close, spj->stream);
+spj->state = state->parent;
+talloc_free (state);
+if(spj->state == NULL)
+   fputc ('\n', spj->stream);
+}
+
+static void
+json_string(struct sprinter *sp, const char *val)
+{
+static const char * const escapes[] = {
+   ['\"'] = "\\\"", ['\\'] = "", ['\b'] = "\\b",
+   ['\f'] = "\\f",  ['\n'] = "\\n",  ['\t'] = "\\t"
+};
+struct sprinter_json *spj = json_begin_value (sp);
+fputc ('"', spj->stream);
+for (; *val; ++val) {
+   unsigned char ch = *val;
+   if (ch < ARRAY_SIZE(escapes) && escapes[ch])
+   fputs (escapes[ch], spj->stream);
+   else if (ch >= 32)
+   fputc (ch, spj->stream);
+   else
+   fprintf (spj->stream, "\\u%04x", ch);
+}
+fputc ('"', spj->stream);
+}
+
+static void
+json_integer(struct sprinter *sp, int val)
+{
+struct sprinter_json *spj = json_begin_value (sp);
+fprintf (spj->stream, "%d", val);
+}
+
+static void
+json_boolean(struct sprinter *sp, notmuch_bool_t val)
+{
+struct sprinter_json *spj = json_begin_value (sp);
+fputs (val ? "true" : "false", spj->stream);
+}
+
+static void
+json_null(struct sprinter *sp)
+{
+struct sprinter_json *spj = json_begin_value (sp);
+fputs ("null", spj->stream);
+}
+
+static void
+json_map_key(struct sprinter *sp, const char *key)
+{
+struct sprinter_json *spj = (struct sprinter_json*)sp;
+json_string (sp, key);
+fputs (": ", spj->stream);
+spj->state->first = true;
+}
+
+static void
+json_frame(struct sprinter *sp)
+{
+struct sprinter_json *spj = (struct sprinter_json*)sp;
+fputc ('\n', spj->stream);
+}
+
+struct sprinter *
+sprinter_json_new(const void *ctx, FILE *stream)
+{
+static const struct sprinter_json template = {
+   .vtable = {
+   .begin_map = json_begin_map,
+   .begin_list = json_begin_list,
+   .end = json_end,
+   .string = json_string,
+   .integer = json_integer,
+   .boolean = json_boolean,
+  

[PATCH v4 1/3] Add support for structured output formatters.

2012-07-12 Thread cra...@gmx.net
This patch adds a new type sprinter_t, which is used for structured
formatting, e.g. JSON or S-Expressions. The structure printer is the
code from Austin Clements (id:87d34hsdx8.fsf at awakening.csail.mit.edu).

The structure printer contains the following function pointers:

/* start a new map/dictionary structure.
 */
void (*begin_map) (struct sprinter *);

/* start a new list/array structure
 */
void (*begin_list) (struct sprinter *);

/* end the last opened list or map structure
 */
void (*end) (struct sprinter *);

/* print one string/integer/boolean/null element (possibly inside a
 * list or map
 */
void (*string) (struct sprinter *, const char *);
void (*integer) (struct sprinter *, int);
void (*boolean) (struct sprinter *, notmuch_bool_t);
void (*null) (struct sprinter *);

/* print the key of a map's key/value pair.
 */
void (*map_key) (struct sprinter *, const char *);

/* print a frame delimiter (such as a newline or extra whitespace)
 */
void (*frame) (struct sprinter *);

The printer can (and should) use internal state to insert delimiters and
syntax at the correct places.

Example:

format->begin_map(format);
format->map_key(format, "foo");
format->begin_list(format);
format->integer(format, 1);
format->integer(format, 2);
format->integer(format, 3);
format->end(format);
format->map_key(format, "bar");
format->begin_map(format);
format->map_key(format, "baaz");
format->string(format, "hello world");
format->end(format);
format->end(format);

would output JSON as follows:

{"foo": [1, 2, 3], "bar": { "baaz": "hello world"}}
---
 sprinter.h | 49 +
 1 file changed, 49 insertions(+)
 create mode 100644 sprinter.h

diff --git a/sprinter.h b/sprinter.h
new file mode 100644
index 000..1dad9a0
--- /dev/null
+++ b/sprinter.h
@@ -0,0 +1,49 @@
+#ifndef NOTMUCH_SPRINTER_H
+#define NOTMUCH_SPRINTER_H
+
+/* for notmuch_bool_t */
+#include "notmuch-client.h"
+
+/* Structure printer interface */
+typedef struct sprinter
+{
+/* start a new map/dictionary structure.
+ */
+void (*begin_map) (struct sprinter *);
+
+/* start a new list/array structure
+ */
+void (*begin_list) (struct sprinter *);
+
+/* end the last opened list or map structure
+ */
+void (*end) (struct sprinter *);
+
+/* print one string/integer/boolean/null element (possibly inside a
+ * list or map
+ */
+void (*string) (struct sprinter *, const char *);
+void (*integer) (struct sprinter *, int);
+void (*boolean) (struct sprinter *, notmuch_bool_t);
+void (*null) (struct sprinter *);
+
+/* print the key of a map's key/value pair.
+ */
+void (*map_key) (struct sprinter *, const char *);
+
+/* print a frame delimiter (such as a newline or extra whitespace)
+ */
+void (*frame) (struct sprinter *);
+} sprinter_t;
+
+/* Create a new structure printer that emits JSON */
+struct sprinter *
+sprinter_json_new(const void *ctx, FILE *stream);
+
+/* A dummy structure printer that signifies that standard text output is
+ * to be used instead of any structured format.
+ */
+struct sprinter *
+sprinter_text;
+
+#endif // NOTMUCH_SPRINTER_H
-- 
1.7.11.1



Structured Formatters for JSON Output

2012-07-12 Thread cra...@gmx.net
Currently there is no easy way to add support for different structured
formatters (like JSON). For example, adding support for S-Expressions
would result in code duplication.

This patch series amends the situation by introducing structured
formatters, which allow different implementations of structured output
for elements like lists, maps, strings and numbers.

The new code in sprinter.h and sprinter.c can be used
instead of the current ad-hoc output in all parts of notmuch, a patch
for notmuch-search.c is included.

In a later patch, all other parts of notmuch should be adapted to the
structured formatters, and the creation of formatters should be
centralised (to make adding new formatters easier).



notmuch-emacs and bbdb

2012-07-12 Thread Jameson Graef Rollins
On Thu, Jul 12 2012, Daniel Bergey  wrote:
> I hacked together the attached elisp yesterday.  It provides bindings to
> put sender or recipients into bbdb.  (Recipients part needs more
> testing.)  It also colors the from line green if the sender is in bbdb,
> or orange otherwise.  When it's been through a bit more testing, I'll
> submit at least the first part as a patch.

Hey, Daniel.  This is really nice!  All I had to do was load it in my
emacs config after loading notmuch:

(require 'notmuch)
(load-file "~/.notmuch/bbdb-notmuch.el")

It works very well: snarfs contacts like a charm, and, I must say, the
coloring of names depending on their status in bbdb is really hot!  Nice
feature!

Thanks so much for this contribution.  I would be thrilled if this made
it into upstream, maybe with a config to let people turn the sender
highlighting on or off.  Maybe in the mean time we can put it in
contrib?

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


notmuch-emacs and bbdb

2012-07-12 Thread Jameson Graef Rollins
On Tue, Jul 10 2012, Daniel Bergey  wrote:
> As far as I can tell, notmuch doesn't integrate as smoothly with bbdb as older
> emacs mailclients.  I'm especially looking for a snarf function that
> distinguishes sender from recipient.
>
> How do other people use bbdb with notmuch?
>
> Does anyone have lisp code like that which ships with bbdb for other
> clients?
>
> If I were to find time to write such code, what would you like it to do?

Hey, Bergey.  This is something that I think needs improvement as well.
I've been just manually constructing the bbdb entries myself.

But what I've really been meaning to get going is address
auto-completion from the database, which I'm pretty sure could obviate
my need for bbdb altogether:

http://notmuchmail.org/emacstips/#index13h2

Getting it working seems more difficult than it should be, though, and
the existing solutions seem a bit slower than they need to be [0].  So I
think there's also room for improvement here.

For instance, I think it would be rad if notmuch provided this
functionality natively, in the CLI, or even in the library [1].  I think
it's definitely doable, and it would be a nice little project.

The emacs integration could be a bit smoother as well.  A single config
option should either turn the functionality on or off.  That would be
very convenient [2].

jamie.

[0] id:"87r4xur3rv.fsf at plc.plecavalier.com"
[1] For what it's worth, I would prefer a solution that didn't involve
any caching of addresses outside of the database.
[2] I also find it changes the behavior of the ido tab completion
interface that I'm used to using in message mode.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 



Re: notmuch-emacs and bbdb

2012-07-12 Thread Jameson Graef Rollins
On Tue, Jul 10 2012, Daniel Bergey ber...@alum.mit.edu wrote:
 As far as I can tell, notmuch doesn't integrate as smoothly with bbdb as older
 emacs mailclients.  I'm especially looking for a snarf function that
 distinguishes sender from recipient.

 How do other people use bbdb with notmuch?

 Does anyone have lisp code like that which ships with bbdb for other
 clients?

 If I were to find time to write such code, what would you like it to do?

Hey, Bergey.  This is something that I think needs improvement as well.
I've been just manually constructing the bbdb entries myself.

But what I've really been meaning to get going is address
auto-completion from the database, which I'm pretty sure could obviate
my need for bbdb altogether:

http://notmuchmail.org/emacstips/#index13h2

Getting it working seems more difficult than it should be, though, and
the existing solutions seem a bit slower than they need to be [0].  So I
think there's also room for improvement here.

For instance, I think it would be rad if notmuch provided this
functionality natively, in the CLI, or even in the library [1].  I think
it's definitely doable, and it would be a nice little project.

The emacs integration could be a bit smoother as well.  A single config
option should either turn the functionality on or off.  That would be
very convenient [2].

jamie.

[0] id:87r4xur3rv@plc.plecavalier.com
[1] For what it's worth, I would prefer a solution that didn't involve
any caching of addresses outside of the database.
[2] I also find it changes the behavior of the ido tab completion
interface that I'm used to using in message mode.


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


Structured Formatters for JSON Output

2012-07-12 Thread craven
Currently there is no easy way to add support for different structured
formatters (like JSON). For example, adding support for S-Expressions
would result in code duplication.

This patch series amends the situation by introducing structured
formatters, which allow different implementations of structured output
for elements like lists, maps, strings and numbers.

The new code in sprinter.h and sprinter.c can be used
instead of the current ad-hoc output in all parts of notmuch, a patch
for notmuch-search.c is included.

In a later patch, all other parts of notmuch should be adapted to the
structured formatters, and the creation of formatters should be
centralised (to make adding new formatters easier).

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


[PATCH v4 3/3] Use the structured format printer for JSON in notmuch search.

2012-07-12 Thread craven
This patch switches from the current ad-hoc printer to the structured
output formatter in sprinter.h.

It removes search_format_t, replaces it by sprinter_t and inlines the
text printer where necessary.

The tests are changed (only whitespaces and regular expressions) in
order to make them PASS for the new structured output formatter.
---
 notmuch-search.c   | 349 ++---
 test/json  |  20 +--
 test/search-output | 270 +
 3 files changed, 288 insertions(+), 351 deletions(-)

diff --git a/notmuch-search.c b/notmuch-search.c
index 3be296d..b853f5f 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -19,6 +19,7 @@
  */
 
 #include notmuch-client.h
+#include sprinter.h
 
 typedef enum {
 OUTPUT_SUMMARY,
@@ -28,91 +29,9 @@ typedef enum {
 OUTPUT_TAGS
 } output_t;
 
-typedef struct search_format {
-const char *results_start;
-const char *item_start;
-void (*item_id) (const void *ctx,
-const char *item_type,
-const char *item_id);
-void (*thread_summary) (const void *ctx,
-   const char *thread_id,
-   const time_t date,
-   const int matched,
-   const int total,
-   const char *authors,
-   const char *subject);
-const char *tag_start;
-const char *tag;
-const char *tag_sep;
-const char *tag_end;
-const char *item_sep;
-const char *item_end;
-const char *results_end;
-const char *results_null;
-} search_format_t;
-
-static void
-format_item_id_text (const void *ctx,
-const char *item_type,
-const char *item_id);
-
-static void
-format_thread_text (const void *ctx,
-   const char *thread_id,
-   const time_t date,
-   const int matched,
-   const int total,
-   const char *authors,
-   const char *subject);
-static const search_format_t format_text = {
-,
-   ,
-   format_item_id_text,
-   format_thread_text,
-(,
-   %s,  ,
-   ), \n,
-   ,
-\n,
-,
-};
-
-static void
-format_item_id_json (const void *ctx,
-const char *item_type,
-const char *item_id);
-
-static void
-format_thread_json (const void *ctx,
-   const char *thread_id,
-   const time_t date,
-   const int matched,
-   const int total,
-   const char *authors,
-   const char *subject);
-
-/* Any changes to the JSON format should be reflected in the file
- * devel/schemata. */
-static const search_format_t format_json = {
-[,
-   {,
-   format_item_id_json,
-   format_thread_json,
-   \tags\: [,
-   \%s\, , ,
-   ], ,\n,
-   },
-]\n,
-]\n,
-};
-
-static void
-format_item_id_text (unused (const void *ctx),
-const char *item_type,
-const char *item_id)
-{
-printf (%s%s, item_type, item_id);
-}
+static const char * text_item_sep = \n;
+static const char * text_results_null = ;
+static const char * text_results_end = \n;
 
 static char *
 sanitize_string (const void *ctx, const char *str)
@@ -131,72 +50,8 @@ sanitize_string (const void *ctx, const char *str)
 return out;
 }
 
-static void
-format_thread_text (const void *ctx,
-   const char *thread_id,
-   const time_t date,
-   const int matched,
-   const int total,
-   const char *authors,
-   const char *subject)
-{
-void *ctx_quote = talloc_new (ctx);
-
-printf (thread:%s %12s [%d/%d] %s; %s,
-   thread_id,
-   notmuch_time_relative_date (ctx, date),
-   matched,
-   total,
-   sanitize_string (ctx_quote, authors),
-   sanitize_string (ctx_quote, subject));
-
-talloc_free (ctx_quote);
-}
-
-static void
-format_item_id_json (const void *ctx,
-unused (const char *item_type),
-const char *item_id)
-{
-void *ctx_quote = talloc_new (ctx);
-
-printf (%s, json_quote_str (ctx_quote, item_id));
-
-talloc_free (ctx_quote);
-
-}
-
-static void
-format_thread_json (const void *ctx,
-   const char *thread_id,
-   const time_t date,
-   const int matched,
-   const int total,
-   const char *authors,
-   const char *subject)
-{
-void *ctx_quote = talloc_new (ctx);
-
-printf (\thread\: %s,\n
-   \timestamp\: %ld,\n
-   \date_relative\: \%s\,\n
-   \matched\: %d,\n
-   \total\: %d,\n
-   

[PATCH v4 2/3] Add structured output formatter for JSON.

2012-07-12 Thread craven
Using the new structured printer support in sprinter.h, implement
sprinter_json_new, which returns a new JSON structured output
formatter.

The formatter prints output similar to the existing JSON, but with
differences in whitespace (mostly newlines).
---
 Makefile.local |   1 +
 sprinter.c | 172 +
 2 files changed, 173 insertions(+)
 create mode 100644 sprinter.c

diff --git a/Makefile.local b/Makefile.local
index a890df2..8baf0c2 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -290,6 +290,7 @@ notmuch_client_srcs =   \
notmuch-show.c  \
notmuch-tag.c   \
notmuch-time.c  \
+   sprinter.c  \
query-string.c  \
mime-node.c \
crypto.c\
diff --git a/sprinter.c b/sprinter.c
new file mode 100644
index 000..649f79a
--- /dev/null
+++ b/sprinter.c
@@ -0,0 +1,172 @@
+#include stdbool.h
+#include stdio.h
+#include talloc.h
+#include sprinter.h
+
+#define ARRAY_SIZE(arr) (sizeof (arr) / sizeof (arr[0]))
+
+struct sprinter *
+sprinter_text = NULL;
+
+/*
+ * Every below here is the implementation of the JSON printer.
+ */
+
+struct sprinter_json
+{
+struct sprinter vtable;
+FILE *stream;
+/* Top of the state stack, or NULL if the printer is not currently
+ * inside any aggregate types. */
+struct json_state *state;
+};
+
+struct json_state
+{
+struct json_state *parent;
+/* True if nothing has been printed in this aggregate yet.
+ * Suppresses the comma before a value. */
+notmuch_bool_t first;
+/* The character that closes the current aggregate. */
+char close;
+};
+
+/* Helper function to set up the stream to print a value.  If this
+ * value follows another value, prints a comma. */
+static struct sprinter_json *
+json_begin_value(struct sprinter *sp)
+{
+struct sprinter_json *spj = (struct sprinter_json*)sp;
+if (spj-state) {
+   if (!spj-state-first)
+   fputs (, , spj-stream);
+   else
+   spj-state-first = false;
+}
+return spj;
+}
+
+/* Helper function to begin an aggregate type.  Prints the open
+ * character and pushes a new state frame. */
+static void
+json_begin_aggregate(struct sprinter *sp, char open, char close)
+{
+struct sprinter_json *spj = json_begin_value (sp);
+struct json_state *state = talloc (spj, struct json_state);
+
+fputc (open, spj-stream);
+state-parent = spj-state;
+state-first = true;
+state-close = close;
+spj-state = state;
+}
+
+static void
+json_begin_map(struct sprinter *sp)
+{
+json_begin_aggregate (sp, '{', '}');
+}
+
+static void
+json_begin_list(struct sprinter *sp)
+{
+json_begin_aggregate (sp, '[', ']');
+}
+
+static void
+json_end(struct sprinter *sp)
+{
+struct sprinter_json *spj = (struct sprinter_json*)sp;
+struct json_state *state = spj-state;
+
+fputc (spj-state-close, spj-stream);
+spj-state = state-parent;
+talloc_free (state);
+if(spj-state == NULL)
+   fputc ('\n', spj-stream);
+}
+
+static void
+json_string(struct sprinter *sp, const char *val)
+{
+static const char * const escapes[] = {
+   ['\'] = \\\, ['\\'] = , ['\b'] = \\b,
+   ['\f'] = \\f,  ['\n'] = \\n,  ['\t'] = \\t
+};
+struct sprinter_json *spj = json_begin_value (sp);
+fputc ('', spj-stream);
+for (; *val; ++val) {
+   unsigned char ch = *val;
+   if (ch  ARRAY_SIZE(escapes)  escapes[ch])
+   fputs (escapes[ch], spj-stream);
+   else if (ch = 32)
+   fputc (ch, spj-stream);
+   else
+   fprintf (spj-stream, \\u%04x, ch);
+}
+fputc ('', spj-stream);
+}
+
+static void
+json_integer(struct sprinter *sp, int val)
+{
+struct sprinter_json *spj = json_begin_value (sp);
+fprintf (spj-stream, %d, val);
+}
+
+static void
+json_boolean(struct sprinter *sp, notmuch_bool_t val)
+{
+struct sprinter_json *spj = json_begin_value (sp);
+fputs (val ? true : false, spj-stream);
+}
+
+static void
+json_null(struct sprinter *sp)
+{
+struct sprinter_json *spj = json_begin_value (sp);
+fputs (null, spj-stream);
+}
+
+static void
+json_map_key(struct sprinter *sp, const char *key)
+{
+struct sprinter_json *spj = (struct sprinter_json*)sp;
+json_string (sp, key);
+fputs (: , spj-stream);
+spj-state-first = true;
+}
+
+static void
+json_frame(struct sprinter *sp)
+{
+struct sprinter_json *spj = (struct sprinter_json*)sp;
+fputc ('\n', spj-stream);
+}
+
+struct sprinter *
+sprinter_json_new(const void *ctx, FILE *stream)
+{
+static const struct sprinter_json template = {
+   .vtable = {
+   .begin_map = json_begin_map,
+   .begin_list = json_begin_list,
+   .end = json_end,
+   .string = json_string,
+   .integer = json_integer,
+   .boolean = json_boolean,
+   .null = json_null,
+   

[PATCH v4 1/3] Add support for structured output formatters.

2012-07-12 Thread craven
This patch adds a new type sprinter_t, which is used for structured
formatting, e.g. JSON or S-Expressions. The structure printer is the
code from Austin Clements (id:87d34hsdx8@awakening.csail.mit.edu).

The structure printer contains the following function pointers:

/* start a new map/dictionary structure.
 */
void (*begin_map) (struct sprinter *);

/* start a new list/array structure
 */
void (*begin_list) (struct sprinter *);

/* end the last opened list or map structure
 */
void (*end) (struct sprinter *);

/* print one string/integer/boolean/null element (possibly inside a
 * list or map
 */
void (*string) (struct sprinter *, const char *);
void (*integer) (struct sprinter *, int);
void (*boolean) (struct sprinter *, notmuch_bool_t);
void (*null) (struct sprinter *);

/* print the key of a map's key/value pair.
 */
void (*map_key) (struct sprinter *, const char *);

/* print a frame delimiter (such as a newline or extra whitespace)
 */
void (*frame) (struct sprinter *);

The printer can (and should) use internal state to insert delimiters and
syntax at the correct places.

Example:

format-begin_map(format);
format-map_key(format, foo);
format-begin_list(format);
format-integer(format, 1);
format-integer(format, 2);
format-integer(format, 3);
format-end(format);
format-map_key(format, bar);
format-begin_map(format);
format-map_key(format, baaz);
format-string(format, hello world);
format-end(format);
format-end(format);

would output JSON as follows:

{foo: [1, 2, 3], bar: { baaz: hello world}}
---
 sprinter.h | 49 +
 1 file changed, 49 insertions(+)
 create mode 100644 sprinter.h

diff --git a/sprinter.h b/sprinter.h
new file mode 100644
index 000..1dad9a0
--- /dev/null
+++ b/sprinter.h
@@ -0,0 +1,49 @@
+#ifndef NOTMUCH_SPRINTER_H
+#define NOTMUCH_SPRINTER_H
+
+/* for notmuch_bool_t */
+#include notmuch-client.h
+
+/* Structure printer interface */
+typedef struct sprinter
+{
+/* start a new map/dictionary structure.
+ */
+void (*begin_map) (struct sprinter *);
+
+/* start a new list/array structure
+ */
+void (*begin_list) (struct sprinter *);
+
+/* end the last opened list or map structure
+ */
+void (*end) (struct sprinter *);
+
+/* print one string/integer/boolean/null element (possibly inside a
+ * list or map
+ */
+void (*string) (struct sprinter *, const char *);
+void (*integer) (struct sprinter *, int);
+void (*boolean) (struct sprinter *, notmuch_bool_t);
+void (*null) (struct sprinter *);
+
+/* print the key of a map's key/value pair.
+ */
+void (*map_key) (struct sprinter *, const char *);
+
+/* print a frame delimiter (such as a newline or extra whitespace)
+ */
+void (*frame) (struct sprinter *);
+} sprinter_t;
+
+/* Create a new structure printer that emits JSON */
+struct sprinter *
+sprinter_json_new(const void *ctx, FILE *stream);
+
+/* A dummy structure printer that signifies that standard text output is
+ * to be used instead of any structured format.
+ */
+struct sprinter *
+sprinter_text;
+
+#endif // NOTMUCH_SPRINTER_H
-- 
1.7.11.1

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


Re: [PATCH v4 1/3] Add support for structured output formatters.

2012-07-12 Thread Mark Walters

Hi

this definitely looks nice. I have a couple of suggestions for the
comments:

On Thu, 12 Jul 2012, cra...@gmx.net wrote:
 This patch adds a new type sprinter_t, which is used for structured
 formatting, e.g. JSON or S-Expressions. The structure printer is the
 code from Austin Clements (id:87d34hsdx8@awakening.csail.mit.edu).

 The structure printer contains the following function pointers:

 /* start a new map/dictionary structure.
  */
 void (*begin_map) (struct sprinter *);

 /* start a new list/array structure
  */
 void (*begin_list) (struct sprinter *);

 /* end the last opened list or map structure
  */
 void (*end) (struct sprinter *);

 /* print one string/integer/boolean/null element (possibly inside a
  * list or map
  */
 void (*string) (struct sprinter *, const char *);
 void (*integer) (struct sprinter *, int);
 void (*boolean) (struct sprinter *, notmuch_bool_t);
 void (*null) (struct sprinter *);

 /* print the key of a map's key/value pair.
  */
 void (*map_key) (struct sprinter *, const char *);

 /* print a frame delimiter (such as a newline or extra whitespace)
  */
 void (*frame) (struct sprinter *);

 The printer can (and should) use internal state to insert delimiters and
 syntax at the correct places.

 Example:

 format-begin_map(format);
 format-map_key(format, foo);
 format-begin_list(format);
 format-integer(format, 1);
 format-integer(format, 2);
 format-integer(format, 3);
 format-end(format);
 format-map_key(format, bar);
 format-begin_map(format);
 format-map_key(format, baaz);
 format-string(format, hello world);
 format-end(format);
 format-end(format);

 would output JSON as follows:

 {foo: [1, 2, 3], bar: { baaz: hello world}}
 ---
  sprinter.h | 49 +
  1 file changed, 49 insertions(+)
  create mode 100644 sprinter.h

 diff --git a/sprinter.h b/sprinter.h
 new file mode 100644
 index 000..1dad9a0
 --- /dev/null
 +++ b/sprinter.h
 @@ -0,0 +1,49 @@
 +#ifndef NOTMUCH_SPRINTER_H
 +#define NOTMUCH_SPRINTER_H
 +
 +/* for notmuch_bool_t */
 +#include notmuch-client.h
 +
 +/* Structure printer interface */
 +typedef struct sprinter
 +{
 +/* start a new map/dictionary structure.
 + */
 +void (*begin_map) (struct sprinter *);
 +
 +/* start a new list/array structure
 + */
 +void (*begin_list) (struct sprinter *);
 +
 +/* end the last opened list or map structure
 + */
 +void (*end) (struct sprinter *);
 +
 +/* print one string/integer/boolean/null element (possibly inside a
 + * list or map
 + */

I think this should say that it also prints any separator necessary.

 +void (*string) (struct sprinter *, const char *);
 +void (*integer) (struct sprinter *, int);
 +void (*boolean) (struct sprinter *, notmuch_bool_t);
 +void (*null) (struct sprinter *);
 +
 +/* print the key of a map's key/value pair.
 + */
 +void (*map_key) (struct sprinter *, const char *);
 +
 +/* print a frame delimiter (such as a newline or extra whitespace)
 + */
 +void (*frame) (struct sprinter *);

I think this should say that the intention is that this only prints
non-syntactic stuff.

Best wishes

Mark


 +} sprinter_t;
 +
 +/* Create a new structure printer that emits JSON */
 +struct sprinter *
 +sprinter_json_new(const void *ctx, FILE *stream);
 +
 +/* A dummy structure printer that signifies that standard text output is
 + * to be used instead of any structured format.
 + */
 +struct sprinter *
 +sprinter_text;
 +
 +#endif // NOTMUCH_SPRINTER_H
 -- 
 1.7.11.1

 ___
 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 v4 1/3] Add support for structured output formatters.

2012-07-12 Thread Tomi Ollila
On Thu, Jul 12 2012, cra...@gmx.net wrote:

 This patch adds a new type sprinter_t, which is used for structured
 formatting, e.g. JSON or S-Expressions. The structure printer is the
 code from Austin Clements (id:87d34hsdx8@awakening.csail.mit.edu).

...

 +
 +/* Create a new structure printer that emits JSON */
 +struct sprinter *
 +sprinter_json_new(const void *ctx, FILE *stream);
 +
 +/* A dummy structure printer that signifies that standard text output is
 + * to be used instead of any structured format.
 + */
 +struct sprinter *
 +sprinter_text;

Looks good... but this needs to be 'extern struct sprinter * sprinter_text;'

see id:1340563714-3103-1-git-send-email-tomi.oll...@iki.fi
for reference.


 +
 +#endif // NOTMUCH_SPRINTER_H
 -- 
 1.7.11.1

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


Re: [PATCH v4 2/3] Add structured output formatter for JSON.

2012-07-12 Thread Tomi Ollila
On Thu, Jul 12 2012, cra...@gmx.net wrote:

 Using the new structured printer support in sprinter.h, implement
 sprinter_json_new, which returns a new JSON structured output
 formatter.

 The formatter prints output similar to the existing JSON, but with
 differences in whitespace (mostly newlines).
 ---
  Makefile.local |   1 +
  sprinter.c | 172 
 +
  2 files changed, 173 insertions(+)
  create mode 100644 sprinter.c

 diff --git a/Makefile.local b/Makefile.local
 index a890df2..8baf0c2 100644
 --- a/Makefile.local
 +++ b/Makefile.local
 @@ -290,6 +290,7 @@ notmuch_client_srcs = \
   notmuch-show.c  \
   notmuch-tag.c   \
   notmuch-time.c  \
 + sprinter.c  \
   query-string.c  \
   mime-node.c \
   crypto.c\
 diff --git a/sprinter.c b/sprinter.c
 new file mode 100644
 index 000..649f79a
 --- /dev/null
 +++ b/sprinter.c
 @@ -0,0 +1,172 @@
 +#include stdbool.h
 +#include stdio.h
 +#include talloc.h
 +#include sprinter.h
 +
 +#define ARRAY_SIZE(arr) (sizeof (arr) / sizeof (arr[0]))

You're including sprinter.h which includes notmuch-client.h which
defines ARRAY_SIZE (Interesting that you did not get error(/warning?)
about this)

Rest looks good -- except the whitespace -- as I looked through Austin's
code yesterday you're just replicated the same lines :D. Easiest
to fix is probably just running 
uncrustify -c devel/uncrustify.cfg --replace sprinter.c 

Tomi

 +
 +struct sprinter *
 +sprinter_text = NULL;
 +
 +/*
 + * Every below here is the implementation of the JSON printer.
 + */
 +
 +struct sprinter_json
 +{
 +struct sprinter vtable;
 +FILE *stream;
 +/* Top of the state stack, or NULL if the printer is not currently
 + * inside any aggregate types. */
 +struct json_state *state;
 +};
 +
 +struct json_state
 +{
 +struct json_state *parent;
 +/* True if nothing has been printed in this aggregate yet.
 + * Suppresses the comma before a value. */
 +notmuch_bool_t first;
 +/* The character that closes the current aggregate. */
 +char close;
 +};
 +
 +/* Helper function to set up the stream to print a value.  If this
 + * value follows another value, prints a comma. */
 +static struct sprinter_json *
 +json_begin_value(struct sprinter *sp)
 +{
 +struct sprinter_json *spj = (struct sprinter_json*)sp;
 +if (spj-state) {
 + if (!spj-state-first)
 + fputs (, , spj-stream);
 + else
 + spj-state-first = false;
 +}
 +return spj;
 +}
 +
 +/* Helper function to begin an aggregate type.  Prints the open
 + * character and pushes a new state frame. */
 +static void
 +json_begin_aggregate(struct sprinter *sp, char open, char close)
 +{
 +struct sprinter_json *spj = json_begin_value (sp);
 +struct json_state *state = talloc (spj, struct json_state);
 +
 +fputc (open, spj-stream);
 +state-parent = spj-state;
 +state-first = true;
 +state-close = close;
 +spj-state = state;
 +}
 +
 +static void
 +json_begin_map(struct sprinter *sp)
 +{
 +json_begin_aggregate (sp, '{', '}');
 +}
 +
 +static void
 +json_begin_list(struct sprinter *sp)
 +{
 +json_begin_aggregate (sp, '[', ']');
 +}
 +
 +static void
 +json_end(struct sprinter *sp)
 +{
 +struct sprinter_json *spj = (struct sprinter_json*)sp;
 +struct json_state *state = spj-state;
 +
 +fputc (spj-state-close, spj-stream);
 +spj-state = state-parent;
 +talloc_free (state);
 +if(spj-state == NULL)
 + fputc ('\n', spj-stream);
 +}
 +
 +static void
 +json_string(struct sprinter *sp, const char *val)
 +{
 +static const char * const escapes[] = {
 + ['\'] = \\\, ['\\'] = , ['\b'] = \\b,
 + ['\f'] = \\f,  ['\n'] = \\n,  ['\t'] = \\t
 +};
 +struct sprinter_json *spj = json_begin_value (sp);
 +fputc ('', spj-stream);
 +for (; *val; ++val) {
 + unsigned char ch = *val;
 + if (ch  ARRAY_SIZE(escapes)  escapes[ch])
 + fputs (escapes[ch], spj-stream);
 + else if (ch = 32)
 + fputc (ch, spj-stream);
 + else
 + fprintf (spj-stream, \\u%04x, ch);
 +}
 +fputc ('', spj-stream);
 +}
 +
 +static void
 +json_integer(struct sprinter *sp, int val)
 +{
 +struct sprinter_json *spj = json_begin_value (sp);
 +fprintf (spj-stream, %d, val);
 +}
 +
 +static void
 +json_boolean(struct sprinter *sp, notmuch_bool_t val)
 +{
 +struct sprinter_json *spj = json_begin_value (sp);
 +fputs (val ? true : false, spj-stream);
 +}
 +
 +static void
 +json_null(struct sprinter *sp)
 +{
 +struct sprinter_json *spj = json_begin_value (sp);
 +fputs (null, spj-stream);
 +}
 +
 +static void
 +json_map_key(struct sprinter *sp, const char *key)
 +{
 +struct sprinter_json *spj = (struct sprinter_json*)sp;
 +json_string (sp, key);
 +fputs (: , spj-stream);
 +spj-state-first = true;
 +}
 +
 

Re: [PATCH v4 1/3] Add support for structured output formatters.

2012-07-12 Thread craven
 what is the advantage of having this as one function rather than end_map
 and end_list? Indeed, my choice (but I think most other people would
 disagree) would be to have both functions but still keep state as this
 currently does and then throw an error if the code closes the wrong
 thing.

There's probably no advantage, one way or the other is fine, I'd say.
I've thought about introducing checks into the formatter functions, to
raise errors for improper closing, map_key not inside a map and things
like that, I just wasn't sure that would be acceptable.

 A second question: do you have an implementation in this style for
 s-expressions. I find it hard to tell whether the interface is right
 with just a single example. Even a completely hacky not ready for review
 example would be helpful.

See the attached patch :)

Thanks for the suggestions!

Peter
From cf2c5eeab814970736510ca2210b909643a8cf19 Mon Sep 17 00:00:00 2001
From: cra...@gmx.net
Date: Thu, 12 Jul 2012 10:17:05 +0200
Subject: [PATCH] Add an S-Expression output format.

---
 notmuch-search.c |   7 ++-
 sprinter.c   | 170 +++
 sprinter.h   |   4 ++
 3 files changed, 180 insertions(+), 1 deletion(-)

diff --git a/notmuch-search.c b/notmuch-search.c
index b853f5f..f8bea9b 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -77,6 +77,7 @@ do_search_threads (sprinter_t *format,
 
 if (format != sprinter_text) {
 	format-begin_list (format);
+	format-frame (format);
 }
 
 for (i = 0;
@@ -380,7 +381,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
 int exclude = EXCLUDE_TRUE;
 unsigned int i;
 
-enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT }
+enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT, NOTMUCH_FORMAT_SEXP }
 	format_sel = NOTMUCH_FORMAT_TEXT;
 
 notmuch_opt_desc_t options[] = {
@@ -391,6 +392,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
 	{ NOTMUCH_OPT_KEYWORD, format_sel, format, 'f',
 	  (notmuch_keyword_t []){ { json, NOTMUCH_FORMAT_JSON },
   { text, NOTMUCH_FORMAT_TEXT },
+  { sexp, NOTMUCH_FORMAT_SEXP },
   { 0, 0 } } },
 	{ NOTMUCH_OPT_KEYWORD, output, output, 'o',
 	  (notmuch_keyword_t []){ { summary, OUTPUT_SUMMARY },
@@ -422,6 +424,9 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
 case NOTMUCH_FORMAT_JSON:
 	format = sprinter_json_new (ctx, stdout);
 	break;
+case NOTMUCH_FORMAT_SEXP:
+	format = sprinter_sexp_new (ctx, stdout);
+break;
 }
 
 config = notmuch_config_open (ctx, NULL, NULL);
diff --git a/sprinter.c b/sprinter.c
index 649f79a..fce0f9b 100644
--- a/sprinter.c
+++ b/sprinter.c
@@ -170,3 +170,173 @@ sprinter_json_new(const void *ctx, FILE *stream)
 res-stream = stream;
 return res-vtable;
 }
+
+/*
+ * Every below here is the implementation of the SEXP printer.
+ */
+
+typedef enum { MAP, LIST } aggregate_t;
+
+struct sprinter_sexp
+{
+struct sprinter vtable;
+FILE *stream;
+/* Top of the state stack, or NULL if the printer is not currently
+ * inside any aggregate types. */
+struct sexp_state *state;
+};
+
+struct sexp_state
+{
+struct sexp_state *parent;
+/* True if nothing has been printed in this aggregate yet.
+ * Suppresses the comma before a value. */
+notmuch_bool_t first;
+/* The character that closes the current aggregate. */
+aggregate_t type;
+};
+
+static struct sprinter_sexp *
+sexp_begin_value(struct sprinter *sp)
+{
+struct sprinter_sexp *spsx = (struct sprinter_sexp*)sp;
+if (spsx-state) {
+	if (!spsx-state-first)
+	fputc (' ', spsx-stream);
+	else
+	spsx-state-first = false;
+}
+return spsx;
+}
+
+static void
+sexp_begin_aggregate(struct sprinter *sp, aggregate_t type)
+{
+struct sprinter_sexp *spsx = (struct sprinter_sexp*)sp;
+struct sexp_state *state = talloc (spsx, struct sexp_state);
+
+fputc ('(', spsx-stream);
+state-parent = spsx-state;
+state-first = true;
+spsx-state = state;
+state-type = type;
+}
+
+static void
+sexp_begin_map(struct sprinter *sp)
+{
+sexp_begin_aggregate (sp, MAP);
+}
+
+static void
+sexp_begin_list(struct sprinter *sp)
+{
+sexp_begin_aggregate (sp, LIST);
+}
+
+static void
+sexp_end(struct sprinter *sp)
+{
+struct sprinter_sexp *spsx = (struct sprinter_sexp*)sp;
+struct sexp_state *state = spsx-state;
+
+fputc (')', spsx-stream);
+spsx-state = state-parent;
+talloc_free (state);
+if(spsx-state == NULL)
+	fputc ('\n', spsx-stream);
+}
+
+static void
+sexp_string(struct sprinter *sp, const char *val)
+{
+static const char * const escapes[] = {
+	['\'] = \\\, ['\\'] = , ['\b'] = \\b,
+	['\f'] = \\f,  ['\n'] = \\n,  ['\t'] = \\t
+};
+struct sprinter_sexp *spsx = sexp_begin_value(sp);
+fputc ('', spsx-stream);
+for (; *val; ++val) {
+	unsigned char ch = *val;
+	if (ch  ARRAY_SIZE(escapes)  escapes[ch])
+	fputs (escapes[ch], spsx-stream);
+	

Re: [PATCH v4 1/3] Add support for structured output formatters.

2012-07-12 Thread Mark Walters
On Thu, 12 Jul 2012, cra...@gmx.net wrote:
 what is the advantage of having this as one function rather than end_map
 and end_list? Indeed, my choice (but I think most other people would
 disagree) would be to have both functions but still keep state as this
 currently does and then throw an error if the code closes the wrong
 thing.

 There's probably no advantage, one way or the other is fine, I'd say.
 I've thought about introducing checks into the formatter functions, to
 raise errors for improper closing, map_key not inside a map and things
 like that, I just wasn't sure that would be acceptable.

I will leave others to comment.

 A second question: do you have an implementation in this style for
 s-expressions. I find it hard to tell whether the interface is right
 with just a single example. Even a completely hacky not ready for review
 example would be helpful.

 See the attached patch :)

This looks great. I found it much easier to review by splitting
sprinter.c into two files sprinter-json.c and sprinter-sexp.c and then
running meld on them. The similarity is then very clear. It might be
worth submitting them as two files, but I leave other people to comment.

(Doing so made some of the difference between json and s-exp clear: like
that keys in mapkeys are quoted in json but not in s-exp)

It could be that some of the code could be merged, but I am not sure
that there is much advantage. I would imagine that these two sprinter.c
files would basically never change so there is not much risk of them
diverging.

I wonder if it would be worth using aggregate_t for both rather than
using the closing symbol for this purpose in the json output.

In any case this patch answers my query: the new structure does
generalise very easily to s-expressions!

Best wishes

Mark




 Thanks for the suggestions!

 Peter
 From cf2c5eeab814970736510ca2210b909643a8cf19 Mon Sep 17 00:00:00 2001
 From: cra...@gmx.net
 Date: Thu, 12 Jul 2012 10:17:05 +0200
 Subject: [PATCH] Add an S-Expression output format.

 ---
  notmuch-search.c |   7 ++-
  sprinter.c   | 170 
 +++
  sprinter.h   |   4 ++
  3 files changed, 180 insertions(+), 1 deletion(-)

 diff --git a/notmuch-search.c b/notmuch-search.c
 index b853f5f..f8bea9b 100644
 --- a/notmuch-search.c
 +++ b/notmuch-search.c
 @@ -77,6 +77,7 @@ do_search_threads (sprinter_t *format,
  
  if (format != sprinter_text) {
   format-begin_list (format);
 + format-frame (format);
  }
  
  for (i = 0;
 @@ -380,7 +381,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
  int exclude = EXCLUDE_TRUE;
  unsigned int i;
  
 -enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT }
 +enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT, NOTMUCH_FORMAT_SEXP }
   format_sel = NOTMUCH_FORMAT_TEXT;
  
  notmuch_opt_desc_t options[] = {
 @@ -391,6 +392,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
   { NOTMUCH_OPT_KEYWORD, format_sel, format, 'f',
 (notmuch_keyword_t []){ { json, NOTMUCH_FORMAT_JSON },
 { text, NOTMUCH_FORMAT_TEXT },
 +   { sexp, NOTMUCH_FORMAT_SEXP },
 { 0, 0 } } },
   { NOTMUCH_OPT_KEYWORD, output, output, 'o',
 (notmuch_keyword_t []){ { summary, OUTPUT_SUMMARY },
 @@ -422,6 +424,9 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
  case NOTMUCH_FORMAT_JSON:
   format = sprinter_json_new (ctx, stdout);
   break;
 +case NOTMUCH_FORMAT_SEXP:
 + format = sprinter_sexp_new (ctx, stdout);
 +break;
  }
  
  config = notmuch_config_open (ctx, NULL, NULL);
 diff --git a/sprinter.c b/sprinter.c
 index 649f79a..fce0f9b 100644
 --- a/sprinter.c
 +++ b/sprinter.c
 @@ -170,3 +170,173 @@ sprinter_json_new(const void *ctx, FILE *stream)
  res-stream = stream;
  return res-vtable;
  }
 +
 +/*
 + * Every below here is the implementation of the SEXP printer.
 + */
 +
 +typedef enum { MAP, LIST } aggregate_t;
 +
 +struct sprinter_sexp
 +{
 +struct sprinter vtable;
 +FILE *stream;
 +/* Top of the state stack, or NULL if the printer is not currently
 + * inside any aggregate types. */
 +struct sexp_state *state;
 +};
 +
 +struct sexp_state
 +{
 +struct sexp_state *parent;
 +/* True if nothing has been printed in this aggregate yet.
 + * Suppresses the comma before a value. */
 +notmuch_bool_t first;
 +/* The character that closes the current aggregate. */
 +aggregate_t type;
 +};
 +
 +static struct sprinter_sexp *
 +sexp_begin_value(struct sprinter *sp)
 +{
 +struct sprinter_sexp *spsx = (struct sprinter_sexp*)sp;
 +if (spsx-state) {
 + if (!spsx-state-first)
 + fputc (' ', spsx-stream);
 + else
 + spsx-state-first = false;
 +}
 +return spsx;
 +}
 +
 +static void
 +sexp_begin_aggregate(struct sprinter *sp, aggregate_t type)
 

Re: [PATCH v4 1/3] Add support for structured output formatters.

2012-07-12 Thread Tomi Ollila
On Thu, Jul 12 2012, Mark Walters markwalters1...@gmail.com wrote:

 On Thu, 12 Jul 2012, cra...@gmx.net wrote:
 what is the advantage of having this as one function rather than end_map
 and end_list? Indeed, my choice (but I think most other people would
 disagree) would be to have both functions but still keep state as this
 currently does and then throw an error if the code closes the wrong
 thing.

 There's probably no advantage, one way or the other is fine, I'd say.
 I've thought about introducing checks into the formatter functions, to
 raise errors for improper closing, map_key not inside a map and things
 like that, I just wasn't sure that would be acceptable.

 I will leave others to comment.

I like the current implementation -- record what has been opened and
have a common closing function which knows what it is closing. Less
checking in the code (i.e. less possible branches).
This makes the use of the interface a bit less self-documenting as
'end' is used instead of list/hash end (defining macros for this 
documentation purposes would be really dumb thing to do ;)

What needs to be checked is that software doesn't attempt to 'end'
too many contexts (i quess it's doing it already). Any other output
errors (like forgetting to 'end' some blocks should be taken care
by proper test cases).

 A second question: do you have an implementation in this style for
 s-expressions. I find it hard to tell whether the interface is right
 with just a single example. Even a completely hacky not ready for review
 example would be helpful.

 See the attached patch :)

 This looks great. I found it much easier to review by splitting
 sprinter.c into two files sprinter-json.c and sprinter-sexp.c and then
 running meld on them. The similarity is then very clear. It might be
 worth submitting them as two files, but I leave other people to comment.

I like that -- is there (currently) need for splinter.c for common code ?
(splinter.h is definitely needed).

 (Doing so made some of the difference between json and s-exp clear: like
 that keys in mapkeys are quoted in json but not in s-exp)

 It could be that some of the code could be merged, but I am not sure
 that there is much advantage. I would imagine that these two sprinter.c
 files would basically never change so there is not much risk of them
 diverging.

I was thinking the same when looking splinter code yesterday -- how to
have even more common code for jsonsexp. Maybe there could be something
to do just for these 2 purposes but It requires more effort and might
add more complexity for humans to perceive. ATM I'd go with this interface
and see later if anyone wants to do/experiment more -- as you said the
risk of diverging is minimal -- and in case there are separate source
files for json  sexp diffing those will be easy.


 I wonder if it would be worth using aggregate_t for both rather than
 using the closing symbol for this purpose in the json output.

 In any case this patch answers my query: the new structure does
 generalise very easily to s-expressions!

 Best wishes

 Mark

Tomi






 Thanks for the suggestions!

 Peter
 From cf2c5eeab814970736510ca2210b909643a8cf19 Mon Sep 17 00:00:00 2001
 From: cra...@gmx.net
 Date: Thu, 12 Jul 2012 10:17:05 +0200
 Subject: [PATCH] Add an S-Expression output format.

 ---
  notmuch-search.c |   7 ++-
  sprinter.c   | 170 
 +++
  sprinter.h   |   4 ++
  3 files changed, 180 insertions(+), 1 deletion(-)

 diff --git a/notmuch-search.c b/notmuch-search.c
 index b853f5f..f8bea9b 100644
 --- a/notmuch-search.c
 +++ b/notmuch-search.c
 @@ -77,6 +77,7 @@ do_search_threads (sprinter_t *format,
  
  if (format != sprinter_text) {
  format-begin_list (format);
 +format-frame (format);
  }
  
  for (i = 0;
 @@ -380,7 +381,7 @@ notmuch_search_command (void *ctx, int argc, char 
 *argv[])
  int exclude = EXCLUDE_TRUE;
  unsigned int i;
  
 -enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT }
 +enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT, NOTMUCH_FORMAT_SEXP }
  format_sel = NOTMUCH_FORMAT_TEXT;
  
  notmuch_opt_desc_t options[] = {
 @@ -391,6 +392,7 @@ notmuch_search_command (void *ctx, int argc, char 
 *argv[])
  { NOTMUCH_OPT_KEYWORD, format_sel, format, 'f',
(notmuch_keyword_t []){ { json, NOTMUCH_FORMAT_JSON },
{ text, NOTMUCH_FORMAT_TEXT },
 +  { sexp, NOTMUCH_FORMAT_SEXP },
{ 0, 0 } } },
  { NOTMUCH_OPT_KEYWORD, output, output, 'o',
(notmuch_keyword_t []){ { summary, OUTPUT_SUMMARY },
 @@ -422,6 +424,9 @@ notmuch_search_command (void *ctx, int argc, char 
 *argv[])
  case NOTMUCH_FORMAT_JSON:
  format = sprinter_json_new (ctx, stdout);
  break;
 +case NOTMUCH_FORMAT_SEXP:
 +format = sprinter_sexp_new (ctx, stdout);
 +break;
  }
  
  config = notmuch_config_open (ctx, 

Re: notmuch-emacs and bbdb

2012-07-12 Thread Jameson Graef Rollins
On Thu, Jul 12 2012, Daniel Bergey ber...@alum.mit.edu wrote:
 I hacked together the attached elisp yesterday.  It provides bindings to
 put sender or recipients into bbdb.  (Recipients part needs more
 testing.)  It also colors the from line green if the sender is in bbdb,
 or orange otherwise.  When it's been through a bit more testing, I'll
 submit at least the first part as a patch.

Hey, Daniel.  This is really nice!  All I had to do was load it in my
emacs config after loading notmuch:

(require 'notmuch)
(load-file ~/.notmuch/bbdb-notmuch.el)

It works very well: snarfs contacts like a charm, and, I must say, the
coloring of names depending on their status in bbdb is really hot!  Nice
feature!

Thanks so much for this contribution.  I would be thrilled if this made
it into upstream, maybe with a config to let people turn the sender
highlighting on or off.  Maybe in the mean time we can put it in
contrib?

jamie.


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


[RFC] emacs: use ido completing read for address completion in message mode

2012-07-12 Thread Mark Walters
This patch uses ido-completing-read for address completion in message
mode.  Although ido-completing-read is nominally a drop-in replacement
for completing-read `initial` and `default` behave rather differently
and it makes sense to use `default` rather than `initial` in the ido
case.
---

There was some interest on irc for using ido-completing-read for
address completion in message-mode so here is a trivial patch.

I am not sure I like the ido-completing-read behaviour as typing in
the minibuffer matches anywhere in the address not just at the start
of the address. I think there is a difference between this and setting
the users from address where it is likely that many of their addresses
start the same.

But it might be a starting point for experiments.

Best wishes

Mark


 emacs/notmuch-address.el |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
index 2bf762b..08e0e38 100644
--- a/emacs/notmuch-address.el
+++ b/emacs/notmuch-address.el
@@ -61,9 +61,9 @@ line.
  ((eq num-options 1)
   (car options))
  (t
-  (completing-read (format Address (%s matches):  
num-options)
-   (cdr options) nil nil (car options)
-   'notmuch-address-history)
+  (ido-completing-read (format Address (%s matches):  
num-options)
+   (cdr options) nil nil nil
+   'notmuch-address-history (car options))
 (if chosen
(progn
  (push chosen notmuch-address-history)
-- 
1.7.9.1

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


Re: notmuch-emacs and bbdb

2012-07-12 Thread Daniel Bergey
I hacked together the attached elisp yesterday.  It provides bindings to
put sender or recipients into bbdb.  (Recipients part needs more
testing.)  It also colors the from line green if the sender is in bbdb,
or orange otherwise.  When it's been through a bit more testing, I'll
submit at least the first part as a patch.

I prefer not to autocapture everything into bbdb, and for the same
reason, I don't want to use notmuch itself as my contacts DB.  Mostly,
this is because I read lots of email on lists, sent by people I'm
unlikely to write back to.  I don't like every John I've heard from
coming up on autocomplete.  Other reasons include sync to phone and
adding contact information from channels other than email.

bergey



bbdb-notmuch.el
Description: application/emacs-lisp


Jameson Graef Rollins jroll...@finestructure.net writes:

 On Tue, Jul 10 2012, Daniel Bergey ber...@alum.mit.edu wrote:
 As far as I can tell, notmuch doesn't integrate as smoothly with bbdb as 
 older
 emacs mailclients.  I'm especially looking for a snarf function that
 distinguishes sender from recipient.

 How do other people use bbdb with notmuch?

 Does anyone have lisp code like that which ships with bbdb for other
 clients?

 If I were to find time to write such code, what would you like it to do?

 Hey, Bergey.  This is something that I think needs improvement as well.
 I've been just manually constructing the bbdb entries myself.

 But what I've really been meaning to get going is address
 auto-completion from the database, which I'm pretty sure could obviate
 my need for bbdb altogether:

 http://notmuchmail.org/emacstips/#index13h2

 Getting it working seems more difficult than it should be, though, and
 the existing solutions seem a bit slower than they need to be [0].  So I
 think there's also room for improvement here.

 For instance, I think it would be rad if notmuch provided this
 functionality natively, in the CLI, or even in the library [1].  I think
 it's definitely doable, and it would be a nice little project.

 The emacs integration could be a bit smoother as well.  A single config
 option should either turn the functionality on or off.  That would be
 very convenient [2].

 jamie.

 [0] id:87r4xur3rv@plc.plecavalier.com
 [1] For what it's worth, I would prefer a solution that didn't involve
 any caching of addresses outside of the database.
 [2] I also find it changes the behavior of the ido tab completion
 interface that I'm used to using in message mode.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v4 1/3] Add support for structured output formatters.

2012-07-12 Thread Austin Clements
Quoth cra...@gmx.net on Jul 12 at  9:43 am:
 This patch adds a new type sprinter_t, which is used for structured
 formatting, e.g. JSON or S-Expressions. The structure printer is the
 code from Austin Clements (id:87d34hsdx8@awakening.csail.mit.edu).
 
 The structure printer contains the following function pointers:
 
 /* start a new map/dictionary structure.
  */
 void (*begin_map) (struct sprinter *);
 
 /* start a new list/array structure
  */
 void (*begin_list) (struct sprinter *);
 
 /* end the last opened list or map structure
  */
 void (*end) (struct sprinter *);
 
 /* print one string/integer/boolean/null element (possibly inside a
  * list or map
  */
 void (*string) (struct sprinter *, const char *);
 void (*integer) (struct sprinter *, int);
 void (*boolean) (struct sprinter *, notmuch_bool_t);
 void (*null) (struct sprinter *);
 
 /* print the key of a map's key/value pair.
  */
 void (*map_key) (struct sprinter *, const char *);
 
 /* print a frame delimiter (such as a newline or extra whitespace)
  */
 void (*frame) (struct sprinter *);
 
 The printer can (and should) use internal state to insert delimiters and
 syntax at the correct places.
 
 Example:
 
 format-begin_map(format);
 format-map_key(format, foo);
 format-begin_list(format);
 format-integer(format, 1);
 format-integer(format, 2);
 format-integer(format, 3);
 format-end(format);
 format-map_key(format, bar);
 format-begin_map(format);
 format-map_key(format, baaz);
 format-string(format, hello world);
 format-end(format);
 format-end(format);
 
 would output JSON as follows:
 
 {foo: [1, 2, 3], bar: { baaz: hello world}}
 ---
  sprinter.h | 49 +
  1 file changed, 49 insertions(+)
  create mode 100644 sprinter.h
 
 diff --git a/sprinter.h b/sprinter.h
 new file mode 100644
 index 000..1dad9a0
 --- /dev/null
 +++ b/sprinter.h
 @@ -0,0 +1,49 @@
 +#ifndef NOTMUCH_SPRINTER_H
 +#define NOTMUCH_SPRINTER_H
 +
 +/* for notmuch_bool_t */

Style/consistency nit: all of the comments should start with a capital
letter and anything that's a sentence should end with a period (some
of your comments do, some of them don't).

 +#include notmuch-client.h
 +
 +/* Structure printer interface */
 +typedef struct sprinter
 +{
 +/* start a new map/dictionary structure.

This should probably mention how other functions should be called
within the map structure.  Perhaps,

/* Start a new map/dictionary structure.  This should be followed by a
 * sequence of alternating calls to map_key and one of the value
 * printing functions until the map is ended.
 */

(You had a comment to this effect in one of your earlier patches.)

 + */
 +void (*begin_map) (struct sprinter *);
 +
 +/* start a new list/array structure
 + */
 +void (*begin_list) (struct sprinter *);
 +
 +/* end the last opened list or map structure
 + */
 +void (*end) (struct sprinter *);
 +
 +/* print one string/integer/boolean/null element (possibly inside a
 + * list or map

Missing close paren.

 + */
 +void (*string) (struct sprinter *, const char *);
 +void (*integer) (struct sprinter *, int);
 +void (*boolean) (struct sprinter *, notmuch_bool_t);
 +void (*null) (struct sprinter *);
 +
 +/* print the key of a map's key/value pair.
 + */
 +void (*map_key) (struct sprinter *, const char *);
 +
 +/* print a frame delimiter (such as a newline or extra whitespace)
 + */
 +void (*frame) (struct sprinter *);

I wonder if frame is still necessary.  At the time, I was thinking
that we would use embedded newline framing to do streaming JSON
parsing, but I wound up going with a general incremental JSON parser,
eliminating the need for framing.

It's probably still useful to have a function that inserts a newline
purely for readability/debuggability purposes.  In practice, the
implementation would be the same as frame (though if it's for
readability, maybe you'd want to print the comma before the newline
instead of after), but since the intent would be different, it would
need different documentation and maybe a different name.  Frame
isn't a bad name for either intent.  We can't use break.
Separator (or just sep)?  Space?  Split?  The comment could be
something like

/* Insert a separator for improved readability without affecting the
 * abstract syntax of the structure being printed.  For JSON, this is
 * simply a line break.
 */

 +} sprinter_t;
 +
 +/* Create a new structure printer that emits JSON */
 +struct sprinter *
 +sprinter_json_new(const void *ctx, FILE *stream);

This should probably be called sprinter_json_create to be consistent
with the naming of other constructors in notmuch.

Also, missing space between the function name and parameter list
(sorry, my fault).

 +
 +/* A dummy structure printer that signifies that standard text output is
 + * to be used instead of any structured format.
 + */
 +struct sprinter *
 +sprinter_text;

It looks like 

Re: [PATCH v4 2/3] Add structured output formatter for JSON.

2012-07-12 Thread Austin Clements
Quoth Tomi Ollila on Jul 12 at  1:10 pm:
 On Thu, Jul 12 2012, cra...@gmx.net wrote:
 
  Using the new structured printer support in sprinter.h, implement
  sprinter_json_new, which returns a new JSON structured output
  formatter.
 
  The formatter prints output similar to the existing JSON, but with
  differences in whitespace (mostly newlines).
  ---
   Makefile.local |   1 +
   sprinter.c | 172 
  +
   2 files changed, 173 insertions(+)
   create mode 100644 sprinter.c
 
  diff --git a/Makefile.local b/Makefile.local
  index a890df2..8baf0c2 100644
  --- a/Makefile.local
  +++ b/Makefile.local
  @@ -290,6 +290,7 @@ notmuch_client_srcs =   \
  notmuch-show.c  \
  notmuch-tag.c   \
  notmuch-time.c  \
  +   sprinter.c  \
  query-string.c  \
  mime-node.c \
  crypto.c\
  diff --git a/sprinter.c b/sprinter.c
  new file mode 100644
  index 000..649f79a
  --- /dev/null
  +++ b/sprinter.c
  @@ -0,0 +1,172 @@
  +#include stdbool.h
  +#include stdio.h
  +#include talloc.h
  +#include sprinter.h
  +
  +#define ARRAY_SIZE(arr) (sizeof (arr) / sizeof (arr[0]))
 
 You're including sprinter.h which includes notmuch-client.h which
 defines ARRAY_SIZE (Interesting that you did not get error(/warning?)
 about this)
 
 Rest looks good -- except the whitespace -- as I looked through Austin's
 code yesterday you're just replicated the same lines :D. Easiest
 to fix is probably just running 
 uncrustify -c devel/uncrustify.cfg --replace sprinter.c 

Oops.  Shame on me.  Is anything wrong besides the missing space
before the argument list of all of the function declarations?
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v4 2/3] Add structured output formatter for JSON.

2012-07-12 Thread Austin Clements
Quoth cra...@gmx.net on Jul 12 at  9:43 am:
 Using the new structured printer support in sprinter.h, implement
 sprinter_json_new, which returns a new JSON structured output
 formatter.
 
 The formatter prints output similar to the existing JSON, but with
 differences in whitespace (mostly newlines).
 ---
  Makefile.local |   1 +
  sprinter.c | 172 
 +
  2 files changed, 173 insertions(+)
  create mode 100644 sprinter.c
 
 diff --git a/Makefile.local b/Makefile.local
 index a890df2..8baf0c2 100644
 --- a/Makefile.local
 +++ b/Makefile.local
 @@ -290,6 +290,7 @@ notmuch_client_srcs = \
   notmuch-show.c  \
   notmuch-tag.c   \
   notmuch-time.c  \
 + sprinter.c  \
   query-string.c  \
   mime-node.c \
   crypto.c\
 diff --git a/sprinter.c b/sprinter.c
 new file mode 100644
 index 000..649f79a
 --- /dev/null
 +++ b/sprinter.c
 @@ -0,0 +1,172 @@
 +#include stdbool.h
 +#include stdio.h
 +#include talloc.h
 +#include sprinter.h
 +
 +#define ARRAY_SIZE(arr) (sizeof (arr) / sizeof (arr[0]))
 +
 +struct sprinter *
 +sprinter_text = NULL;
 +
 +/*
 + * Every below here is the implementation of the JSON printer.

s/Every/Everything/

Or this can be shortened to just

/*
 * JSON printer
 */

If we wind up with separate files for the JSON and S-exp printers, I
don't think this comment is necessary at all.

 + */
 +
 +struct sprinter_json
 +{
 +struct sprinter vtable;
 +FILE *stream;
 +/* Top of the state stack, or NULL if the printer is not currently
 + * inside any aggregate types. */
 +struct json_state *state;
 +};
 +
 +struct json_state
 +{
 +struct json_state *parent;
 +/* True if nothing has been printed in this aggregate yet.
 + * Suppresses the comma before a value. */
 +notmuch_bool_t first;
 +/* The character that closes the current aggregate. */
 +char close;
 +};
 +
 +/* Helper function to set up the stream to print a value.  If this
 + * value follows another value, prints a comma. */
 +static struct sprinter_json *
 +json_begin_value(struct sprinter *sp)

As Tomi pointed out, there should be spaces before the parameter
lists (sorry).

 +{
 +struct sprinter_json *spj = (struct sprinter_json*)sp;
 +if (spj-state) {
 + if (!spj-state-first)
 + fputs (, , spj-stream);
 + else
 + spj-state-first = false;
 +}
 +return spj;
 +}
 +
 +/* Helper function to begin an aggregate type.  Prints the open
 + * character and pushes a new state frame. */
 +static void
 +json_begin_aggregate(struct sprinter *sp, char open, char close)
 +{
 +struct sprinter_json *spj = json_begin_value (sp);
 +struct json_state *state = talloc (spj, struct json_state);
 +
 +fputc (open, spj-stream);
 +state-parent = spj-state;
 +state-first = true;
 +state-close = close;
 +spj-state = state;
 +}
 +
 +static void
 +json_begin_map(struct sprinter *sp)
 +{
 +json_begin_aggregate (sp, '{', '}');
 +}
 +
 +static void
 +json_begin_list(struct sprinter *sp)
 +{
 +json_begin_aggregate (sp, '[', ']');
 +}
 +
 +static void
 +json_end(struct sprinter *sp)
 +{
 +struct sprinter_json *spj = (struct sprinter_json*)sp;
 +struct json_state *state = spj-state;
 +
 +fputc (spj-state-close, spj-stream);
 +spj-state = state-parent;
 +talloc_free (state);
 +if(spj-state == NULL)

Ah, another missing space.

 + fputc ('\n', spj-stream);
 +}
 +
 +static void
 +json_string(struct sprinter *sp, const char *val)
 +{
 +static const char * const escapes[] = {
 + ['\'] = \\\, ['\\'] = , ['\b'] = \\b,
 + ['\f'] = \\f,  ['\n'] = \\n,  ['\t'] = \\t
 +};
 +struct sprinter_json *spj = json_begin_value (sp);
 +fputc ('', spj-stream);
 +for (; *val; ++val) {
 + unsigned char ch = *val;
 + if (ch  ARRAY_SIZE(escapes)  escapes[ch])
 + fputs (escapes[ch], spj-stream);
 + else if (ch = 32)
 + fputc (ch, spj-stream);
 + else
 + fprintf (spj-stream, \\u%04x, ch);
 +}
 +fputc ('', spj-stream);
 +}
 +
 +static void
 +json_integer(struct sprinter *sp, int val)
 +{
 +struct sprinter_json *spj = json_begin_value (sp);
 +fprintf (spj-stream, %d, val);
 +}
 +
 +static void
 +json_boolean(struct sprinter *sp, notmuch_bool_t val)
 +{
 +struct sprinter_json *spj = json_begin_value (sp);
 +fputs (val ? true : false, spj-stream);
 +}
 +
 +static void
 +json_null(struct sprinter *sp)
 +{
 +struct sprinter_json *spj = json_begin_value (sp);
 +fputs (null, spj-stream);
 +}
 +
 +static void
 +json_map_key(struct sprinter *sp, const char *key)
 +{
 +struct sprinter_json *spj = (struct sprinter_json*)sp;
 +json_string (sp, key);
 +fputs (: , spj-stream);
 +spj-state-first = true;
 +}
 +
 +static void
 +json_frame(struct sprinter *sp)
 +{
 +struct sprinter_json 

Re: [PATCH v4 1/3] Add support for structured output formatters.

2012-07-12 Thread Austin Clements
Quoth cra...@gmx.net on Jul 12 at  9:43 am:
 This patch adds a new type sprinter_t, which is used for structured
 formatting, e.g. JSON or S-Expressions. The structure printer is the
 code from Austin Clements (id:87d34hsdx8@awakening.csail.mit.edu).
 
 The structure printer contains the following function pointers:
 
 /* start a new map/dictionary structure.
  */
 void (*begin_map) (struct sprinter *);
 
 /* start a new list/array structure
  */
 void (*begin_list) (struct sprinter *);
 
 /* end the last opened list or map structure
  */
 void (*end) (struct sprinter *);
 
 /* print one string/integer/boolean/null element (possibly inside a
  * list or map
  */
 void (*string) (struct sprinter *, const char *);
 void (*integer) (struct sprinter *, int);
 void (*boolean) (struct sprinter *, notmuch_bool_t);
 void (*null) (struct sprinter *);
 
 /* print the key of a map's key/value pair.
  */
 void (*map_key) (struct sprinter *, const char *);
 
 /* print a frame delimiter (such as a newline or extra whitespace)
  */
 void (*frame) (struct sprinter *);
 
 The printer can (and should) use internal state to insert delimiters and
 syntax at the correct places.
 
 Example:
 
 format-begin_map(format);
 format-map_key(format, foo);
 format-begin_list(format);
 format-integer(format, 1);
 format-integer(format, 2);
 format-integer(format, 3);
 format-end(format);
 format-map_key(format, bar);
 format-begin_map(format);
 format-map_key(format, baaz);
 format-string(format, hello world);
 format-end(format);
 format-end(format);
 
 would output JSON as follows:
 
 {foo: [1, 2, 3], bar: { baaz: hello world}}
 ---
  sprinter.h | 49 +
  1 file changed, 49 insertions(+)
  create mode 100644 sprinter.h
 
 diff --git a/sprinter.h b/sprinter.h
 new file mode 100644
 index 000..1dad9a0
 --- /dev/null
 +++ b/sprinter.h
 @@ -0,0 +1,49 @@
 +#ifndef NOTMUCH_SPRINTER_H
 +#define NOTMUCH_SPRINTER_H
 +
 +/* for notmuch_bool_t */
 +#include notmuch-client.h
 +
 +/* Structure printer interface */
 +typedef struct sprinter
 +{
 +/* start a new map/dictionary structure.
 + */
 +void (*begin_map) (struct sprinter *);
 +
 +/* start a new list/array structure
 + */
 +void (*begin_list) (struct sprinter *);
 +
 +/* end the last opened list or map structure
 + */
 +void (*end) (struct sprinter *);
 +
 +/* print one string/integer/boolean/null element (possibly inside a
 + * list or map
 + */
 +void (*string) (struct sprinter *, const char *);

Oh, also, the documentation for string should mention the string
argument should be UTF-8 encoded.

 +void (*integer) (struct sprinter *, int);
 +void (*boolean) (struct sprinter *, notmuch_bool_t);
 +void (*null) (struct sprinter *);
 +
 +/* print the key of a map's key/value pair.
 + */
 +void (*map_key) (struct sprinter *, const char *);
 +
 +/* print a frame delimiter (such as a newline or extra whitespace)
 + */
 +void (*frame) (struct sprinter *);
 +} sprinter_t;
 +
 +/* Create a new structure printer that emits JSON */
 +struct sprinter *
 +sprinter_json_new(const void *ctx, FILE *stream);
 +
 +/* A dummy structure printer that signifies that standard text output is
 + * to be used instead of any structured format.
 + */
 +struct sprinter *
 +sprinter_text;
 +
 +#endif // NOTMUCH_SPRINTER_H
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 1/2] contrib/nmbug/ nmbug-status: restored out['subject']... block level

2012-07-12 Thread David Bremner
On Wed, 11 Jul 2012 12:10:04 +0300, Tomi Ollila tomi.oll...@iki.fi wrote:
 In reformatting the line 111 accidentally indented to one indentation
 level too much (happens easily when interactively indenting python
 code using emacs). The line now has 4 spacess less indentation, thus
 restoring it to the block level it belongs.

Thanks Tomi, both pushed

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


Re: [PATCH v4 3/3] Use the structured format printer for JSON in notmuch search.

2012-07-12 Thread Austin Clements
This is fantastic.  It simplifies the code a lot, and I think it opens
up opportunities to simplify it even further.

Detailed comments are below, but first one general comment.  For the
text format, I wonder if most of the special case code would go away
with a stub sprinter that did nothing for most operations and for
string printed the string followed by a newline (or maybe the newline
would be printed by the frame method or whatever you end up calling
it).  I believe this would unify all of the code in do_search_tags and
do_search_files between the two formats.  For do_search_threads,
you'll still need some special-casing to format the summary line, but
I think it would unify all of the framing code.  (If this does work
out, some of my comments below will be irrelevant.)

Quoth cra...@gmx.net on Jul 12 at  9:43 am:
 This patch switches from the current ad-hoc printer to the structured
 output formatter in sprinter.h.
 
 It removes search_format_t, replaces it by sprinter_t and inlines the
 text printer where necessary.
 
 The tests are changed (only whitespaces and regular expressions) in
 order to make them PASS for the new structured output formatter.
 ---
  notmuch-search.c   | 349 
 ++---
  test/json  |  20 +--
  test/search-output | 270 +
  3 files changed, 288 insertions(+), 351 deletions(-)
 
 diff --git a/notmuch-search.c b/notmuch-search.c
 index 3be296d..b853f5f 100644
 --- a/notmuch-search.c
 +++ b/notmuch-search.c
 @@ -19,6 +19,7 @@
   */
  
  #include notmuch-client.h
 +#include sprinter.h
  
  typedef enum {
  OUTPUT_SUMMARY,
 @@ -28,91 +29,9 @@ typedef enum {
  OUTPUT_TAGS
  } output_t;
  
 -typedef struct search_format {
 -const char *results_start;
 -const char *item_start;
 -void (*item_id) (const void *ctx,
 -  const char *item_type,
 -  const char *item_id);
 -void (*thread_summary) (const void *ctx,
 - const char *thread_id,
 - const time_t date,
 - const int matched,
 - const int total,
 - const char *authors,
 - const char *subject);
 -const char *tag_start;
 -const char *tag;
 -const char *tag_sep;
 -const char *tag_end;
 -const char *item_sep;
 -const char *item_end;
 -const char *results_end;
 -const char *results_null;
 -} search_format_t;
 -
 -static void
 -format_item_id_text (const void *ctx,
 -  const char *item_type,
 -  const char *item_id);
 -
 -static void
 -format_thread_text (const void *ctx,
 - const char *thread_id,
 - const time_t date,
 - const int matched,
 - const int total,
 - const char *authors,
 - const char *subject);
 -static const search_format_t format_text = {
 -,
 - ,
 - format_item_id_text,
 - format_thread_text,
 -  (,
 - %s,  ,
 - ), \n,
 - ,
 -\n,
 -,
 -};
 -
 -static void
 -format_item_id_json (const void *ctx,
 -  const char *item_type,
 -  const char *item_id);
 -
 -static void
 -format_thread_json (const void *ctx,
 - const char *thread_id,
 - const time_t date,
 - const int matched,
 - const int total,
 - const char *authors,
 - const char *subject);
 -
 -/* Any changes to the JSON format should be reflected in the file
 - * devel/schemata. */
 -static const search_format_t format_json = {
 -[,
 - {,
 - format_item_id_json,
 - format_thread_json,
 - \tags\: [,
 - \%s\, , ,
 - ], ,\n,
 - },
 -]\n,
 -]\n,
 -};
 -
 -static void
 -format_item_id_text (unused (const void *ctx),
 -  const char *item_type,
 -  const char *item_id)
 -{
 -printf (%s%s, item_type, item_id);
 -}
 +static const char * text_item_sep = \n;
 +static const char * text_results_null = ;
 +static const char * text_results_end = \n;

Given that you're special-casing the text format anyway, I think it's
actually more readable if you hard-code these in the appropriate
places below.

  
  static char *
  sanitize_string (const void *ctx, const char *str)
 @@ -131,72 +50,8 @@ sanitize_string (const void *ctx, const char *str)
  return out;
  }
  
 -static void
 -format_thread_text (const void *ctx,
 - const char *thread_id,
 - const time_t date,
 - const int matched,
 - const int total,
 - const char *authors,
 - const char *subject)
 -{
 -void *ctx_quote = talloc_new (ctx);
 -
 -printf (thread:%s %12s [%d/%d] %s; %s,
 - thread_id,
 

Re: [PATCH v4 3/3] Use the structured format printer for JSON in notmuch search.

2012-07-12 Thread Austin Clements
Quoth myself on Jul 12 at  8:02 pm:
 This is fantastic.  It simplifies the code a lot, and I think it opens
 up opportunities to simplify it even further.
 
 Detailed comments are below, but first one general comment.  For the
 text format, I wonder if most of the special case code would go away
 with a stub sprinter that did nothing for most operations and for
 string printed the string followed by a newline (or maybe the newline
 would be printed by the frame method or whatever you end up calling
 it).  I believe this would unify all of the code in do_search_tags and
 do_search_files between the two formats.  For do_search_threads,
 you'll still need some special-casing to format the summary line, but
 I think it would unify all of the framing code.  (If this does work
 out, some of my comments below will be irrelevant.)

Oh, wait, the text format adds prefixes...

I think something along the lines of what I described above would
still simplify things.  I can think of two ways to do it.  You could
have a completely stub sprinter where everything is a no-op; that
would at least save the predication of calls like
format-begin_list ().  Or, you could have an additional function for
the text formatter that registers a prefix, and have the string method
first print that prefix and then the argument string.  This additional
function doesn't have to be a method of sprinter; it could just be a
regular function that stashes the prefix away somewhere (a global
variable's probably fine; if you want to be fancy you store it in an
sprinter_text extension of sprinter_t like the one JSON uses).  For
the JSON format, this would be a no-op, so you could call it
regardless of what format you've selected.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 2/7] emacs: Use text properties instead of overlays for tag coloring

2012-07-12 Thread Austin Clements
Previously, tag-based search result highlighting was done by creating
an overlay over each search result.  However, overlays have annoying
front- and rear-advancement semantics that make it difficult to
manipulate text at their boundaries, which the next patch will do.
They also have performance problems (creating an overlay is linear in
the number of overlays between point and the new overlay, making
highlighting a search buffer quadratic in the number of results).

Text properties have neither problem.  However, text properties make
it more difficult to apply multiple faces since, unlike with overlays,
a given character can only have a single 'face text property.  Hence,
we introduce a utility function that combines faces into any existing
'face text properties.

Using this utility function, it's straightforward to apply all of the
appropriate tag faces in notmuch-search-color-line.
---
 emacs/notmuch-lib.el |   15 +++
 emacs/notmuch.el |   21 +++--
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index aa25513..30db58f 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -269,6 +269,21 @@ current buffer, if possible.
   (loop for (key value . rest) on plist by #'cddr
collect (cons (intern (substring (symbol-name key) 1)) value)))
 
+(defun notmuch-combine-face-text-property (start end face)
+  Combine FACE into the 'face text property between START and END.
+
+This function combines FACE with any existing faces between START
+and END.  Attributes specified by FACE take precedence over
+existing attributes.  FACE must be a face name (a symbol or
+string), a property list of face attributes, or a list of these.
+
+  (let ((pos start))
+(while ( pos end)
+  (let ((cur (get-text-property pos 'face))
+   (next (next-single-property-change pos 'face nil end)))
+   (put-text-property pos next 'face (cons face cur))
+   (setq pos next)
+
 ;; Compatibility functions for versions of emacs before emacs 23.
 ;;
 ;; Both functions here were copied from emacs 23 with the following copyright:
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index ef18927..82c148d 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -633,20 +633,13 @@ foreground and blue background.
 
 (defun notmuch-search-color-line (start end line-tag-list)
   Colorize lines in `notmuch-show' based on tags.
-  ;; Create the overlay only if the message has tags which match one
-  ;; of those specified in `notmuch-search-line-faces'.
-  (let (overlay)
-(mapc (lambda (elem)
-   (let ((tag (car elem))
- (attributes (cdr elem)))
- (when (member tag line-tag-list)
-   (when (not overlay)
- (setq overlay (make-overlay start end)))
-   ;; Merge the specified properties with any already
-   ;; applied from an earlier match.
-   (overlay-put overlay 'face
-(append (overlay-get overlay 'face) attributes)
- notmuch-search-line-faces)))
+  (mapc (lambda (elem)
+ (let ((tag (car elem))
+   (attributes (cdr elem)))
+   (when (member tag line-tag-list)
+ (notmuch-combine-face-text-property start end attributes
+   ;; Reverse the list so earlier entries take precedence
+   (reverse notmuch-search-line-faces)))
 
 (defun notmuch-search-author-propertize (authors)
   Split `authors' into matching and non-matching authors and
-- 
1.7.10

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


[PATCH 4/7] emacs: Use result text properties for search result iteration

2012-07-12 Thread Austin Clements
This simplifies the traversal of regions of results and eliminates the
need for save-excursions (which tend to get in the way of maintaining
point when we make changes to the buffer).  It also fixes some strange
corner cases in the old line-based code where results that bordered
the region but were not included in it could be affected by region
commands.  Coincidentally, this also essentially enables multi-line
search result formats; the only remaining non-multi-line-capable
functions are notmuch-search-{next,previous}-thread, which are only
used interactively.
---
 emacs/notmuch.el |   61 ++
 1 file changed, 34 insertions(+), 27 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 2f83425..3f72427 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -427,17 +427,33 @@ returns nil
 (next-single-property-change (or pos (point)) 'notmuch-search-result
 nil (point-max
 
+(defmacro notmuch-search-do-results (beg end pos-sym rest body)
+  Invoke BODY for each result between BEG and END.
+
+POS-SYM will be bound to the point at the beginning of the
+current result.
+  (declare (indent 3))
+  (let ((end-sym (make-symbol end))
+   (first-sym (make-symbol first)))
+`(let ((,pos-sym (notmuch-search-result-beginning ,beg))
+  ;; End must be a marker in case body changes the text
+  (,end-sym (copy-marker ,end))
+  ;; Make sure we examine one result, even if (= beg end)
+  (,first-sym t))
+   ;; We have to be careful if the region extends beyond the
+   ;; results.  In this case, pos could be null or there could be
+   ;; no result at pos.
+   (while (and ,pos-sym (or ( ,pos-sym ,end-sym) ,first-sym))
+(when (notmuch-search-get-result ,pos-sym)
+  ,@body)
+(setq ,pos-sym (notmuch-search-result-end ,pos-sym)
+  ,first-sym nil)
+
 (defun notmuch-search-properties-in-region (property beg end)
-  (save-excursion
-(let ((output nil)
- (last-line (line-number-at-pos end))
- (max-line (- (line-number-at-pos (point-max)) 2)))
-  (goto-char beg)
-  (beginning-of-line)
-  (while (= (line-number-at-pos) (min last-line max-line))
-   (setq output (cons (get-text-property (point) property) output))
-   (forward-line 1))
-  output)))
+  (let (output)
+(notmuch-search-do-results beg end pos
+  (push (get-text-property pos property) output))
+output))
 
 (defun notmuch-search-find-thread-id ()
   Return the thread for the current thread
@@ -517,28 +533,19 @@ and will also appear in a buffer named \*Notmuch 
errors*\.
   (plist-get (notmuch-search-get-result pos) :tags))
 
 (defun notmuch-search-get-tags-region (beg end)
-  (save-excursion
-(let ((output nil)
- (last-line (line-number-at-pos end))
- (max-line (- (line-number-at-pos (point-max)) 2)))
-  (goto-char beg)
-  (while (= (line-number-at-pos) (min last-line max-line))
-   (setq output (append output (notmuch-search-get-tags)))
-   (forward-line 1))
-  output)))
+  (let (output)
+(notmuch-search-do-results beg end pos
+  (setq output (append output (notmuch-search-get-tags pos
+output))
 
 (defun notmuch-search-tag-region (beg end optional tag-changes)
   Change tags for threads in the given region.
   (let ((search-string (notmuch-search-find-thread-id-region-search beg end)))
 (setq tag-changes (funcall 'notmuch-tag search-string tag-changes))
-(save-excursion
-  (let ((last-line (line-number-at-pos end))
-   (max-line (- (line-number-at-pos (point-max)) 2)))
-   (goto-char beg)
-   (while (= (line-number-at-pos) (min last-line max-line))
- (notmuch-search-set-tags
-  (notmuch-update-tags (notmuch-search-get-tags) tag-changes))
- (forward-line))
+(notmuch-search-do-results beg end pos
+  (notmuch-search-set-tags
+   (notmuch-update-tags (notmuch-search-get-tags pos) tag-changes)
+   pos
 
 (defun notmuch-search-tag (optional tag-changes)
   Change tags for the currently selected thread or region.
-- 
1.7.10

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


[PATCH 1/7] emacs: Record thread search result object in a text property

2012-07-12 Thread Austin Clements
This also provides utility functions for working with this text
property that get its value, find its start, and find its end.
---
 emacs/notmuch.el |   27 +++
 1 file changed, 27 insertions(+)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index fabb7c0..ef18927 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -401,6 +401,32 @@ Complete list of currently available key bindings:
mode-name notmuch-search)
   (setq buffer-read-only t))
 
+(defun notmuch-search-get-result (optional pos)
+  Return the result object for the thread at POS (or point).
+
+If there is no thread at POS (or point), returns nil.
+  (get-text-property (or pos (point)) 'notmuch-search-result))
+
+(defun notmuch-search-result-beginning (optional pos)
+  Return the point at the beginning of the thread at POS (or point).
+
+If there is no thread at POS (or point), returns nil.
+  (when (notmuch-search-get-result pos)
+;; We pass 1+point because previous-single-property-change starts
+;; searching one before the position we give it.
+(previous-single-property-change (1+ (or pos (point)))
+'notmuch-search-result nil (point-min
+
+(defun notmuch-search-result-end (optional pos)
+  Return the point at the end of the thread at POS (or point).
+
+The returned point will be just after the newline character that
+ends the result line.  If there is no thread at POS (or point),
+returns nil
+  (when (notmuch-search-get-result pos)
+(next-single-property-change (or pos (point)) 'notmuch-search-result
+nil (point-max
+
 (defun notmuch-search-properties-in-region (property beg end)
   (save-excursion
 (let ((output nil)
@@ -736,6 +762,7 @@ non-authors is found, assume that all of the authors match.
  (notmuch-search-insert-field (car spec) (cdr spec) result))
(insert \n)
(notmuch-search-color-line beg (point) (plist-get result :tags))
+   (put-text-property beg (point) 'notmuch-search-result result)
(put-text-property beg (point) 'notmuch-search-thread-id
   (concat thread: (plist-get result :thread)))
(put-text-property beg (point) 'notmuch-search-authors
-- 
1.7.10

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


[PATCH 3/7] emacs: Update tags by rewriting the search result line in place

2012-07-12 Thread Austin Clements
Now that we keep the full thread result object, we can refresh a
result after any changes by simply deleting and reconstructing the
result line from scratch.

A convenient side-effect of this wholesale replacement is that search
now re-applies notmuch-search-line-faces when tags change.
---
 emacs/notmuch.el |   55 ++
 1 file changed, 31 insertions(+), 24 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 82c148d..2f83425 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -509,28 +509,12 @@ and will also appear in a buffer named \*Notmuch 
errors*\.
(error (buffer-substring beg end))
))
 
-(defun notmuch-search-set-tags (tags)
-  (save-excursion
-(end-of-line)
-(re-search-backward ()
-(forward-char)
-(let ((beg (point))
- (inhibit-read-only t))
-  (re-search-forward ))
-  (backward-char)
-  (let ((end (point)))
-   (delete-region beg end)
-   (insert (propertize (mapconcat  'identity tags  )
-   'face 'notmuch-tag-face))
-
-(defun notmuch-search-get-tags ()
-  (save-excursion
-(end-of-line)
-(re-search-backward ()
-(let ((beg (+ (point) 1)))
-  (re-search-forward ))
-  (let ((end (- (point) 1)))
-   (split-string (buffer-substring-no-properties beg end))
+(defun notmuch-search-set-tags (tags optional pos)
+  (let ((new-result (plist-put (notmuch-search-get-result pos) :tags tags)))
+(notmuch-search-update-result new-result pos)))
+
+(defun notmuch-search-get-tags (optional pos)
+  (plist-get (notmuch-search-get-result pos) :tags))
 
 (defun notmuch-search-get-tags-region (beg end)
   (save-excursion
@@ -583,6 +567,29 @@ This function advances the next thread when finished.
   (notmuch-search-tag '(-inbox))
   (notmuch-search-next-thread))
 
+(defun notmuch-search-update-result (result optional pos)
+  Update the result object of the current thread and redraw it.
+  (let ((start (notmuch-search-result-beginning pos))
+   (end (notmuch-search-result-end pos))
+   (init-point (point))
+   (inhibit-read-only t))
+;; Delete the current thread
+(delete-region start end)
+;; Insert the updated thread
+(notmuch-search-show-result result start)
+;; There may have been markers pointing into the text we just
+;; replaced.  For the most part, there's nothing we can do about
+;; this, but we can fix markers that were at point (which includes
+;; point itself and any save-excursions for which point hasn't
+;; moved) by re-inserting the text that should come before point
+;; before markers.
+(when (and (= init-point start) (= init-point end))
+  (let* ((new-end (notmuch-search-result-end start))
+(new-point (if (= init-point end)
+   new-end
+ (min init-point (- new-end 1)
+   (insert-before-markers (delete-and-extract-region start new-point))
+
 (defun notmuch-search-process-sentinel (proc msg)
   Add a message to let user know when \notmuch search\ exits
   (let ((buffer (process-buffer proc))
@@ -745,10 +752,10 @@ non-authors is found, assume that all of the authors 
match.
 (mapconcat 'identity (plist-get result :tags)  )
 'font-lock-face 'notmuch-tag-face) ))
 
-(defun notmuch-search-show-result (result)
+(defun notmuch-search-show-result (result optional pos)
   ;; Ignore excluded matches
   (unless (= (plist-get result :matched) 0)
-(let ((beg (point-max)))
+(let ((beg (or pos (point-max
   (save-excursion
(goto-char beg)
(dolist (spec notmuch-search-result-format)
-- 
1.7.10

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


[PATCH 0/7] emacs: JSON-based search cleanups

2012-07-12 Thread Austin Clements
This series builds on the JSON-based search series [0] to clean up
several other aspects of search-mode.  It removes constraints on the
formatting of tags in the result line (you can even leave them out
entirely), it recolors lines when tags change, it adds supports for
multi-line result formats, and rendering big search buffers should be
less quadratic (it might even be linear).  Much of this derives from
having a single object representation of a result (the JSON plist) and
a simple method for rendering it to the buffer.

[0] 1341870162-17782-1-git-send-email-amdra...@mit.edu

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


[PATCH 7/7] emacs: Fix navigation of multi-line search result formats

2012-07-12 Thread Austin Clements
At this point, the only remaining functions that don't support
multi-line search result formats are the thread navigation functions.
This patch fixes that by rewriting them in terms of
notmuch-search-result-{beginning,end}.
---
 emacs/notmuch.el |   12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index f32cfb0..2ece97d 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -287,18 +287,24 @@ For a mouse binding, return nil.
 (defun notmuch-search-next-thread ()
   Select the next thread in the search results.
   (interactive)
-  (forward-line 1))
+  (when (notmuch-search-get-result (notmuch-search-result-end))
+(goto-char (notmuch-search-result-end
 
 (defun notmuch-search-previous-thread ()
   Select the previous thread in the search results.
   (interactive)
-  (forward-line -1))
+  (if (notmuch-search-get-result)
+  (unless (bobp)
+   (goto-char (notmuch-search-result-beginning (- (point) 1
+;; We must be past the end; jump to the last result
+(notmuch-search-last-thread)))
 
 (defun notmuch-search-last-thread ()
   Select the last thread in the search results.
   (interactive)
   (goto-char (point-max))
-  (forward-line -2))
+  (forward-line -2)
+  (goto-char (notmuch-search-result-beginning)))
 
 (defun notmuch-search-first-thread ()
   Select the first thread in the search results.
-- 
1.7.10

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


[PATCH 5/7] emacs: Replace other search text properties with result property

2012-07-12 Thread Austin Clements
Since the result object contains everything that the other text
properties recorded, we can remove the other text properties and
simply look in the plist of the appropriate result object.
---
 emacs/notmuch.el |   24 ++--
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 3f72427..a5cf0dc 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -452,16 +452,18 @@ current result.
 (defun notmuch-search-properties-in-region (property beg end)
   (let (output)
 (notmuch-search-do-results beg end pos
-  (push (get-text-property pos property) output))
+  (push (plist-get (notmuch-search-get-result pos) property) output))
 output))
 
 (defun notmuch-search-find-thread-id ()
   Return the thread for the current thread
-  (get-text-property (point) 'notmuch-search-thread-id))
+  (let ((thread (plist-get (notmuch-search-get-result) :thread)))
+(when thread (concat thread: thread
 
 (defun notmuch-search-find-thread-id-region (beg end)
   Return a list of threads for the current region
-  (notmuch-search-properties-in-region 'notmuch-search-thread-id beg end))
+  (mapcar (lambda (thread) (concat thread: thread))
+ (notmuch-search-properties-in-region :thread beg end)))
 
 (defun notmuch-search-find-thread-id-region-search (beg end)
   Return a search string for threads for the current region
@@ -469,19 +471,19 @@ current result.
 
 (defun notmuch-search-find-authors ()
   Return the authors for the current thread
-  (get-text-property (point) 'notmuch-search-authors))
+  (plist-get (notmuch-search-get-result) :authors))
 
 (defun notmuch-search-find-authors-region (beg end)
   Return a list of authors for the current region
-  (notmuch-search-properties-in-region 'notmuch-search-authors beg end))
+  (notmuch-search-properties-in-region :authors beg end))
 
 (defun notmuch-search-find-subject ()
   Return the subject for the current thread
-  (get-text-property (point) 'notmuch-search-subject))
+  (plist-get (notmuch-search-get-result) :subject))
 
 (defun notmuch-search-find-subject-region (beg end)
   Return a list of authors for the current region
-  (notmuch-search-properties-in-region 'notmuch-search-subject beg end))
+  (notmuch-search-properties-in-region :subject beg end))
 
 (defun notmuch-search-show-thread ()
   Display the currently selected thread.
@@ -769,13 +771,7 @@ non-authors is found, assume that all of the authors 
match.
  (notmuch-search-insert-field (car spec) (cdr spec) result))
(insert \n)
(notmuch-search-color-line beg (point) (plist-get result :tags))
-   (put-text-property beg (point) 'notmuch-search-result result)
-   (put-text-property beg (point) 'notmuch-search-thread-id
-  (concat thread: (plist-get result :thread)))
-   (put-text-property beg (point) 'notmuch-search-authors
-  (plist-get result :authors))
-   (put-text-property beg (point) 'notmuch-search-subject
-  (plist-get result :subject)))
+   (put-text-property beg (point) 'notmuch-search-result result))
   (when (string= (plist-get result :thread) notmuch-search-target-thread)
(setq notmuch-search-target-thread found)
(goto-char beg)
-- 
1.7.10

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


Re: [PATCH v3 1/9] emacs: Clean up notmuch-search-show-result

2012-07-12 Thread David Bremner
Austin Clements amdra...@mit.edu writes:

 This simplifies the code and makes it no longer cubic in the number of
 result fields.
 ---

series pushed.

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


Re: [PATCH 1/3] emacs: add no-display arg to notmuch-hello-refresh-hook

2012-07-12 Thread Austin Clements
Quoth Jani Nikula on Jul 10 at  8:10 am:
On Jul 10, 2012 4:49 AM, Austin Clements [1]amdra...@mit.edu wrote:

 Quoth Jani Nikula on Jun 15 at  6:53 pm:
  Add no-display arg to notmuch-hello-refresh-hook to allow each hook to
  decide what is appropriate when no-display is t, which is typically
  the case when called non-interactively. This is used by the following
  patch.
 
  This breaks existing hooks people might have, which will now need to
  accept the argument.
 
  Signed-off-by: Jani Nikula [2]j...@nikula.org

 This seems like an overloaded use of no-display.  If I'm reading the
 code right, no-display indicates whether or not the notmuch-hello
 buffer should be switched to and seems like a workaround for some
 particular corner-case (I'm not even sure what).  This seems like a
 strange condition to predicate a hook on (but maybe I just don't
 understand).  What condition, abstractly speaking, is
 notmuch-hello-refresh-status-message trying to run under?
 
IIUC, no-display is useful for calling refresh from outside of emacs, e.g.
from post-new hook in an automated fashion, so you can have an up-to-date
buffer when you switch to it. There's no point in displaying the refresh
message when you don't also switch to the buffer, is there? And this way
you'll get the diff between the manual (through user interaction)
refreshes of the buffer, not between two cron jobs.

Oh, I see.  This makes more sense.  The only use of no-display in the
notmuch code I can find appears to be to refresh the hello buffer when
you exit a search buffer started from hello, when it might actually
make sense to run the difference hook in your patch (I assume it wants
to avoid switching to the hello buffer in this case because the buffer
order may have changed?  Of course, there are other ways to get to the
hello buffer other than exiting a search buffer started from it...  I
found all of this code very confusing).

Maybe this just needs better documentation?
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch