unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH 0/7] emacs: show tag changes in buffer
@ 2014-01-18 23:30 Mark Walters
  2014-01-18 23:30 ` [PATCH 1/7] emacs: tag split customise option for format-tags into a widget Mark Walters
                   ` (8 more replies)
  0 siblings, 9 replies; 16+ messages in thread
From: Mark Walters @ 2014-01-18 23:30 UTC (permalink / raw)
  To: notmuch

This is a reworked and expanded version of the single patch
id:1387065197-15776-4-git-send-email-markwalters1009@gmail.com

This shows any tags changed in the show buffer since it was loaded or
refreshed. By default a removed tag is displayed with strike-through
in red (if strike-through is not available, eg on a terminal, inverse
video is used instead) and an added tag is displayed underlined in
green. This version works in all three views show, search and tree.

My motivation for this is part of a rework of the unread handling: we
can do a lot more automatically if we make it clear to the user what
we have done (which this does). It also make automatic updating of
buffers on tag-changes such as the search buffer showing tag changes
done in the show buffer a sensible possibility (as the user does not
lose information)

The patch series is large but I have tried to split it up into small pieces

Patches 1-3 are preparation mostly adding defcustoms for the format
for added or deleted tags.

Patch 4 does all the actual logic to allow added and deleted tags to be shown.

Patches 5-7 make show/search and tree keep track of the original tags
to use the functionality in Patch 4.

I have been running this series (with the unread and auto-updating
described above) for about a month and it seems to work well for my use.

Best wishes

Mark



Mark Walters (7):
  emacs: tag split customise option for format-tags into a widget
  emacs: tag: allow default case in notmuch-tag-formats
  emacs: tag: add customize for deleted/added tag formats
  emacs: show: mark tags changed since buffer loaded
  emacs: show: use orig-tags for tag display
  emacs: search: use orig-tags in search
  emacs: tree: use orig-tags in search

 emacs/notmuch-show.el |    6 ++-
 emacs/notmuch-tag.el  |  116 ++++++++++++++++++++++++++++++++++--------------
 emacs/notmuch-tree.el |    9 +++-
 emacs/notmuch.el      |   12 ++++-
 test/test-lib.el      |    4 ++
 5 files changed, 105 insertions(+), 42 deletions(-)

-- 
1.7.9.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/7] emacs: tag split customise option for format-tags into a widget
  2014-01-18 23:30 [PATCH 0/7] emacs: show tag changes in buffer Mark Walters
@ 2014-01-18 23:30 ` Mark Walters
  2014-01-18 23:30 ` [PATCH 2/7] emacs: tag: allow default case in notmuch-tag-formats Mark Walters
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Mark Walters @ 2014-01-18 23:30 UTC (permalink / raw)
  To: notmuch

We will re-use the customize option for format-tags for formattting
deleted tags to added tags in the next patch so split it into a
widget. There should be no functional change.
---
 emacs/notmuch-tag.el |   55 ++++++++++++++++++++++++++-----------------------
 1 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
