Re:

[Mark Walters <markwalters1009@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 863 bytes --]


Hi

This is looking good: I have two comments the second of which is significant.

The first is do you want to sort (alphabetically) the headerline tags?
As it stands they are in the order they appear in the thread which is
probably not what is wanted.

The second is that there is a notmuch-show-tag-all functions to tag all
messages in the thread. Your patch is quadratic for the update (as it
calculates the list of thread tags once for each message). The attached
patch would avoid this and doesn't look too bad. (Note I retained
no-headerline-update as an optional argument in case there are out of
tree callers, eg users' .emacs files)

[Note this is not purely of academic interest: on my test large thread
(178 messages) updating the display after tagging all messages took some
seconds without my patch and almost no time with it.]

Best wishes 

Mark


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-mjw-tweak.patch --]
[-- Type: text/x-diff, Size: 2186 bytes --]

From ab15a4bdb50bcf6b2851806195bbe8bea3b099dc Mon Sep 17 00:00:00 2001
From: Mark Walters <markwalters1009@gmail.com>
Date: Thu, 13 Dec 2012 11:23:09 +0000
Subject: [PATCH] Avoid quadratic update

---
 emacs/notmuch-show.el |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 93bce07..8dd6010 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -356,7 +356,7 @@ operation on the contents of the current buffer."
   "Return a string comprised of `n' spaces."
   (make-string n ? ))
 
-(defun notmuch-show-update-tags (tags)
+(defun notmuch-show-update-tags (tags &optional no-headerline-update)
   "Update the displayed tags of the current message."
   (save-excursion
     (goto-char (notmuch-show-message-top))
@@ -364,7 +364,8 @@ operation on the contents of the current buffer."
 	(let ((inhibit-read-only t))
 	  (replace-match (propertize (notmuch-tagger-format-tags tags)
 				     'face 'notmuch-tag-face)))))
-  (notmuch-show-update-header-line))
+  (unless no-headerline-update
+    (notmuch-show-update-header-line)))
 
 (defun notmuch-clean-address (address)
   "Try to clean a single email ADDRESS for display. Return a cons
@@ -1461,10 +1462,10 @@ current thread."
 (defun notmuch-show-get-depth ()
   (notmuch-show-get-prop :depth))
 
-(defun notmuch-show-set-tags (tags)
+(defun notmuch-show-set-tags (tags &optional no-headerline-update)
   "Set the tags of the current message."
   (notmuch-show-set-prop :tags tags)
-  (notmuch-show-update-tags tags))
+  (notmuch-show-update-tags tags no-headerline-update))
 
 (defun notmuch-show-get-tags ()
   "Return the tags of the current message."
@@ -1778,7 +1779,8 @@ See `notmuch-tag' for information on the format of TAG-CHANGES."
      (let* ((current-tags (notmuch-show-get-tags))
 	    (new-tags (notmuch-update-tags current-tags tag-changes)))
        (unless (equal current-tags new-tags)
-	 (notmuch-show-set-tags new-tags))))))
+	 (notmuch-show-set-tags new-tags t)))))
+  (notmuch-show-update-header-line))
 
 (defun notmuch-show-add-tag ()
   "Same as `notmuch-show-tag' but sets initial input to '+'."
-- 
1.7.9.1


[-- Attachment #3: Type: text/plain, Size: 1489 bytes --]




On Tue, 11 Dec 2012, Damien Cassou <damien.cassou@gmail.com> wrote:
> From: Damien Cassou <damien.cassou@gmail.com>
> Subject: [PATCH v4] emacs: display tags in notmuch-show with links
> In-Reply-To: 
>
> This patch obsoletes:
> id:1355149964-27905-1-git-send-email-damien.cassou@gmail.com
>
> [PATCH 1/4] emacs: Add a thread's tags to notmuch-show header-line
> [PATCH 2/4] emacs: Make tags in notmuch-show header-line clickable
> [PATCH 3/4] emacs: Make all tags in `notmuch-show' clickable
> [PATCH 4/4] emacs: Add unit-tests for clickable tags
>
> These patches make clickable all tags that appear in notmuch-show
> buffers. Each tag is a link to open a new notmuch-search buffer for
> this tag. Additionally, the buffer's header-line now shows the
> thread's tags (clickable only if the `header-button' library is loaded
> or loadable).
>
> These patches are the first of an upcoming series whose goal is to
> integrate notmuch-labeler into notmuch. See the following for more
> details: https://github.com/DamienCassou/notmuch-labeler
>
> With respect to v3, I took care of the comments you made:
> - the header-line now updates when tags are changed
> - the tags in the body stays clickable when tags are changed
>
> Additionally, I added two unit tests to cover the above two comments
> and fixed some others unit tests of mine.
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch