[PATCH 0/4] contrib: pick: allow more general format lines

[PATCH 0/4] contrib: pick: allow more general format lines

[Mark Walters]

Currently pick makes the tree box graphics part of the "subject". This
is rather unsatisfactory: the tree graphics should be a field in their
own right.

However, there is no mechanism in the current setup for allowing 2
fields to have fixed combined width but variable individual
width. Thus making the tree graphics a seperate field means that we
cannot keep the current display which is 

date (12 characters) authors (20 characters) tree+subject (54 characters) tags

We solve this by extending the format specifier. Previously this was a
list of cons cells (field-name . format-string) We now allow the
left-side (the field-name) itself to be a list of cons cells in which case we
apply the formatting recursively.

This means we can separate the tree box graphics into their own field
while maintaining the current format.

Note that this will mean that people who have customised the
result-format will need to update their customisation.

The patch is larger but relatively simple.

Patch 1/4 is unrelated except I found it while doing this update: we
consistency we should print () for messages with no tags.

Patch 2/4 is large but trivial: it moves the insertion up a level in
preparation for the recursive formatting.

Patch 3/4 is also simple: it just allows the recursive formatting.

Patch 4/4 separates out the tree field. This patch is large as
it allows separate tree faces (matching and non-matching). The
"genuine code" part of the patch is relatively small.

Finally, this is the last series (apart from some extra tests and
keeping up with mainline progress) that I think pick needs before it's
ready for consideration in non-contrib mainline.

Best wishes

Mark



Mark Walters (4):
  contrib: pick: print () for a message with no tags
  contrib: pick: move the insertion of fields up a level
  contrib: pick: allow recursive message field formats
  contrib: pick: make the tree graphics a proper part of the format

 contrib/notmuch-pick/notmuch-pick.el |   93 ++++++++++++++++++++++++---------
 1 files changed, 67 insertions(+), 26 deletions(-)

-- 
1.7.9.1

[PATCH 1/4] contrib: pick: print () for a message with no tags

[Mark Walters]

Dating back to the earliest notmuch-pick we have not printed anything
for the tag field for a message with no tags. This is inconsistent
with search and show both of which print "()". Change pick to be
consistent.
---
 contrib/notmuch-pick/notmuch-pick.el |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/contrib/notmuch-pick/notmuch-pick.el b/contrib/notmuch-pick/notmuch-pick.el
