On Wed, 25 Nov 2009 10:28:00 +0530, "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> This add two faces, notmuch-show-subject-face and
> notmuch-tag-face. The first face is used to show the subject
> line in the notmuch-show-mode and the second one to show tags
> in the notmuch-search-mode.
> 
First, I definetly think fontification is the way to go instead of
the adhoc/crappy way that cworth and I have been doing this in the
past.

[snip]

> +(defvar notmuch-show-font-lock-keywords
> +  (list ;; header in font-lock-type-face
> +   (list "\\(Subject:.*$\\)"
> +	 '(1  'notmuch-show-subject-face)))
> +  "Additonal expression to hightlight in notmuch-show-mode")
> +
So what happens if I have Subject: xxxx in my message? We already ran
into a problem where a patch sent the list that included notmuch control
character caused it to go into an infinite loop, I'd prefer our
fontification code be a bit more resilient. At the very least this
should use the font-lock syntax tables stuff to only do header
fontification when inside the header block. This would probably require
that fontification occur before the message is post-processed by 
notmuch-show to remove the section markers etc.

Also +1 for more subduded colors than red. :) Possibly consider copying
the message-mode colors, so that there is a bit of consistency between
sending and viewing mail? Might even be able to steal the message-mode
faces by importing their symbols.

[snip]

>  (defun notmuch-search-mode ()
>    "Major mode for searching mail with notmuch.
> @@ -865,7 +888,18 @@ global search.
>    (setq truncate-lines t)
>    (setq major-mode 'notmuch-search-mode
>  	mode-name "notmuch-search")
> -  (setq buffer-read-only t))
> +  (setq buffer-read-only t)
> +  (if (not notmuch-tag-face-alist)
> +      (add-to-list 'notmuch-search-font-lock-keywords (list
> +		"\\(([^)]*)$\\)" '(1  'notmuch-tag-face)))
This way of detecting the tags seems ok, but I think it would be nicer
if it could be done even more deterministically. :) One idea that be
neat is to have a --format=sexp for notmuch search, which exports sexps
(probably alists, but could be some other format) for the search results
that can just be eval'd and processed in a cleaner way (and would also
make for nicer APIs in emacs for querying notmuch itself). Actually I
really like the idea of a sexp output mode for show too, instead of the
markers.... *plots*

> +    (progn
> +  (setq notmuch-search-tags (mapcar 'car notmuch-tag-face-alist))
> +  (loop for notmuch-search-tag  in notmuch-search-tags
> +    do (add-to-list 'notmuch-search-font-lock-keywords (list
> +				(concat "\\(" notmuch-search-tag "\\)")
> +		    `(1  ,(cdr (assoc notmuch-search-tag notmuch-tag-face-alist))))))))
> +  (set (make-local-variable 'font-lock-defaults)
> +         '(notmuch-search-font-lock-keywords t)))
>  
I don't really see the point of fontifying all tags the same way if no
tag-faces have been set, especially if none of the rest of the search
results are fontified.

Alex