Date: Tue, 20 Nov 2012 00:32:07 -0500
From: Austin Clements <amdragon@MIT.EDU>
To: Damien Cassou <damien.cassou@gmail.com>
Subject: Re: [PATCH 1/4] Add a thread's tags to emacs header-line
Message-ID: <20121120053207.GB32514@mit.edu>
References: <1353266322-20318-1-git-send-email-damien.cassou@gmail.com>
	<1353266322-20318-2-git-send-email-damien.cassou@gmail.com>
	<20121120051320.GA32514@mit.edu>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20121120051320.GA32514@mit.edu>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: notmuch mailing list <notmuch@notmuchmail.org>
Precedence: list

Sorry, I was a little trigger-happy on the send.  A few more comments
below.

Quoth myself on Nov 20 at 12:13 am:
> Quoth Damien Cassou on Nov 18 at  8:18 pm:
> > Signed-off-by: Damien Cassou <damien.cassou@gmail.com>
> > ---
> >  emacs/notmuch-show.el   |   24 +++++++++++++++++++++---
> >  emacs/notmuch-tagger.el |   44 ++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 65 insertions(+), 3 deletions(-)
> >  create mode 100644 emacs/notmuch-tagger.el
> > 
> > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> > index 5b3e70e..988e27c 100644
> > --- a/emacs/notmuch-show.el
> > +++ b/emacs/notmuch-show.el
> > @@ -36,6 +36,7 @@
> >  (require 'notmuch-mua)
> >  (require 'notmuch-crypto)
> >  (require 'notmuch-print)
> > +(require 'notmuch-tagger)
> >  
> >  (declare-function notmuch-call-notmuch-process "notmuch" (&rest args))
> >  (declare-function notmuch-fontify-headers "notmuch" nil)
> > @@ -1121,11 +1122,28 @@ function is used."
> >  
> >        (jit-lock-register #'notmuch-show-buttonise-links)
> >  
> > -      ;; Set the header line to the subject of the first message.
> > -      (setq header-line-format (notmuch-show-strip-re (notmuch-show-get-subject)))
> > -
> > +      (notmuch-show-update-header-line)
> >        (run-hooks 'notmuch-show-hook))))
> >  
> > +(defun notmuch-show-thread-tags ()
> > +  "Return the list of tags for the current thread."
> > +  (let ((tags (list)))
> > +    (notmuch-show-mapc (lambda ()
> > +			 (mapcar (lambda (elt)
> > +				   ;; Avoid adding duplicate tags
> > +				   (add-to-list 'tags elt))
> > +				 (notmuch-show-get-tags))))
> > +    tags))
> > +
> > +(defun notmuch-show-update-header-line ()
> > +  "Make the header-line show the thread's subject and tags."
> > +  (let ((thread-subject (notmuch-show-strip-re (notmuch-show-get-subject))))
> > +    (setq header-line-format
> > +	  (list
> > +	   thread-subject
> > +	   " "
> > +	   (notmuch-tagger-present-tags-header-line (notmuch-show-thread-tags))))))
> > +
> >  (defun notmuch-show-capture-state ()
> >    "Capture the state of the current buffer.
> >  
> > diff --git a/emacs/notmuch-tagger.el b/emacs/notmuch-tagger.el
> > new file mode 100644
> > index 0000000..5ca190e
> > --- /dev/null
> > +++ b/emacs/notmuch-tagger.el
> > @@ -0,0 +1,44 @@
> > +;; notmuch-tagger.el --- Library to improve the way tags are displayed
> > +;;
> > +;; Copyright © Damien Cassou
> > +;;
> > +;; This file is part of Notmuch.
> > +;;
> > +;; Notmuch is free software: you can redistribute it and/or modify it
> > +;; under the terms of the GNU General Public License as published by
> > +;; the Free Software Foundation, either version 3 of the License, or
> > +;; (at your option) any later version.
> > +;;
> > +;; Notmuch is distributed in the hope that it will be useful, but
> > +;; WITHOUT ANY WARRANTY; without even the implied warranty of
> > +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +;; General Public License for more details.
> > +;;
> > +;; You should have received a copy of the GNU General Public License
> > +;; along with Notmuch.  If not, see <http://www.gnu.org/licenses/>.
> > +;;
> > +;; Authors: Damien Cassou <damien.cassou@gmail.com>
> > +;;; Commentary:
> > +;;
> > +;;; Code:
> > +;;
> > +(defun notmuch-tagger-separate-elems (list sep)
> 
> notmuch-tagger-intersperse?
> 
> This could also reasonably go in notmuch-lib.el as a general utility.
> 
> (How do neither Elisp nor 'cl have an intersperse?)
> 
> > +  "Return a list with all elements of LIST separated by SEP."
> > +  (let ((first t)
> > +        (res nil))
> > +    (dolist (elt (reverse list) res)
> 
> (dolist (elt list (nreverse res)) ...) to save a list copy?
> 
> > +      (unless first
> > +        (push sep res))
> > +      (setq first nil)
> > +      (push elt res))))
> > +
> > +(defun notmuch-tagger-present-tags-header-line (tags)

notmuch-tagger-format-tags-header-line?  It doesn't actually present
them (though "format" isn't great either.)

> > +  "Return a property list to present TAGS in emacs header-line."

This doesn't return a property list, it returns a mode-line-format
template.  Maybe something like,

  "Format TAGS as a mode-line-format template.
The result is suitable for inclusion in `header-line-format' or
`mode-line-format'."

(Is it actually suitable for mode-line-format or do header-line
buttons really only work in the header line?)

> > +  (list
> > +   "("
> > +   (notmuch-tagger-separate-elems tags " ")
> > +   ")"))
> > +
> > +
> > +(provide 'notmuch-tagger)
> > +;;; notmuch-tagger.el ends here