index 37dc161..063d660 100644
--- a/contrib/notmuch-pick/notmuch-pick.el
+++ b/contrib/notmuch-pick/notmuch-pick.el
@@ -652,10 +652,9 @@ unchanged ADDRESS if parsing fails."
 	    (face (if match
 			  'notmuch-pick-match-tag-face
 			'notmuch-pick-no-match-tag-face)))
-	(when tags
-	  (insert (propertize (format format-string
-				      (mapconcat #'identity tags ", "))
-			      'face face))))))))
+	(insert (propertize (format format-string
+				    (mapconcat #'identity tags ", "))
+			    'face face)))))))
 
 (defun notmuch-pick-insert-msg (msg)
   "Insert the message MSG according to notmuch-pick-result-format"
-- 
1.7.9.1

[PATCH 2/4] contrib: pick: move the insertion of fields up a level

[Mark Walters]

This moves the actual insertion of message fields up from the field
formatting function into the message insertion function. This will be
useful in the next patch as we can apply further formatting to the
insertion string before inserting.
---
 contrib/notmuch-pick/notmuch-pick.el |   41 +++++++++++++++++++--------------
 1 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/contrib/notmuch-pick/notmuch-pick.el b/contrib/notmuch-pick/notmuch-pick.el
index 063d660..f01be94 100644
--- a/contrib/notmuch-pick/notmuch-pick.el
+++ b/contrib/notmuch-pick/notmuch-pick.el
@@ -610,16 +610,19 @@ unchanged ADDRESS if parsing fails."
     ;; If we have a name return that otherwise return the address.
     (or p-name p-address)))
 
-(defun notmuch-pick-insert-field (field format-string msg)
+(defun notmuch-pick-format-field (field format-string msg)
+  "Format a FIELD of MSG according to FORMAT-STRING and return string"
   (let* ((headers (plist-get msg :headers))
-	(match (plist-get msg :match)))
+	(match (plist-get msg :match))
+	formatted-field)
     (cond
      ((string-equal field "date")
       (let ((face (if match
 		      'notmuch-pick-match-date-face
 		    'notmuch-pick-no-match-date-face)))
-	(insert (propertize (format format-string (plist-get msg :date_relative))
-			    'face face))))
+	(setq formatted-field
+	      (propertize (format format-string (plist-get msg :date_relative))
+			  'face face))))
 
      ((string-equal field "subject")
       (let ((tree-status (plist-get msg :tree-status))
@@ -627,13 +630,14 @@ unchanged ADDRESS if parsing fails."
 	    (face (if match
 		      'notmuch-pick-match-subject-face
 		    'notmuch-pick-no-match-subject-face)))
-	(insert (propertize (format format-string
-				    (concat
-				     (mapconcat #'identity (reverse tree-status) "")
-				     (if (string= notmuch-pick-previous-subject bare-subject)
-					 " ..."
-				       bare-subject)))
-			    'face face))
+	(setq formatted-field
+	      (propertize (format format-string
+				  (concat
+				   (mapconcat #'identity (reverse tree-status) "")
+				   (if (string= notmuch-pick-previous-subject bare-subject)
+				       " ..."
+				     bare-subject)))
+			  'face face))
 	(setq notmuch-pick-previous-subject bare-subject)))
 
      ((string-equal field "authors")
@@ -644,17 +648,20 @@ unchanged ADDRESS if parsing fails."
 		    'notmuch-pick-no-match-author-face)))
 	(when (> (length author) len)
 	  (setq author (substring author 0 len)))
-	(insert (propertize (format format-string author)
-			    'face face))))
+	(setq formatted-field
+	      (propertize (format format-string author)
+			  'face face))))
 
      ((string-equal field "tags")
       (let ((tags (plist-get msg :tags))
 	    (face (if match
 			  'notmuch-pick-match-tag-face
 			'notmuch-pick-no-match-tag-face)))
-	(insert (propertize (format format-string
-				    (mapconcat #'identity tags ", "))
-			    'face face)))))))
+	(setq formatted-field
+	      (propertize (format format-string
+				  (mapconcat #'identity tags ", "))
+			  'face face)))))
+    formatted-field))
 
 (defun notmuch-pick-insert-msg (msg)
   "Insert the message MSG according to notmuch-pick-result-format"
@@ -662,7 +669,7 @@ unchanged ADDRESS if parsing fails."
   ;; by the insert-field calls.
   (let ((previous-subject notmuch-pick-previous-subject))
     (dolist (spec notmuch-pick-result-format)
-      (notmuch-pick-insert-field (car spec) (cdr spec) msg))
+      (insert (notmuch-pick-format-field (car spec) (cdr spec) msg)))
     (notmuch-pick-set-message-properties msg)
     (notmuch-pick-set-prop :previous-subject previous-subject)
     (insert "\n")))
-- 
1.7.9.1

[PATCH 3/4] contrib: pick: allow recursive message field formats

[Mark Walters]

Previously, the message format was fixed: each part had to be a
certain width and either left or right justified. This allows the user
to specify that two parts can be variable width but that combined they
should be some fixed width. We do this by allowing the user to set as
a "field" a list of the normal result-format form which is formatted
and then itself inserted according to the format string specified.

This means all existing formats work but allows more general things
too. This will be used in the next patch to allow the user to specify
where the tree box graphics are drawn but allow, e.g., the total width
of the tree box graphics and subject to be specified.
---
 contrib/notmuch-pick/notmuch-pick.el |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/contrib/notmuch-pick/notmuch-pick.el b/contrib/notmuch-pick/notmuch-pick.el
index f01be94..5dfcef3 100644
--- a/contrib/notmuch-pick/notmuch-pick.el
+++ b/contrib/notmuch-pick/notmuch-pick.el
@@ -616,6 +616,10 @@ unchanged ADDRESS if parsing fails."
 	(match (plist-get msg :match))
 	formatted-field)
     (cond
+     ((listp field)
+      (setq formatted-field
+	    (format format-string (notmuch-pick-format-field-list field msg))))
+
      ((string-equal field "date")
       (let ((face (if match
 		      'notmuch-pick-match-date-face
@@ -663,13 +667,19 @@ unchanged ADDRESS if parsing fails."
 			  'face face)))))
     formatted-field))
 
+(defun notmuch-pick-format-field-list (field-list msg)
+  "Format fields of MSG according to FIELD-LIST and return string"
+  (let (result-string)
+    (dolist (spec field-list result-string)
+      (let ((field-string (notmuch-pick-format-field (car spec) (cdr spec) msg)))
+	(setq result-string (concat result-string field-string))))))
+
 (defun notmuch-pick-insert-msg (msg)
   "Insert the message MSG according to notmuch-pick-result-format"
   ;; We need to save the previous subject as it will get overwritten
   ;; by the insert-field calls.
   (let ((previous-subject notmuch-pick-previous-subject))
-    (dolist (spec notmuch-pick-result-format)
-      (insert (notmuch-pick-format-field (car spec) (cdr spec) msg)))
+    (insert (notmuch-pick-format-field-list notmuch-pick-result-format msg))
     (notmuch-pick-set-message-properties msg)
     (notmuch-pick-set-prop :previous-subject previous-subject)
     (insert "\n")))
-- 
1.7.9.1

[PATCH 4/4] contrib: pick: make the tree graphics a proper part of the format

[Mark Walters]

Previously the box graphics in the pick view were always attached to
the subject. Make them a field in their own right. We use the
recursive insert to change the default notmuch-pick-result-format so
that the user view does not change. (The subject touches the tree box
graphics but the next column (tags) is still vertically aligned.)
---
 contrib/notmuch-pick/notmuch-pick.el |   49 +++++++++++++++++++++++++--------
 1 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/contrib/notmuch-pick/notmuch-pick.el b/contrib/notmuch-pick/notmuch-pick.el
index 5dfcef3..b6f7362 100644
--- a/contrib/notmuch-pick/notmuch-pick.el
+++ b/contrib/notmuch-pick/notmuch-pick.el
@@ -65,13 +65,18 @@
 (defcustom notmuch-pick-result-format
   `(("date" . "%12s  ")
     ("authors" . "%-20s")
-    ("subject" . " %-54s ")
+    ((("tree" . "%s")("subject" . "%s")) ." %-54s ")
     ("tags" . "(%s)"))
   "Result formatting for Pick. Supported fields are: date,
-        authors, subject, tags Note: subject includes the tree
-        structure graphics, and the author string should not
-        contain whitespace (put it in the neighbouring fields
-        instead).  For example:
+        authors, subject, tree, tags.  Tree means the thread tree
+        box graphics. The field may also be a list in which case
+        the formatting rules are applied recursively and then the
+        output of all the fields in the list is inserted
+        according to format-string.
+
+Note the author string should not contain
+        whitespace (put it in the neighbouring fields instead).
+        For example:
         (setq notmuch-pick-result-format \(\(\"authors\" . \"%-40s\"\)
                                              \(\"subject\" . \"%s\"\)\)\)"
   :type '(alist :key-type (string) :value-type (string))
@@ -108,6 +113,12 @@
   :group 'notmuch-pick
   :group 'notmuch-faces)
 
+(defface notmuch-pick-match-tree-face
+  '((t :inherit default))
+  "Face used in pick mode for the thread tree block graphics in messages matching the query."
+  :group 'notmuch-pick
+  :group 'notmuch-faces)
+
 (defface notmuch-pick-match-tag-face
   '((((class color)
       (background dark))
@@ -134,6 +145,12 @@
   :group 'notmuch-pick
   :group 'notmuch-faces)
 
+(defface notmuch-pick-no-match-tree-face
+  '((t (:foreground "gray")))
+  "Face used in pick mode for the thread tree block graphics in messages matching the query."
+  :group 'notmuch-pick
+  :group 'notmuch-faces)
+
 (defface notmuch-pick-no-match-author-face
   '((t (:foreground "gray")))
   "Face used in pick mode for the date in messages matching the query."
@@ -628,19 +645,27 @@ unchanged ADDRESS if parsing fails."
 	      (propertize (format format-string (plist-get msg :date_relative))
 			  'face face))))
 
-     ((string-equal field "subject")
+     ((string-equal field "tree")
       (let ((tree-status (plist-get msg :tree-status))
-	    (bare-subject (notmuch-show-strip-re (plist-get headers :Subject)))
+	    (face (if match
+		      'notmuch-pick-match-tree-face
+		    'notmuch-pick-no-match-tree-face)))
+
+	(setq formatted-field
+	      (propertize (format format-string
+				  (mapconcat #'identity (reverse tree-status) ""))
+			  'face face))))
+
+     ((string-equal field "subject")
+      (let ((bare-subject (notmuch-show-strip-re (plist-get headers :Subject)))
 	    (face (if match
 		      'notmuch-pick-match-subject-face
 		    'notmuch-pick-no-match-subject-face)))
 	(setq formatted-field
 	      (propertize (format format-string
-				  (concat
-				   (mapconcat #'identity (reverse tree-status) "")
-				   (if (string= notmuch-pick-previous-subject bare-subject)
-				       " ..."
-				     bare-subject)))
+				  (if (string= notmuch-pick-previous-subject bare-subject)
+				      " ..."
+				    bare-subject))
 			  'face face))
 	(setq notmuch-pick-previous-subject bare-subject)))
 
-- 
1.7.9.1

Re: [PATCH 0/4] contrib: pick: allow more general format lines

[Mark Walters]

On Mon, 02 Sep 2013, Mark Walters <markwalters1009@gmail.com> wrote:
> Currently pick makes the tree box graphics part of the "subject". This
> is rather unsatisfactory: the tree graphics should be a field in their
> own right.
>
> However, there is no mechanism in the current setup for allowing 2
> fields to have fixed combined width but variable individual
> width. Thus making the tree graphics a seperate field means that we
> cannot keep the current display which is 
>
> date (12 characters) authors (20 characters) tree+subject (54 characters) tags
>
> We solve this by extending the format specifier. Previously this was a
> list of cons cells (field-name . format-string) We now allow the
> left-side (the field-name) itself to be a list of cons cells in which case we
> apply the formatting recursively.
>
> This means we can separate the tree box graphics into their own field
> while maintaining the current format.
>
> Note that this will mean that people who have customised the
> result-format will need to update their customisation.
>
> The patch is larger but relatively simple.
>
> Patch 1/4 is unrelated except I found it while doing this update: we
> consistency we should print () for messages with no tags.
>
> Patch 2/4 is large but trivial: it moves the insertion up a level in
> preparation for the recursive formatting.
>
> Patch 3/4 is also simple: it just allows the recursive formatting.
>
> Patch 4/4 separates out the tree field. This patch is large as
> it allows separate tree faces (matching and non-matching). The
> "genuine code" part of the patch is relatively small.

I should have added that everything works at each intermediate stage
(and I think each patch makes sense incrementally): so
the patches do not need to be reviewed as a batch.

Best wishes

Mark

>
> Finally, this is the last series (apart from some extra tests and
> keeping up with mainline progress) that I think pick needs before it's
> ready for consideration in non-contrib mainline.
>
> Best wishes
>
> Mark
>
>
>
> Mark Walters (4):
>   contrib: pick: print () for a message with no tags
>   contrib: pick: move the insertion of fields up a level
>   contrib: pick: allow recursive message field formats
>   contrib: pick: make the tree graphics a proper part of the format
>
>  contrib/notmuch-pick/notmuch-pick.el |   93 ++++++++++++++++++++++++---------
>  1 files changed, 67 insertions(+), 26 deletions(-)
>
> -- 
> 1.7.9.1

Re: [PATCH 0/4] contrib: pick: allow more general format lines

[Tomi Ollila]

On Mon, Sep 02 2013, Mark Walters <markwalters1009@gmail.com> wrote:

> Currently pick makes the tree box graphics part of the "subject". This
> is rather unsatisfactory: the tree graphics should be a field in their
> own right.
>
> However, there is no mechanism in the current setup for allowing 2
> fields to have fixed combined width but variable individual
> width. Thus making the tree graphics a seperate field means that we
> cannot keep the current display which is 
>
> date (12 characters) authors (20 characters) tree+subject (54 characters) tags
>
> We solve this by extending the format specifier. Previously this was a
> list of cons cells (field-name . format-string) We now allow the
> left-side (the field-name) itself to be a list of cons cells in which case we
> apply the formatting recursively.
>
> This means we can separate the tree box graphics into their own field
> while maintaining the current format.
>
> Note that this will mean that people who have customised the
> result-format will need to update their customisation.
>
> The patch is larger but relatively simple.
>
> Patch 1/4 is unrelated except I found it while doing this update: we
> consistency we should print () for messages with no tags.
>
> Patch 2/4 is large but trivial: it moves the insertion up a level in
> preparation for the recursive formatting.
>
> Patch 3/4 is also simple: it just allows the recursive formatting.
>
> Patch 4/4 separates out the tree field. This patch is large as
> it allows separate tree faces (matching and non-matching). The
> "genuine code" part of the patch is relatively small.
>
> Finally, this is the last series (apart from some extra tests and
> keeping up with mainline progress) that I think pick needs before it's
> ready for consideration in non-contrib mainline.

This series LGTM. One thing that I saw could be "improved" a bit:
in notmuch-pick-format-field drop the formatted-field parameter
and setq's to it. Something that can be done in follow-up patch
if there is nothing else to be changed. I don't want to have any
more review effort on this patchset due to that change :D

+1

>
> Best wishes
>
> Mark

Tomi


>
>
>
> Mark Walters (4):
>   contrib: pick: print () for a message with no tags
>   contrib: pick: move the insertion of fields up a level
>   contrib: pick: allow recursive message field formats
>   contrib: pick: make the tree graphics a proper part of the format
>
>  contrib/notmuch-pick/notmuch-pick.el |   93 ++++++++++++++++++++++++---------
>  1 files changed, 67 insertions(+), 26 deletions(-)
>
> -- 
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 0/4] contrib: pick: allow more general format lines

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> Currently pick makes the tree box graphics part of the "subject". This
> is rather unsatisfactory: the tree graphics should be a field in their
> own right.
>

series pushed

d