index 908e7ad..2153068 100644
--- a/emacs/notmuch-tag.el
+++ b/emacs/notmuch-tag.el
@@ -28,6 +28,34 @@
 (require 'crm)
 (require 'notmuch-lib)
 
+(define-widget 'notmuch-tag-format-type 'lazy
+  "Customize widget for notmuch-tag-format and friends"
+  :type '(alist :key-type (string :tag "Tag")
+		:extra-offset -3
+		:value-type
+		(radio :format "%v"
+		       (const :tag "Hidden" nil)
+		       (set :tag "Modified"
+			    (string :tag "Display as")
+			    (list :tag "Face" :extra-offset -4
+				  (const :format "" :inline t
+					 (propertize tag 'face))
+				  (list :format "%v"
+					(const :format "" quote)
+					custom-face-edit))
+			    (list :format "%v" :extra-offset -4
+				  (const :format "" :inline t
+					 (notmuch-tag-format-image-data tag))
+				  (choice :tag "Image"
+					  (const :tag "Star"
+						 (notmuch-tag-star-icon))
+					  (const :tag "Empty star"
+						 (notmuch-tag-star-empty-icon))
+					  (const :tag "Tag"
+						 (notmuch-tag-tag-icon))
+					  (string :tag "Custom")))
+			    (sexp :tag "Custom")))))
+
 (defcustom notmuch-tag-formats
   '(("unread" (propertize tag 'face '(:foreground "red")))
     ("flagged" (propertize tag 'face '(:foreground "blue"))
@@ -53,34 +81,9 @@ of a tag to red, use the expression
 
 See also `notmuch-tag-format-image', which can help replace tags
 with images."
-
   :group 'notmuch-search
   :group 'notmuch-show
-  :type '(alist :key-type (string :tag "Tag")
-		:extra-offset -3
-		:value-type
-		(radio :format "%v"
-		       (const :tag "Hidden" nil)
-		       (set :tag "Modified"
-			    (string :tag "Display as")
-			    (list :tag "Face" :extra-offset -4
-				  (const :format "" :inline t
-					 (propertize tag 'face))
-				  (list :format "%v"
-					(const :format "" quote)
-					custom-face-edit))
-			    (list :format "%v" :extra-offset -4
-				  (const :format "" :inline t
-					 (notmuch-tag-format-image-data tag))
-				  (choice :tag "Image"
-					  (const :tag "Star"
-						 (notmuch-tag-star-icon))
-					  (const :tag "Empty star"
-						 (notmuch-tag-star-empty-icon))
-					  (const :tag "Tag"
-						 (notmuch-tag-tag-icon))
-					  (string :tag "Custom")))
-			    (sexp :tag "Custom")))))
+  :type 'notmuch-tag-format-type)
 
 (defun notmuch-tag-format-image-data (tag data)
   "Replace TAG with image DATA, if available.
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/7] emacs: tag: allow default case in notmuch-tag-formats
  2014-01-18 23:30 [PATCH 0/7] emacs: show tag changes in buffer Mark Walters
  2014-01-18 23:30 ` [PATCH 1/7] emacs: tag split customise option for format-tags into a widget Mark Walters
@ 2014-01-18 23:30 ` Mark Walters
  2014-02-10 18:19   ` Austin Clements
  2014-01-18 23:30 ` [PATCH 3/7] emacs: tag: add customize for deleted/added tag formats Mark Walters
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Mark Walters @ 2014-01-18 23:30 UTC (permalink / raw)
  To: notmuch

Allow an empty string in notmuch-tag-formats which matches all tags
except those matched explicitly matched. This allows the user to tell
notmuch to hide all tags except those specified.

This will be useful once formatting for deleted/added tags is added
later in the series: a user might want to hide all deleted tags for
example.
---
 emacs/notmuch-tag.el |   20 +++++++++++---------
 1 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
index 2153068..92c1249 100644
--- a/emacs/notmuch-tag.el
+++ b/emacs/notmuch-tag.el
@@ -65,14 +65,15 @@
 This gives a list that maps from tag names to lists of formatting
 expressions.  The car of each element gives a tag name and the
 cdr gives a list of Elisp expressions that modify the tag.  If
-the list is empty, the tag will simply be hidden.  Otherwise,
-each expression will be evaluated in order: for the first
-expression, the variable `tag' will be bound to the tag name; for
-each later expression, the variable `tag' will be bound to the
-result of the previous expression.  In this way, each expression
-can build on the formatting performed by the previous expression.
-The result of the last expression will displayed in place of the
-tag.
+the car is an empty string it matches all tags that do not have
+an explicit match.  If the list is empty, the tag will simply be
+hidden.  Otherwise, each expression will be evaluated in order:
+for the first expression, the variable `tag' will be bound to the
+tag name; for each later expression, the variable `tag' will be
+bound to the result of the previous expression.  In this way,
+each expression can build on the formatting performed by the
+previous expression.  The result of the last expression will
+displayed in place of the tag.
 
 For example, to replace a tag with another string, simply use
 that string as a formatting expression.  To change the foreground
@@ -140,7 +141,8 @@ This can be used with `notmuch-tag-format-image-data'."
 
 (defun notmuch-tag-format-tag (tag)
   "Format TAG by looking into `notmuch-tag-formats'."
-  (let ((formats (assoc tag notmuch-tag-formats)))
+  (let ((formats (or (assoc tag notmuch-tag-formats)
+		     (assoc "" notmuch-tag-formats))))
     (cond
      ((null formats)		;; - Tag not in `notmuch-tag-formats',
       tag)			;;   the format is the tag itself.
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/7] emacs: tag: add customize for deleted/added tag formats
  2014-01-18 23:30 [PATCH 0/7] emacs: show tag changes in buffer Mark Walters
  2014-01-18 23:30 ` [PATCH 1/7] emacs: tag split customise option for format-tags into a widget Mark Walters
  2014-01-18 23:30 ` [PATCH 2/7] emacs: tag: allow default case in notmuch-tag-formats Mark Walters
@ 2014-01-18 23:30 ` Mark Walters
  2014-01-18 23:30 ` [PATCH 4/7] emacs: show: mark tags changed since buffer loaded Mark Walters
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Mark Walters @ 2014-01-18 23:30 UTC (permalink / raw)
  To: notmuch

Add customize options for deleted/added tag formats.  These are not
used yet but will be later in the series.
---
 emacs/notmuch-tag.el |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
index 92c1249..9757a0e 100644
--- a/emacs/notmuch-tag.el
+++ b/emacs/notmuch-tag.el
@@ -86,6 +86,31 @@ with images."
   :group 'notmuch-show
   :type 'notmuch-tag-format-type)
 
+(defcustom notmuch-tag-deleted-formats
+  '(("" (propertize tag 'face
+		   (if (display-supports-face-attributes-p '(:strike-through "red"))
+		       '(:strike-through "red")
+		     '(:inverse-video t)))))
+  "Custom formats for tags when deleted.
+
+By default this shows deleted tags with strike-through in red,
+unless strike-through is not available (e.g., emacs is running in
+a terminal) in which case it uses inverse video. To hide deleted
+tags completely set this to
+  '((\"\" nil))
+
+See `notmuch-tag-formats' for full documentation."
+  :group 'notmuch-show
+  :type 'notmuch-tag-format-type)
+
+(defcustom notmuch-tag-added-formats
+  '(("" (propertize tag 'face '(:underline "green"))))
+  "Custom formats for tags when added.
+
+See `notmuch-tag-formats' for full documentation."
+  :group 'notmuch-show
+  :type 'notmuch-tag-format-type)
+
 (defun notmuch-tag-format-image-data (tag data)
   "Replace TAG with image DATA, if available.
 
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/7] emacs: show: mark tags changed since buffer loaded
  2014-01-18 23:30 [PATCH 0/7] emacs: show tag changes in buffer Mark Walters
                   ` (2 preceding siblings ...)
  2014-01-18 23:30 ` [PATCH 3/7] emacs: tag: add customize for deleted/added tag formats Mark Walters
@ 2014-01-18 23:30 ` Mark Walters
  2014-02-12  1:21   ` Austin Clements
  2014-01-18 23:30 ` [PATCH 5/7] emacs: show: use orig-tags for tag display Mark Walters
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Mark Walters @ 2014-01-18 23:30 UTC (permalink / raw)
  To: notmuch

This allows (and requires) the original-tags to be passed along with
the current-tags to be passed to notmuch-tag-format-tags. This allows
the tag formatting to show added and deleted tags.By default a removed
tag is displayed with strike-through in red (if strike-through is not
available, eg on a terminal, inverse video is used instead) and an
added tag is displayed underlined in green.

If the caller does not wish to use the new feature it can pass
current-tags for both arguments and, at this point, we do exactly that
in the three callers of this function.

Note, we cannot tidily allow original-tags to be optional because we would
need to distinguish nil meaning "we are not specifying original-tags"
from nil meaning there were no original-tags (an empty list).

We use this in subsequent patches to make it clear when a message was
unread when you first loaded a show buffer (previously the unread tag
could be removed before a user realised that it had been unread).

The code adds into the existing tag formatting code. The user can
specify exactly how a tag should be displayed normally, when deleted,
or when added. For convenience an entry for the empty string in the
notmuch-tag-formats (and the corresponding notmuch-tag-deleted-formats
notmuch-tag-added-formats) is applied to all tags which do not have an
explicit match.

This means that a user can tell notmuch not to show deleted tags at
all by setting notmuch-tag-deleted-formats to
'(("" nil))
or not to show any deleted tags except "unread" by setting it to
'(("" nil)
  ("unread" (propertize tag 'face '(strike-through "red"))))

All the variables are customizable; however, more complicated cases
like changing the face depending on the type of display will require
custom lisp.

Currently this overrides notmuch-tag-deleted-formats for the tests
setting it to '(("" nil)) so that they get removed from the display
and, thus, all tests still pass.
---
 emacs/notmuch-show.el |    4 ++--
 emacs/notmuch-tag.el  |   32 +++++++++++++++++++++++++-------
 emacs/notmuch-tree.el |    2 +-
 emacs/notmuch.el      |    2 +-
 test/test-lib.el      |    4 ++++
 5 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 1ac80ca..1ce56f9 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -344,7 +344,7 @@ operation on the contents of the current buffer."
     (if (re-search-forward "(\\([^()]*\\))$" (line-end-position) t)
 	(let ((inhibit-read-only t))
 	  (replace-match (concat "("
-				 (notmuch-tag-format-tags tags)
+				 (notmuch-tag-format-tags tags tags)
 				 ")"))))))
 
 (defun notmuch-clean-address (address)
@@ -423,7 +423,7 @@ message at DEPTH in the current thread."
 	    " ("
 	    date
 	    ") ("
-	    (notmuch-tag-format-tags tags)
+	    (notmuch-tag-format-tags tags tags)
 	    ")\n")
     (overlay-put (make-overlay start (point)) 'face 'notmuch-message-summary-face)))
 
diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
index 9757a0e..6bc47fb 100644
--- a/emacs/notmuch-tag.el
+++ b/emacs/notmuch-tag.el
@@ -164,10 +164,25 @@ This can be used with `notmuch-tag-format-image-data'."
   </g>
 </svg>")
 
-(defun notmuch-tag-format-tag (tag)
-  "Format TAG by looking into `notmuch-tag-formats'."
-  (let ((formats (or (assoc tag notmuch-tag-formats)
-		     (assoc "" notmuch-tag-formats))))
+(defun notmuch-tag-format-tag (tags orig-tags tag)
+  "Format TAG by looking into `notmuch-tag-formats'.
+
+TAGS and ORIG-TAGS are lists of the current tags and the original
+tags; tags which have been deleted (i.e., are in ORIG-TAGS but
+are not in TAGS) are shown using formats from
+`notmuch-tag-deleted-formats'; tags which have been added (i.e.,
+are in TAGS but are not in ORIG-TAGS) are shown using formats
+from `notmuch-tag-added-formats' and tags which have not been
+changed (the normal case) are shown using formats from
+`notmuch-tag-formats'"
+  (let* ((status-formats (cond ((and (member tag tags) (member tag orig-tags))
+				notmuch-tag-formats)
+			       ((not (member tag tags))
+				notmuch-tag-deleted-formats)
+			       ((not (member tag orig-tags))
+				notmuch-tag-added-formats)))
+	 (formats (or (assoc tag status-formats)
+		      (assoc "" status-formats))))
     (cond
      ((null formats)		;; - Tag not in `notmuch-tag-formats',
       tag)			;;   the format is the tag itself.
@@ -178,13 +193,16 @@ This can be used with `notmuch-tag-format-image-data'."
 	(dolist (format (cdr formats) tag)
 	  (setq tag (eval format))))))))
 
-(defun notmuch-tag-format-tags (tags &optional face)
+(defun notmuch-tag-format-tags (tags orig-tags &optional face)
   "Return a string representing formatted TAGS."
-  (let ((face (or face 'notmuch-tag-face)))
+  (let ((face (or face 'notmuch-tag-face))
+	(all-tags (sort (delete-dups (append tags orig-tags)) #'string<)))
     (notmuch-combine-face-text-property-string
      (mapconcat #'identity
 		;; nil indicated that the tag was deliberately hidden
-		(delq nil (mapcar #'notmuch-tag-format-tag tags))
+		(delq nil (mapcar
+			   (apply-partially #'notmuch-tag-format-tag tags orig-tags)
+			   all-tags))
 		" ")
      face
      t)))
diff --git a/emacs/notmuch-tree.el b/emacs/notmuch-tree.el
index 4f2ac02..b37e2cd 100644
--- a/emacs/notmuch-tree.el
+++ b/emacs/notmuch-tree.el
@@ -704,7 +704,7 @@ unchanged ADDRESS if parsing fails."
 	    (face (if match
 		      'notmuch-tree-match-tag-face
 		    'notmuch-tree-no-match-tag-face)))
-	(format format-string (notmuch-tag-format-tags tags face)))))))
+	(format format-string (notmuch-tag-format-tags tags tags face)))))))
 
 (defun notmuch-tree-format-field-list (field-list msg)
   "Format fields of MSG according to FIELD-LIST and return string"
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 0471750..1436e5a 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -754,7 +754,7 @@ non-authors is found, assume that all of the authors match."
 
    ((string-equal field "tags")
     (let ((tags (plist-get result :tags)))
-      (insert (format format-string (notmuch-tag-format-tags tags)))))))
+      (insert (format format-string (notmuch-tag-format-tags tags tags)))))))
 
 (defun notmuch-search-show-result (result &optional pos)
   "Insert RESULT at POS or the end of the buffer if POS is null."
diff --git a/test/test-lib.el b/test/test-lib.el
index 37fcb3d..6cbd57c 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -165,3 +165,7 @@ nothing."
 
      (t
       (notmuch-test-report-unexpected output expected)))))
+
+;; hide deleted tags
+(setq notmuch-tag-deleted-formats
+      '(("" nil)))
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/7] emacs: show: use orig-tags for tag display
  2014-01-18 23:30 [PATCH 0/7] emacs: show tag changes in buffer Mark Walters
                   ` (3 preceding siblings ...)
  2014-01-18 23:30 ` [PATCH 4/7] emacs: show: mark tags changed since buffer loaded Mark Walters
@ 2014-01-18 23:30 ` Mark Walters
  2014-01-18 23:30 ` [PATCH 6/7] emacs: search: use orig-tags in search Mark Walters
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Mark Walters @ 2014-01-18 23:30 UTC (permalink / raw)
  To: notmuch

This uses the previous patch to show the tag changes that have occured
in the show buffer since it was last loaded/refreshed.
---
 emacs/notmuch-show.el |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 1ce56f9..97243dc 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -344,7 +344,7 @@ operation on the contents of the current buffer."
     (if (re-search-forward "(\\([^()]*\\))$" (line-end-position) t)
 	(let ((inhibit-read-only t))
 	  (replace-match (concat "("
-				 (notmuch-tag-format-tags tags tags)
+				 (notmuch-tag-format-tags tags (notmuch-show-get-prop :orig-tags))
 				 ")"))))))
 
 (defun notmuch-clean-address (address)
@@ -1167,6 +1167,8 @@ function is used."
 
       (jit-lock-register #'notmuch-show-buttonise-links)
 
+      (notmuch-show-mapc (lambda () (notmuch-show-set-prop :orig-tags (notmuch-show-get-tags))))
+
       ;; Set the header line to the subject of the first message.
       (setq header-line-format (notmuch-sanitize (notmuch-show-strip-re (notmuch-show-get-subject))))
 
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 6/7] emacs: search: use orig-tags in search
  2014-01-18 23:30 [PATCH 0/7] emacs: show tag changes in buffer Mark Walters
                   ` (4 preceding siblings ...)
  2014-01-18 23:30 ` [PATCH 5/7] emacs: show: use orig-tags for tag display Mark Walters
@ 2014-01-18 23:30 ` Mark Walters
  2014-02-12  1:30   ` Austin Clements
  2014-01-18 23:30 ` [PATCH 7/7] emacs: tree: " Mark Walters
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Mark Walters @ 2014-01-18 23:30 UTC (permalink / raw)
  To: notmuch

This uses the recent functionality to show the tag changes in the
search buffer. Currently this is only used to show changes the search
buffer makes itself: i.e., it does not make display any changes
reflecting tagging done by other notmuch-buffers.
---
 emacs/notmuch.el |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 1436e5a..f0ea5d4 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -753,14 +753,20 @@ non-authors is found, assume that all of the authors match."
      format-string (notmuch-sanitize (plist-get result :authors))))
 
    ((string-equal field "tags")
-    (let ((tags (plist-get result :tags)))
-      (insert (format format-string (notmuch-tag-format-tags tags tags)))))))
+    (let ((tags (plist-get result :tags))
+	  (orig-tags (plist-get result :orig-tags)))
+      (insert (format format-string (notmuch-tag-format-tags tags orig-tags)))))))
 
 (defun notmuch-search-show-result (result &optional pos)
   "Insert RESULT at POS or the end of the buffer if POS is null."
   ;; Ignore excluded matches
   (unless (= (plist-get result :matched) 0)
-    (let ((beg (or pos (point-max))))
+    (let ((beg (or pos (point-max)))
+	  ;; If we are inserting the result for the first time POS
+	  ;; will be nil and we should set orig-tags.
+	  (result (if pos
+		      result
+		    (plist-put result :orig-tags (plist-get result :tags)))))
       (save-excursion
 	(goto-char beg)
 	(dolist (spec notmuch-search-result-format)
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 7/7] emacs: tree: use orig-tags in search
  2014-01-18 23:30 [PATCH 0/7] emacs: show tag changes in buffer Mark Walters
                   ` (5 preceding siblings ...)
  2014-01-18 23:30 ` [PATCH 6/7] emacs: search: use orig-tags in search Mark Walters
@ 2014-01-18 23:30 ` Mark Walters
  2014-01-25 19:35 ` [PATCH 0/7] emacs: show tag changes in buffer Jani Nikula
  2014-02-05 22:25 ` Tomi Ollila
  8 siblings, 0 replies; 16+ messages in thread
From: Mark Walters @ 2014-01-18 23:30 UTC (permalink / raw)
  To: notmuch

This uses the recent functionality to show the tag changes in the tree
buffer. Currently this is only used to show changes the tree buffer
makes itself: i.e., it does not make display any changes reflecting
tagging done by other notmuch-buffers.
---
 emacs/notmuch-tree.el |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch-tree.el b/emacs/notmuch-tree.el
index b37e2cd..0f4cd47 100644
--- a/emacs/notmuch-tree.el
+++ b/emacs/notmuch-tree.el
@@ -701,10 +701,11 @@ unchanged ADDRESS if parsing fails."
 
      ((string-equal field "tags")
       (let ((tags (plist-get msg :tags))
+	    (orig-tags (plist-get msg :orig-tags))
 	    (face (if match
 		      'notmuch-tree-match-tag-face
 		    'notmuch-tree-no-match-tag-face)))
-	(format format-string (notmuch-tag-format-tags tags tags face)))))))
+	(format format-string (notmuch-tag-format-tags tags orig-tags face)))))))
 
 (defun notmuch-tree-format-field-list (field-list msg)
   "Format fields of MSG according to FIELD-LIST and return string"
@@ -766,8 +767,10 @@ message together with all its descendents."
 	(push "├" tree-status)))
 
       (push (concat (if replies "┬" "─") "►") tree-status)
-      (plist-put msg :first (and first (eq 0 depth)))
-      (notmuch-tree-goto-and-insert-msg (plist-put msg :tree-status tree-status))
+      (setq msg (plist-put msg :first (and first (eq 0 depth))))
+      (setq msg (plist-put msg :tree-status tree-status))
+      (setq msg (plist-put msg :orig-tags (plist-get msg :tags)))
+      (notmuch-tree-goto-and-insert-msg msg)
       (pop tree-status)
       (pop tree-status)
 
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/7] emacs: show tag changes in buffer
  2014-01-18 23:30 [PATCH 0/7] emacs: show tag changes in buffer Mark Walters
                   ` (6 preceding siblings ...)
  2014-01-18 23:30 ` [PATCH 7/7] emacs: tree: " Mark Walters
@ 2014-01-25 19:35 ` Jani Nikula
  2014-02-05 22:25 ` Tomi Ollila
  8 siblings, 0 replies; 16+ messages in thread
From: Jani Nikula @ 2014-01-25 19:35 UTC (permalink / raw)
  To: Mark Walters, notmuch

On Sun, 19 Jan 2014, Mark Walters <markwalters1009@gmail.com> wrote:
> I have been running this series (with the unread and auto-updating
> described above) for about a month and it seems to work well for my use.

I've been running this for a while now, and I really like it. Haven't
noticed any issues. I'm anticipating the follow-up changing how we
decide a message is read (i.e. the rest of the series at [1]).

Unfortunately reviewing elisp is way beyond my bandwidth right now.

BR,
Jani.


[1] id:1387065197-15776-1-git-send-email-markwalters1009@gmail.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/7] emacs: show tag changes in buffer
  2014-01-18 23:30 [PATCH 0/7] emacs: show tag changes in buffer Mark Walters
                   ` (7 preceding siblings ...)
  2014-01-25 19:35 ` [PATCH 0/7] emacs: show tag changes in buffer Jani Nikula
@ 2014-02-05 22:25 ` Tomi Ollila
  8 siblings, 0 replies; 16+ messages in thread
From: Tomi Ollila @ 2014-02-05 22:25 UTC (permalink / raw)
  To: Mark Walters, notmuch

On Sun, Jan 19 2014, Mark Walters <markwalters1009@gmail.com> wrote:

> This is a reworked and expanded version of the single patch
> id:1387065197-15776-4-git-send-email-markwalters1009@gmail.com
>
> This shows any tags changed in the show buffer since it was loaded or
> refreshed. By default a removed tag is displayed with strike-through
> in red (if strike-through is not available, eg on a terminal, inverse
> video is used instead) and an added tag is displayed underlined in
> green. This version works in all three views show, search and tree.


tl;dr ... ;D -- well, maybe someone reads it through ;)

Anyway, this patch series looks good to me (one commit message
tl;dr but I guess it is useful for anyone who actually reads it).
I don't see anything it may break and I started using this in 
my notmuch setup from now on.

Tomi

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/7] emacs: tag: allow default case in notmuch-tag-formats
  2014-01-18 23:30 ` [PATCH 2/7] emacs: tag: allow default case in notmuch-tag-formats Mark Walters
@ 2014-02-10 18:19   ` Austin Clements
  2014-02-11 10:23     ` Mark Walters
  0 siblings, 1 reply; 16+ messages in thread
From: Austin Clements @ 2014-02-10 18:19 UTC (permalink / raw)
  To: Mark Walters, notmuch

On Sat, 18 Jan 2014, Mark Walters <markwalters1009@gmail.com> wrote:
> Allow an empty string in notmuch-tag-formats which matches all tags
> except those matched explicitly matched. This allows the user to tell

Typo.

> notmuch to hide all tags except those specified.
>
> This will be useful once formatting for deleted/added tags is added
> later in the series: a user might want to hide all deleted tags for
> example.
> ---
>  emacs/notmuch-tag.el |   20 +++++++++++---------
>  1 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
> index 2153068..92c1249 100644
> --- a/emacs/notmuch-tag.el
> +++ b/emacs/notmuch-tag.el
> @@ -65,14 +65,15 @@
>  This gives a list that maps from tag names to lists of formatting
>  expressions.  The car of each element gives a tag name and the
>  cdr gives a list of Elisp expressions that modify the tag.  If
> -the list is empty, the tag will simply be hidden.  Otherwise,
> -each expression will be evaluated in order: for the first
> -expression, the variable `tag' will be bound to the tag name; for
> -each later expression, the variable `tag' will be bound to the
> -result of the previous expression.  In this way, each expression
> -can build on the formatting performed by the previous expression.
> -The result of the last expression will displayed in place of the
> -tag.
> +the car is an empty string it matches all tags that do not have
> +an explicit match.  If the list is empty, the tag will simply be

Hmm.  I'm not sure I like overloading of the meanings of strings.  Could
we instead use a symbol to represent this case?  For example, `t' would
parallel the fall-through case of `cond' and `case', or `_' would
parallel `pcase' [1].  Or even a separate variable like
notmuch-tag-default-format?

The former would require some tweaking of the customize widget, but that
should probably happen anyway to support this special case.
Unfortunately, we may need a custom alist widget variant to do that.  (I
tried and failed to tweak it in a way that both worked and looked
decent.)  Hence my suggestion of a separate variable, which would only
require pulling out the :value-type into its own define-widget.

I'm also slightly bothered that this would introduce a second way to
control the default formatting of tags in addition to notmuch-tag-face,
but only slightly.

[1] It's unfortunate that pcase wasn't introduced until Emacs 24.  I've
been tempted to backport it for notmuch multiple times now.  Then we
could just treat notmuch-tag-formats as a list of pcase conditions.

> +hidden.  Otherwise, each expression will be evaluated in order:
> +for the first expression, the variable `tag' will be bound to the
> +tag name; for each later expression, the variable `tag' will be
> +bound to the result of the previous expression.  In this way,
> +each expression can build on the formatting performed by the
> +previous expression.  The result of the last expression will
> +displayed in place of the tag.
>  
>  For example, to replace a tag with another string, simply use
>  that string as a formatting expression.  To change the foreground
> @@ -140,7 +141,8 @@ This can be used with `notmuch-tag-format-image-data'."
>  
>  (defun notmuch-tag-format-tag (tag)
>    "Format TAG by looking into `notmuch-tag-formats'."
> -  (let ((formats (assoc tag notmuch-tag-formats)))
> +  (let ((formats (or (assoc tag notmuch-tag-formats)
> +		     (assoc "" notmuch-tag-formats))))
>      (cond
>       ((null formats)		;; - Tag not in `notmuch-tag-formats',
>        tag)			;;   the format is the tag itself.
> -- 
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/7] emacs: tag: allow default case in notmuch-tag-formats
  2014-02-10 18:19   ` Austin Clements
@ 2014-02-11 10:23     ` Mark Walters
  2014-02-11 22:57       ` Austin Clements
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Walters @ 2014-02-11 10:23 UTC (permalink / raw)
  To: Austin Clements, notmuch


Thanks for the review.

On Mon, 10 Feb 2014, Austin Clements <amdragon@MIT.EDU> wrote:
> On Sat, 18 Jan 2014, Mark Walters <markwalters1009@gmail.com> wrote:
>> Allow an empty string in notmuch-tag-formats which matches all tags
>> except those matched explicitly matched. This allows the user to tell
>
> Typo.

Will fix.

>> notmuch to hide all tags except those specified.
>>
>> This will be useful once formatting for deleted/added tags is added
>> later in the series: a user might want to hide all deleted tags for
>> example.
>> ---
>>  emacs/notmuch-tag.el |   20 +++++++++++---------
>>  1 files changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
>> index 2153068..92c1249 100644
>> --- a/emacs/notmuch-tag.el
>> +++ b/emacs/notmuch-tag.el
>> @@ -65,14 +65,15 @@
>>  This gives a list that maps from tag names to lists of formatting
>>  expressions.  The car of each element gives a tag name and the
>>  cdr gives a list of Elisp expressions that modify the tag.  If
>> -the list is empty, the tag will simply be hidden.  Otherwise,
>> -each expression will be evaluated in order: for the first
>> -expression, the variable `tag' will be bound to the tag name; for
>> -each later expression, the variable `tag' will be bound to the
>> -result of the previous expression.  In this way, each expression
>> -can build on the formatting performed by the previous expression.
>> -The result of the last expression will displayed in place of the
>> -tag.
>> +the car is an empty string it matches all tags that do not have
>> +an explicit match.  If the list is empty, the tag will simply be
>
> Hmm.  I'm not sure I like overloading of the meanings of strings.  Could
> we instead use a symbol to represent this case?  For example, `t' would
> parallel the fall-through case of `cond' and `case', or `_' would
> parallel `pcase' [1].  Or even a separate variable like
> notmuch-tag-default-format?

I would prefer not to have a separate variable as I want the default
case for added/deleted tags too (see next patch), so it would need to be
3 separate variables. But maybe that makes things clearer and is worth
doing?

One other possibility that would solve the customize problem would be to
allow regexps for the matches. The regexp would have to match the
complete tag and only the first match would be used. This has advantages
(the user can highlight all notmuch::.* tags or can hide all X-
tags). The downside is that finding/testing for a regexp match is about
20 times slower than using assoc, primarily because assoc is written in
C and the regexp match bit in lisp.

> The former would require some tweaking of the customize widget, but that
> should probably happen anyway to support this special case.
> Unfortunately, we may need a custom alist widget variant to do that.  (I
> tried and failed to tweak it in a way that both worked and looked
> decent.)  Hence my suggestion of a separate variable, which would only
> require pulling out the :value-type into its own define-widget.

If we go this route I may need some help getting this to work: pulling
out value-type didn't work on my first attempt.

> I'm also slightly bothered that this would introduce a second way to
> control the default formatting of tags in addition to notmuch-tag-face,
> but only slightly.

Yes that slightly bothered me but I didn't see a solution.

> [1] It's unfortunate that pcase wasn't introduced until Emacs 24.  I've
> been tempted to backport it for notmuch multiple times now.  Then we
> could just treat notmuch-tag-formats as a list of pcase conditions.

Would that still involve a lisp loop so would be comparable to the
regexp bit above? I haven't looked at pcase enough to work out its
details.

Best wishes

Mark


>
>> +hidden.  Otherwise, each expression will be evaluated in order:
>> +for the first expression, the variable `tag' will be bound to the
>> +tag name; for each later expression, the variable `tag' will be
>> +bound to the result of the previous expression.  In this way,
>> +each expression can build on the formatting performed by the
>> +previous expression.  The result of the last expression will
>> +displayed in place of the tag.
>>  
>>  For example, to replace a tag with another string, simply use
>>  that string as a formatting expression.  To change the foreground
>> @@ -140,7 +141,8 @@ This can be used with `notmuch-tag-format-image-data'."
>>  
>>  (defun notmuch-tag-format-tag (tag)
>>    "Format TAG by looking into `notmuch-tag-formats'."
>> -  (let ((formats (assoc tag notmuch-tag-formats)))
>> +  (let ((formats (or (assoc tag notmuch-tag-formats)
>> +		     (assoc "" notmuch-tag-formats))))
>>      (cond
>>       ((null formats)		;; - Tag not in `notmuch-tag-formats',
>>        tag)			;;   the format is the tag itself.
>> -- 
>> 1.7.9.1
>>
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/7] emacs: tag: allow default case in notmuch-tag-formats
  2014-02-11 10:23     ` Mark Walters
@ 2014-02-11 22:57       ` Austin Clements
  2014-02-12 17:32         ` [WIP PATCH] Make keys of notmuch-tag-formats regexps and use caching Austin Clements
  0 siblings, 1 reply; 16+ messages in thread
From: Austin Clements @ 2014-02-11 22:57 UTC (permalink / raw)
  To: Mark Walters, notmuch

On Tue, 11 Feb 2014, Mark Walters <markwalters1009@gmail.com> wrote:
> Thanks for the review.
>
> On Mon, 10 Feb 2014, Austin Clements <amdragon@MIT.EDU> wrote:
>> On Sat, 18 Jan 2014, Mark Walters <markwalters1009@gmail.com> wrote:
>>> Allow an empty string in notmuch-tag-formats which matches all tags
>>> except those matched explicitly matched. This allows the user to tell
>>
>> Typo.
>
> Will fix.
>
>>> notmuch to hide all tags except those specified.
>>>
>>> This will be useful once formatting for deleted/added tags is added
>>> later in the series: a user might want to hide all deleted tags for
>>> example.
>>> ---
>>>  emacs/notmuch-tag.el |   20 +++++++++++---------
>>>  1 files changed, 11 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
>>> index 2153068..92c1249 100644
>>> --- a/emacs/notmuch-tag.el
>>> +++ b/emacs/notmuch-tag.el
>>> @@ -65,14 +65,15 @@
>>>  This gives a list that maps from tag names to lists of formatting
>>>  expressions.  The car of each element gives a tag name and the
>>>  cdr gives a list of Elisp expressions that modify the tag.  If
>>> -the list is empty, the tag will simply be hidden.  Otherwise,
>>> -each expression will be evaluated in order: for the first
>>> -expression, the variable `tag' will be bound to the tag name; for
>>> -each later expression, the variable `tag' will be bound to the
>>> -result of the previous expression.  In this way, each expression
>>> -can build on the formatting performed by the previous expression.
>>> -The result of the last expression will displayed in place of the
>>> -tag.
>>> +the car is an empty string it matches all tags that do not have
>>> +an explicit match.  If the list is empty, the tag will simply be
>>
>> Hmm.  I'm not sure I like overloading of the meanings of strings.  Could
>> we instead use a symbol to represent this case?  For example, `t' would
>> parallel the fall-through case of `cond' and `case', or `_' would
>> parallel `pcase' [1].  Or even a separate variable like
>> notmuch-tag-default-format?
>
> I would prefer not to have a separate variable as I want the default
> case for added/deleted tags too (see next patch), so it would need to be
> 3 separate variables. But maybe that makes things clearer and is worth
> doing?

Ah, I see.  In that case I agree this shouldn't be a separate variable
(that idea was just a workaround anyway).

> One other possibility that would solve the customize problem would be to
> allow regexps for the matches. The regexp would have to match the
> complete tag and only the first match would be used. This has advantages
> (the user can highlight all notmuch::.* tags or can hide all X-
> tags). The downside is that finding/testing for a regexp match is about
> 20 times slower than using assoc, primarily because assoc is written in
> C and the regexp match bit in lisp.

I really like the idea of using regexps for this.  I wouldn't worry
about the performance.  We can almost certainly eliminate that problem
with a sprinkle of caching (which would probably be even faster than
assoc), and that can be added separately.

>> The former would require some tweaking of the customize widget, but that
>> should probably happen anyway to support this special case.
>> Unfortunately, we may need a custom alist widget variant to do that.  (I
>> tried and failed to tweak it in a way that both worked and looked
>> decent.)  Hence my suggestion of a separate variable, which would only
>> require pulling out the :value-type into its own define-widget.
>
> If we go this route I may need some help getting this to work: pulling
> out value-type didn't work on my first attempt.
>
>> I'm also slightly bothered that this would introduce a second way to
>> control the default formatting of tags in addition to notmuch-tag-face,
>> but only slightly.
>
> Yes that slightly bothered me but I didn't see a solution.
>
>> [1] It's unfortunate that pcase wasn't introduced until Emacs 24.  I've
>> been tempted to backport it for notmuch multiple times now.  Then we
>> could just treat notmuch-tag-formats as a list of pcase conditions.
>
> Would that still involve a lisp loop so would be comparable to the
> regexp bit above? I haven't looked at pcase enough to work out its
> details.

pcase compiles into a decision tree, which could in principle reduce
this to a fast lookup when possible, though I don't know if the
implementation is actually that smart.  At any rate, I like the idea of
using regexps for this better anyway.

> Best wishes
>
> Mark
>
>
>>
>>> +hidden.  Otherwise, each expression will be evaluated in order:
>>> +for the first expression, the variable `tag' will be bound to the
>>> +tag name; for each later expression, the variable `tag' will be
>>> +bound to the result of the previous expression.  In this way,
>>> +each expression can build on the formatting performed by the
>>> +previous expression.  The result of the last expression will
>>> +displayed in place of the tag.
>>>  
>>>  For example, to replace a tag with another string, simply use
>>>  that string as a formatting expression.  To change the foreground
>>> @@ -140,7 +141,8 @@ This can be used with `notmuch-tag-format-image-data'."
>>>  
>>>  (defun notmuch-tag-format-tag (tag)
>>>    "Format TAG by looking into `notmuch-tag-formats'."
>>> -  (let ((formats (assoc tag notmuch-tag-formats)))
>>> +  (let ((formats (or (assoc tag notmuch-tag-formats)
>>> +		     (assoc "" notmuch-tag-formats))))
>>>      (cond
>>>       ((null formats)		;; - Tag not in `notmuch-tag-formats',
>>>        tag)			;;   the format is the tag itself.
>>> -- 
>>> 1.7.9.1
>>>
>>> _______________________________________________
>>> notmuch mailing list
>>> notmuch@notmuchmail.org
>>> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/7] emacs: show: mark tags changed since buffer loaded
  2014-01-18 23:30 ` [PATCH 4/7] emacs: show: mark tags changed since buffer loaded Mark Walters
@ 2014-02-12  1:21   ` Austin Clements
  0 siblings, 0 replies; 16+ messages in thread
From: Austin Clements @ 2014-02-12  1:21 UTC (permalink / raw)
  To: Mark Walters, notmuch

On Sat, 18 Jan 2014, Mark Walters <markwalters1009@gmail.com> wrote:
> This allows (and requires) the original-tags to be passed along with
> the current-tags to be passed to notmuch-tag-format-tags. This allows
> the tag formatting to show added and deleted tags.By default a removed
> tag is displayed with strike-through in red (if strike-through is not
> available, eg on a terminal, inverse video is used instead) and an
> added tag is displayed underlined in green.
>
> If the caller does not wish to use the new feature it can pass
> current-tags for both arguments and, at this point, we do exactly that
> in the three callers of this function.
>
> Note, we cannot tidily allow original-tags to be optional because we would
> need to distinguish nil meaning "we are not specifying original-tags"
> from nil meaning there were no original-tags (an empty list).
>
> We use this in subsequent patches to make it clear when a message was
> unread when you first loaded a show buffer (previously the unread tag
> could be removed before a user realised that it had been unread).
>
> The code adds into the existing tag formatting code. The user can
> specify exactly how a tag should be displayed normally, when deleted,
> or when added. For convenience an entry for the empty string in the
> notmuch-tag-formats (and the corresponding notmuch-tag-deleted-formats
> notmuch-tag-added-formats) is applied to all tags which do not have an
> explicit match.
>
> This means that a user can tell notmuch not to show deleted tags at
> all by setting notmuch-tag-deleted-formats to
> '(("" nil))
> or not to show any deleted tags except "unread" by setting it to
> '(("" nil)
>   ("unread" (propertize tag 'face '(strike-through "red"))))
>
> All the variables are customizable; however, more complicated cases
> like changing the face depending on the type of display will require
> custom lisp.
>
> Currently this overrides notmuch-tag-deleted-formats for the tests
> setting it to '(("" nil)) so that they get removed from the display
> and, thus, all tests still pass.
> ---
>  emacs/notmuch-show.el |    4 ++--
>  emacs/notmuch-tag.el  |   32 +++++++++++++++++++++++++-------
>  emacs/notmuch-tree.el |    2 +-
>  emacs/notmuch.el      |    2 +-
>  test/test-lib.el      |    4 ++++
>  5 files changed, 33 insertions(+), 11 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 1ac80ca..1ce56f9 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -344,7 +344,7 @@ operation on the contents of the current buffer."
>      (if (re-search-forward "(\\([^()]*\\))$" (line-end-position) t)
>  	(let ((inhibit-read-only t))
>  	  (replace-match (concat "("
> -				 (notmuch-tag-format-tags tags)
> +				 (notmuch-tag-format-tags tags tags)
>  				 ")"))))))
>  
>  (defun notmuch-clean-address (address)
> @@ -423,7 +423,7 @@ message at DEPTH in the current thread."
>  	    " ("
>  	    date
>  	    ") ("
> -	    (notmuch-tag-format-tags tags)
> +	    (notmuch-tag-format-tags tags tags)
>  	    ")\n")
>      (overlay-put (make-overlay start (point)) 'face 'notmuch-message-summary-face)))
>  
> diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
> index 9757a0e..6bc47fb 100644
> --- a/emacs/notmuch-tag.el
> +++ b/emacs/notmuch-tag.el
> @@ -164,10 +164,25 @@ This can be used with `notmuch-tag-format-image-data'."
>    </g>
>  </svg>")
>  
> -(defun notmuch-tag-format-tag (tag)
> -  "Format TAG by looking into `notmuch-tag-formats'."
> -  (let ((formats (or (assoc tag notmuch-tag-formats)
> -		     (assoc "" notmuch-tag-formats))))
> +(defun notmuch-tag-format-tag (tags orig-tags tag)
> +  "Format TAG by looking into `notmuch-tag-formats'.
> +
> +TAGS and ORIG-TAGS are lists of the current tags and the original
> +tags; tags which have been deleted (i.e., are in ORIG-TAGS but
> +are not in TAGS) are shown using formats from
> +`notmuch-tag-deleted-formats'; tags which have been added (i.e.,
> +are in TAGS but are not in ORIG-TAGS) are shown using formats
> +from `notmuch-tag-added-formats' and tags which have not been
> +changed (the normal case) are shown using formats from
> +`notmuch-tag-formats'"
> +  (let* ((status-formats (cond ((and (member tag tags) (member tag orig-tags))
> +				notmuch-tag-formats)
> +			       ((not (member tag tags))
> +				notmuch-tag-deleted-formats)
> +			       ((not (member tag orig-tags))
> +				notmuch-tag-added-formats)))
> +	 (formats (or (assoc tag status-formats)
> +		      (assoc "" status-formats))))

I would expect this to be cumulative.  That is, I would want added and
deleted tags to have my regular `notmuch-tag-formats' formatting in
addition to extra formatting specified by
`notmuch-tag-{added,deleted}-formats' (especially if, for example, it's
replaced with an image).

I think this should just be a matter of something like
  (let ((formats
         (append
          (notmuch-tag--get-format tag notmuch-tag-formats)
          (cond ((not (member tag tags))
                 (notmuch-tag--get-format tag notmuch-tag-deleted-formats))
                ((not (member tag orig-tags)
                 (notmuch-tag--get-format tag notmuch-tag-added-formats)))))))
    ...)
where notmuch-tag--get-format is something I made up to abstract the
assoc (or the regexp lookup).  This also has the benefit of reducing the
number of member tests.

>      (cond
>       ((null formats)		;; - Tag not in `notmuch-tag-formats',
>        tag)			;;   the format is the tag itself.
> @@ -178,13 +193,16 @@ This can be used with `notmuch-tag-format-image-data'."
>  	(dolist (format (cdr formats) tag)
>  	  (setq tag (eval format))))))))
>  
> -(defun notmuch-tag-format-tags (tags &optional face)
> +(defun notmuch-tag-format-tags (tags orig-tags &optional face)
>    "Return a string representing formatted TAGS."
> -  (let ((face (or face 'notmuch-tag-face)))
> +  (let ((face (or face 'notmuch-tag-face))
> +	(all-tags (sort (delete-dups (append tags orig-tags)) #'string<)))

This may mutate orig-tags.  It's not obvious to me that's okay.  You
could use remove-duplicates from cl.  (I'm surprised I can't find a
uniq-like function in cl, which would save the O(n^2) operation, but I
doubt it matters.)

>      (notmuch-combine-face-text-property-string
>       (mapconcat #'identity
>  		;; nil indicated that the tag was deliberately hidden
> -		(delq nil (mapcar #'notmuch-tag-format-tag tags))
> +		(delq nil (mapcar
> +			   (apply-partially #'notmuch-tag-format-tag tags orig-tags)
> +			   all-tags))
>  		" ")
>       face
>       t)))
> diff --git a/emacs/notmuch-tree.el b/emacs/notmuch-tree.el
> index 4f2ac02..b37e2cd 100644
> --- a/emacs/notmuch-tree.el
> +++ b/emacs/notmuch-tree.el
> @@ -704,7 +704,7 @@ unchanged ADDRESS if parsing fails."
>  	    (face (if match
>  		      'notmuch-tree-match-tag-face
>  		    'notmuch-tree-no-match-tag-face)))
> -	(format format-string (notmuch-tag-format-tags tags face)))))))
> +	(format format-string (notmuch-tag-format-tags tags tags face)))))))
>  
>  (defun notmuch-tree-format-field-list (field-list msg)
>    "Format fields of MSG according to FIELD-LIST and return string"
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 0471750..1436e5a 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -754,7 +754,7 @@ non-authors is found, assume that all of the authors match."
>  
>     ((string-equal field "tags")
>      (let ((tags (plist-get result :tags)))
> -      (insert (format format-string (notmuch-tag-format-tags tags)))))))
> +      (insert (format format-string (notmuch-tag-format-tags tags tags)))))))
>  
>  (defun notmuch-search-show-result (result &optional pos)
>    "Insert RESULT at POS or the end of the buffer if POS is null."
> diff --git a/test/test-lib.el b/test/test-lib.el
> index 37fcb3d..6cbd57c 100644
> --- a/test/test-lib.el
> +++ b/test/test-lib.el
> @@ -165,3 +165,7 @@ nothing."
>  
>       (t
>        (notmuch-test-report-unexpected output expected)))))
> +
> +;; hide deleted tags

Maybe "For historical reasons, we hide deleted tags by default in the
test suite"?

> +(setq notmuch-tag-deleted-formats
> +      '(("" nil)))
> -- 
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 6/7] emacs: search: use orig-tags in search
  2014-01-18 23:30 ` [PATCH 6/7] emacs: search: use orig-tags in search Mark Walters
@ 2014-02-12  1:30   ` Austin Clements
  0 siblings, 0 replies; 16+ messages in thread
From: Austin Clements @ 2014-02-12  1:30 UTC (permalink / raw)
  To: Mark Walters, notmuch

On Sat, 18 Jan 2014, Mark Walters <markwalters1009@gmail.com> wrote:
> This uses the recent functionality to show the tag changes in the
> search buffer. Currently this is only used to show changes the search
> buffer makes itself: i.e., it does not make display any changes
> reflecting tagging done by other notmuch-buffers.
> ---
>  emacs/notmuch.el |   12 +++++++++---
>  1 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 1436e5a..f0ea5d4 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -753,14 +753,20 @@ non-authors is found, assume that all of the authors match."
>       format-string (notmuch-sanitize (plist-get result :authors))))
>  
>     ((string-equal field "tags")
> -    (let ((tags (plist-get result :tags)))
> -      (insert (format format-string (notmuch-tag-format-tags tags tags)))))))
> +    (let ((tags (plist-get result :tags))
> +	  (orig-tags (plist-get result :orig-tags)))
> +      (insert (format format-string (notmuch-tag-format-tags tags orig-tags)))))))
>  
>  (defun notmuch-search-show-result (result &optional pos)
>    "Insert RESULT at POS or the end of the buffer if POS is null."
>    ;; Ignore excluded matches
>    (unless (= (plist-get result :matched) 0)
> -    (let ((beg (or pos (point-max))))
> +    (let ((beg (or pos (point-max)))
> +	  ;; If we are inserting the result for the first time POS
> +	  ;; will be nil and we should set orig-tags.
> +	  (result (if pos
> +		      result
> +		    (plist-put result :orig-tags (plist-get result :tags)))))

This seems rather obtuse.  I'd prefer a little wrapper that sits between
`notmuch-sexp-parse-partial-list' and `notmuch-search-show-result' that
adds :orig-tags.  The wrapper could also pass (point-max) for pos and
pos could be made a required argument.  E.g.,

(defun notmuch-search-process-result (result)
  ;; Record original tags of this message
  (setq result (plist-put result :orig-tags (plist-get result :tags)))
  ;; Append the result to the search buffer
  (notmuch-search-show-result result (point-max)))

>        (save-excursion
>  	(goto-char beg)
>  	(dolist (spec notmuch-search-result-format)
> -- 
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [WIP PATCH] Make keys of notmuch-tag-formats regexps and use caching
  2014-02-11 22:57       ` Austin Clements
@ 2014-02-12 17:32         ` Austin Clements
  0 siblings, 0 replies; 16+ messages in thread
From: Austin Clements @ 2014-02-12 17:32 UTC (permalink / raw)
  To: notmuch

This was a little hack to test the feasibility of switching
notmuch-tag-formats to use regexps with caching for performance.  In
the end it works fine and isn't particularly complex, though there
were a few gotchas:

1) We have to clear the cache somehow on changes to
notmuch-tag-formats.  I opted to use a defcustom :set plus some
documentation telling people what to do if they change it directly
from Elisp.  This is less automatic than I would like, but I doubt
people are changing this very often and I concluded that any machinery
to automatically detect changes to notmuch-tag-formats would probably
outweigh the benefits of caching.  Alternatively, we could require
search/show/tree buffers to "opt in" to caching when they start
building.

2) I spent way too long trying to use assoc-default before realizing
this it just wouldn't work, since there's no way to distinguish a
missing key from a present key with a null cdr.  assoc* from cl works
fine.

Performance-wise, the caching of regexp lookup makes this just as fast
as assoc for unformatted tags (it would probably be faster if someone
had a really big `notmuch-tag-formats') and the caching of eval
results makes this much *faster* than the current code for formatted
tags.

                inbox (usec)   unread (usec)
assoc:              0.4            2.8
regexp:             3.2            7.2
regexp+caching:     0.4            0.4

That said, even at 7.2 usec, tag formatting is still *very* fast
(though regexp matching may get noticeably slower with larger
`notmuch-tag-formats').  Tag formatting is nowhere near our top
bottleneck.
---
 emacs/notmuch-tag.el | 75 +++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 53 insertions(+), 22 deletions(-)

diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
index b60f46c..07f5772 100644
--- a/emacs/notmuch-tag.el
+++ b/emacs/notmuch-tag.el
@@ -28,35 +28,56 @@
 (require 'crm)
 (require 'notmuch-lib)
 
+;; (notmuch-tag-clear-cache will be called by the defcustom
+;; notmuch-tag-formats, so it has to be defined first.)
+
+(defvar notmuch-tag--format-cache (make-hash-table :test 'equal)
+  "Cache of tag format lookup.  Internal to `notmuch-tag-format-tag'.")
+
+(defun notmuch-tag-clear-cache ()
+  "Clear the internal cache of tag formats.
+
+This must be called after changes to `notmuch-tag-formats'."
+  (clrhash notmuch-tag--format-cache))
+
 (defcustom notmuch-tag-formats
   '(("unread" (propertize tag 'face '(:foreground "red")))
     ("flagged" (propertize tag 'face '(:foreground "blue"))
      (notmuch-tag-format-image-data tag (notmuch-tag-star-icon))))
   "Custom formats for individual tags.
 
-This gives a list that maps from tag names to lists of formatting
-expressions.  The car of each element gives a tag name and the
-cdr gives a list of Elisp expressions that modify the tag.  If
-the list is empty, the tag will simply be hidden.  Otherwise,
-each expression will be evaluated in order: for the first
-expression, the variable `tag' will be bound to the tag name; for
-each later expression, the variable `tag' will be bound to the
-result of the previous expression.  In this way, each expression
-can build on the formatting performed by the previous expression.
-The result of the last expression will displayed in place of the
-tag.
+This is an association list that maps from tag name regexps to
+lists of formatting expressions.  The first entry whose car
+regexp-matches a tag will be used to format that tag.  The regexp
+is implicitly anchored, so to match a literal tag name, just use
+that tag name (if it contains special regexp characters like
+\".\" or \"*\", these have to be escaped).  The cdr of the
+matching entry gives a list of Elisp expressions that modify the
+tag.  If the list is empty, the tag will simply be hidden.
+Otherwise, each expression will be evaluated in order: for the
+first expression, the variable `tag' will be bound to the tag
+name; for each later expression, the variable `tag' will be bound
+to the result of the previous expression.  In this way, each
+expression can build on the formatting performed by the previous
+expression.  The result of the last expression will displayed in
+place of the tag.
 
 For example, to replace a tag with another string, simply use
 that string as a formatting expression.  To change the foreground
 of a tag to red, use the expression
   (propertize tag 'face '(:foreground \"red\"))
 
+After modifying this variable in Elisp, be sure to call
+`notmuch-tag-clear-cache'.  Modifying this via customize does
+this automatically.
+
 See also `notmuch-tag-format-image', which can help replace tags
 with images."
 
   :group 'notmuch-search
   :group 'notmuch-show
-  :type '(alist :key-type (string :tag "Tag")
+  :set (lambda (var val) (set-default var val) (notmuch-tag-clear-cache))
+  :type '(alist :key-type (regexp :tag "Tag")
 		:extra-offset -3
 		:value-type
 		(radio :format "%v"
@@ -137,16 +158,26 @@ This can be used with `notmuch-tag-format-image-data'."
 
 (defun notmuch-tag-format-tag (tag)
   "Format TAG by looking into `notmuch-tag-formats'."
-  (let ((formats (assoc tag notmuch-tag-formats)))
-    (cond
-     ((null formats)		;; - Tag not in `notmuch-tag-formats',
-      tag)			;;   the format is the tag itself.
-     ((null (cdr formats))	;; - Tag was deliberately hidden,
-      nil)			;;   no format must be returned
-     (t				;; - Tag was found and has formats,
-      (let ((tag tag))		;;   we must apply all the formats.
-	(dolist (format (cdr formats) tag)
-	  (setq tag (eval format))))))))
+  (let ((formatted (gethash tag notmuch-tag--format-cache 'missing)))
+    (when (eq formatted 'missing)
+      (let* ((formats
+	      (save-match-data
+		(assoc* tag notmuch-tag-formats
+			:test (lambda (key tag)
+				(and (eq (string-match key tag) 0)
+				     (= (match-end 0) (length tag))))))))
+	(setq formatted
+	      (cond
+	       ((null formats)		;; - Tag not in `notmuch-tag-formats',
+		tag)			;;   the format is the tag itself.
+	       ((null (cdr formats))	;; - Tag was deliberately hidden,
+		nil)			;;   no format must be returned
+	       (t			;; - Tag was found and has formats,
+		(let ((tag tag))	;;   we must apply all the formats.
+		  (dolist (format (cdr formats) tag)
+		    (setq tag (eval format)))))))
+	(puthash tag formatted notmuch-tag--format-cache)))
+    formatted))
 
 (defun notmuch-tag-format-tags (tags)
   "Return a string representing formatted TAGS."
-- 
1.8.4.rc3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2014-02-12 17:32 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-18 23:30 [PATCH 0/7] emacs: show tag changes in buffer Mark Walters
2014-01-18 23:30 ` [PATCH 1/7] emacs: tag split customise option for format-tags into a widget Mark Walters
2014-01-18 23:30 ` [PATCH 2/7] emacs: tag: allow default case in notmuch-tag-formats Mark Walters
2014-02-10 18:19   ` Austin Clements
2014-02-11 10:23     ` Mark Walters
2014-02-11 22:57       ` Austin Clements
2014-02-12 17:32         ` [WIP PATCH] Make keys of notmuch-tag-formats regexps and use caching Austin Clements
2014-01-18 23:30 ` [PATCH 3/7] emacs: tag: add customize for deleted/added tag formats Mark Walters
2014-01-18 23:30 ` [PATCH 4/7] emacs: show: mark tags changed since buffer loaded Mark Walters
2014-02-12  1:21   ` Austin Clements
2014-01-18 23:30 ` [PATCH 5/7] emacs: show: use orig-tags for tag display Mark Walters
2014-01-18 23:30 ` [PATCH 6/7] emacs: search: use orig-tags in search Mark Walters
2014-02-12  1:30   ` Austin Clements
2014-01-18 23:30 ` [PATCH 7/7] emacs: tree: " Mark Walters
2014-01-25 19:35 ` [PATCH 0/7] emacs: show tag changes in buffer Jani Nikula
2014-02-05 22:25 ` Tomi Ollila

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).