[PATCH] emacs: notmuch-search: fix faces

[PATCH] emacs: notmuch-search: fix faces

[Michal Nazarewicz]

From: Michal Nazarewicz <mina86@mina86.com>

For some reason the faces do not get applied when 'face property is
used, but they work correctly with 'font-lock-face property.  This
commit changes notmuch-search to use the latter.
---
 emacs/notmuch.el |   23 +++++++++++++----------
 1 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 7b61e9b..44cbe28 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -692,10 +692,10 @@ propertize appropriately. If no boundary between authors and
 non-authors is found, assume that all of the authors match."
   (if (string-match "\\(.*\\)|\\(.*\\)" authors)
       (concat (propertize (concat (match-string 1 authors) ",")
-			  'face 'notmuch-search-matching-authors)
+			  'font-lock-face 'notmuch-search-matching-authors)
 	      (propertize (match-string 2 authors)
-			  'face 'notmuch-search-non-matching-authors))
-    (propertize authors 'face 'notmuch-search-matching-authors)))
+			  'font-lock-face 'notmuch-search-non-matching-authors))
+    (propertize authors 'font-lock-face 'notmuch-search-matching-authors)))
 
 (defun notmuch-search-insert-authors (format-string authors)
   ;; Save the match data to avoid interfering with
@@ -741,11 +741,14 @@ non-authors is found, assume that all of the authors match."
 	  (setq visible-string (notmuch-search-author-propertize visible-string)
 		;; The invisible string must contain only non-matching
 		;; authors, as the visible-string contains both.
-		invisible-string (propertize invisible-string
-					     'face 'notmuch-search-non-matching-authors))
+		invisible-string
+		(propertize invisible-string
+			    'font-lock-face
+			    'notmuch-search-non-matching-authors))
 	;; The visible string contains only matching authors.
 	(setq visible-string (propertize visible-string
-					 'face 'notmuch-search-matching-authors)
+					 'font-lock-face
+					 'notmuch-search-matching-authors)
 	      ;; The invisible string may contain both matching and
 	      ;; non-matching authors.
 	      invisible-string (notmuch-search-author-propertize invisible-string)))
@@ -770,15 +773,15 @@ non-authors is found, assume that all of the authors match."
   (cond
    ((string-equal field "date")
     (insert (propertize (format format-string (plist-get result :date_relative))
-			'face 'notmuch-search-date)))
+			'font-lock-face 'notmuch-search-date)))
    ((string-equal field "count")
     (insert (propertize (format format-string
 				(format "[%s/%s]" (plist-get result :matched)
 					(plist-get result :total)))
-			'face 'notmuch-search-count)))
+			'font-lock-face 'notmuch-search-count)))
    ((string-equal field "subject")
     (insert (propertize (format format-string (plist-get result :subject))
-			'face 'notmuch-search-subject)))
+			'font-lock-face 'notmuch-search-subject)))
 
    ((string-equal field "authors")
     (notmuch-search-insert-authors format-string (plist-get result :authors)))
@@ -786,7 +789,7 @@ non-authors is found, assume that all of the authors match."
    ((string-equal field "tags")
     (let ((tags-str (mapconcat 'identity (plist-get result :tags) " ")))
       (insert (propertize (format format-string tags-str)
-			  'face 'notmuch-tag-face))))))
+			  'font-lock-face 'notmuch-tag-face))))))
 
 (defun notmuch-search-show-result (result &optional pos)
   "Insert RESULT at POS or the end of the buffer if POS is null."
-- 
1.7.7.3

Re: [PATCH] emacs: notmuch-search: fix faces

[Jameson Graef Rollins]

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

On Thu, Aug 23 2012, Michal Nazarewicz <mpn@google.com> wrote:
> For some reason the faces do not get applied when 'face property is
> used, but they work correctly with 'font-lock-face property.  This
> commit changes notmuch-search to use the latter.

Hi, Michal.  Can you say a bit more about what's motivating this?  This
feature seems to work fine for me, so I would like to understand what's
not working for you.  I'm certainly no expert on emacs font handling,
though, so maybe this is the proper thing to do.  Thanks.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH] emacs: notmuch-search: fix faces

[Michal Nazarewicz]

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

> On Thu, Aug 23 2012, Michal Nazarewicz <mpn@google.com> wrote:
>> For some reason the faces do not get applied when 'face property is
>> used, but they work correctly with 'font-lock-face property.  This
>> commit changes notmuch-search to use the latter.

Jameson Graef Rollins <jrollins@finestructure.net> writes:
> Hi, Michal.  Can you say a bit more about what's motivating this?  This
> feature seems to work fine for me, so I would like to understand what's
> not working for you.  I'm certainly no expert on emacs font handling,
> though, so maybe this is the proper thing to do.  Thanks.

I'm not an expert either, but with this patch applied I see colours,
without this patch, I don't see colours, ie. everything is rendered
using the default face.

I'm also not entirely sure if that's the correct way of doing things
since 'face seems to be working in other modes (most notably
notmuch-show).  By posting, I'm also hoping that someone more
experienced will maybe take a look to see what's going on here... ;)

For some more info, in notmuch-show mode, I get-text-property of nil for
both 'face and 'font-lock-face, but in message-mode or in *scratch*
buffer I'm getting nil for 'font-lock-face but non-nil for 'face.  With
the patch, I'm getting the same non-nil for both 'face and
'font-lock-face; without the patch, I'm getting nil for both.

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH] emacs: notmuch-search: fix faces

[Austin Clements]

Quoth Michal Nazarewicz on Aug 28 at  1:11 am:
> > On Thu, Aug 23 2012, Michal Nazarewicz <mpn@google.com> wrote:
> >> For some reason the faces do not get applied when 'face property is
> >> used, but they work correctly with 'font-lock-face property.  This
> >> commit changes notmuch-search to use the latter.
> 
> Jameson Graef Rollins <jrollins@finestructure.net> writes:
> > Hi, Michal.  Can you say a bit more about what's motivating this?  This
> > feature seems to work fine for me, so I would like to understand what's
> > not working for you.  I'm certainly no expert on emacs font handling,
> > though, so maybe this is the proper thing to do.  Thanks.
> 
> I'm not an expert either, but with this patch applied I see colours,
> without this patch, I don't see colours, ie. everything is rendered
> using the default face.
> 
> I'm also not entirely sure if that's the correct way of doing things
> since 'face seems to be working in other modes (most notably
> notmuch-show).  By posting, I'm also hoping that someone more
> experienced will maybe take a look to see what's going on here... ;)
> 
> For some more info, in notmuch-show mode, I get-text-property of nil for
> both 'face and 'font-lock-face, but in message-mode or in *scratch*
> buffer I'm getting nil for 'font-lock-face but non-nil for 'face.  With
> the patch, I'm getting the same non-nil for both 'face and
> 'font-lock-face; without the patch, I'm getting nil for both.

This is odd.  Could you give more details about your environment?  In
particular, what Emacs version are you running and might you have any
unusual Emacs packages installed or customizations set?

The unusual thing about font-lock-face is that it only gets applied if
font-lock is enabled; otherwise it's ignored.  One theory is that the
nil font-lock-face (without your patch) is causing font-lock to
*override* the face property that we set.  I don't think font-lock
usually does that, but perhaps in some situations, it will?

Re: [PATCH] emacs: notmuch-search: fix faces

[Michal Nazarewicz]

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

> Quoth Michal Nazarewicz on Aug 28 at  1:11 am:
>> I'm not an expert either, but with this patch applied I see colours,
>> without this patch, I don't see colours, ie. everything is rendered
>> using the default face.
>> 
>> I'm also not entirely sure if that's the correct way of doing things
>> since 'face seems to be working in other modes (most notably
>> notmuch-show).  By posting, I'm also hoping that someone more
>> experienced will maybe take a look to see what's going on here... ;)
>> 
>> For some more info, in notmuch-show mode, I get-text-property of nil for
>> both 'face and 'font-lock-face, but in message-mode or in *scratch*
>> buffer I'm getting nil for 'font-lock-face but non-nil for 'face.  With
>> the patch, I'm getting the same non-nil for both 'face and
>> 'font-lock-face; without the patch, I'm getting nil for both.

Austin Clements <amdragon@MIT.EDU> writes:
> This is odd.  Could you give more details about your environment?  In
> particular, what Emacs version are you running and might you have any
> unusual Emacs packages installed or customizations set?

I'm running Emacs compiled from a week old bzr head, emacs-version
reports: “GNU Emacs 24.2.50.1 (x86_64-unknown-linux-gnu) of 2012-08-21
on mpn-glaptop”.

I don't think I have any “unusual” packages, but I do have quite a bit
of customization, which you might find at
<https://github.com/mina86/dot-files/blob/master/dot-emacs>.

> The unusual thing about font-lock-face is that it only gets applied if
> font-lock is enabled; otherwise it's ignored.  One theory is that the
> nil font-lock-face (without your patch) is causing font-lock to
> *override* the face property that we set.  I don't think font-lock
> usually does that, but perhaps in some situations, it will?

Ha!  That could be the cause.  I've disabled global-font-lock-mode, and
the colours appeared.  Getting property returns non-nil for 'face and
nil for 'font-lock-face.  When I enable font-lock-mode the property seem
to disappear and disabling font-lock-mode again does not help.

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH] emacs: notmuch-search: fix faces

[Austin Clements]

Quoth Michal Nazarewicz on Aug 28 at  2:51 am:
> > Quoth Michal Nazarewicz on Aug 28 at  1:11 am:
> >> I'm not an expert either, but with this patch applied I see colours,
> >> without this patch, I don't see colours, ie. everything is rendered
> >> using the default face.
> >> 
> >> I'm also not entirely sure if that's the correct way of doing things
> >> since 'face seems to be working in other modes (most notably
> >> notmuch-show).  By posting, I'm also hoping that someone more
> >> experienced will maybe take a look to see what's going on here... ;)
> >> 
> >> For some more info, in notmuch-show mode, I get-text-property of nil for
> >> both 'face and 'font-lock-face, but in message-mode or in *scratch*
> >> buffer I'm getting nil for 'font-lock-face but non-nil for 'face.  With
> >> the patch, I'm getting the same non-nil for both 'face and
> >> 'font-lock-face; without the patch, I'm getting nil for both.
> 
> Austin Clements <amdragon@MIT.EDU> writes:
> > This is odd.  Could you give more details about your environment?  In
> > particular, what Emacs version are you running and might you have any
> > unusual Emacs packages installed or customizations set?
> 
> I'm running Emacs compiled from a week old bzr head, emacs-version
> reports: “GNU Emacs 24.2.50.1 (x86_64-unknown-linux-gnu) of 2012-08-21
> on mpn-glaptop”.
> 
> I don't think I have any “unusual” packages, but I do have quite a bit
> of customization, which you might find at
> <https://github.com/mina86/dot-files/blob/master/dot-emacs>.

I think the culprit is your "Show blanks and FIXME"
font-lock-mode-hook.

> > The unusual thing about font-lock-face is that it only gets applied if
> > font-lock is enabled; otherwise it's ignored.  One theory is that the
> > nil font-lock-face (without your patch) is causing font-lock to
> > *override* the face property that we set.  I don't think font-lock
> > usually does that, but perhaps in some situations, it will?
> 
> Ha!  That could be the cause.  I've disabled global-font-lock-mode, and
> the colours appeared.  Getting property returns non-nil for 'face and
> nil for 'font-lock-face.  When I enable font-lock-mode the property seem
> to disappear and disabling font-lock-mode again does not help.

It's possible we should use font-lock-face.  I'm not sure.  Poking
around the standard elisp, it looks like some things use 'face and
some things use 'font-lock-face.  I think 'face is more common, but
it's hard to grep for.

Re: [PATCH] emacs: notmuch-search: fix faces

[Michal Nazarewicz]

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

> Quoth Michal Nazarewicz on Aug 28 at  2:51 am:
>> I'm running Emacs compiled from a week old bzr head, emacs-version
>> reports: “GNU Emacs 24.2.50.1 (x86_64-unknown-linux-gnu) of 2012-08-21
>> on mpn-glaptop”.
>> 
>> I don't think I have any “unusual” packages, but I do have quite a bit
>> of customization, which you might find at
>> <https://github.com/mina86/dot-files/blob/master/dot-emacs>.

Austin Clements <amdragon@MIT.EDU> writes:
> I think the culprit is your "Show blanks and FIXME"
> font-lock-mode-hook.

No, I've commented it and the problem remained.

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH] emacs: notmuch-search: fix faces

[Tomi Ollila]

On Tue, Aug 28 2012, Michal Nazarewicz <mpn@google.com> wrote:

>> Quoth Michal Nazarewicz on Aug 28 at  2:51 am:
>>> I'm running Emacs compiled from a week old bzr head, emacs-version
>>> reports: “GNU Emacs 24.2.50.1 (x86_64-unknown-linux-gnu) of 2012-08-21
>>> on mpn-glaptop”.
>>> 
>>> I don't think I have any “unusual” packages, but I do have quite a bit
>>> of customization, which you might find at
>>> <https://github.com/mina86/dot-files/blob/master/dot-emacs>.
>
> Austin Clements <amdragon@MIT.EDU> writes:
>> I think the culprit is your "Show blanks and FIXME"
>> font-lock-mode-hook.
>
> No, I've commented it and the problem remained.

Interesting problem; You could try stripping all customizations
(emacs -q ...) and see whether coloring works -- and if it does
then "bisect" (or something) the culprit.

Whether we'd use 'face or 'font-lock-face in the future is another
issue which needs good explanation why such change is done.

> -- 
> Best regards,                                         _     _
> ..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)

Tomi

Re: [PATCH] emacs: notmuch-search: fix faces

[Michal Nazarewicz]

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

Tomi Ollila <tomi.ollila@iki.fi> writes:
> Interesting problem; You could try stripping all customizations
> (emacs -q ...) and see whether coloring works -- and if it does
> then "bisect" (or something) the culprit.

FYI, I've identified the problem.  I'll follow up when I find the
solution, or separate it into a tiny piece of code. :)

> Whether we'd use 'face or 'font-lock-face in the future is another
> issue which needs good explanation why such change is done.

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH] emacs: notmuch-search: fix faces

[Michal Nazarewicz]

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

On Thu, Aug 23 2012, Michal Nazarewicz wrote:
> For some reason the faces do not get applied when 'face property is
> used, but they work correctly with 'font-lock-face property.  This
> commit changes notmuch-search to use the latter.

OK, forget about this.  The problem “fixed itself” when I upgraded the
package that caused the issue.  Strange thing is that I cannot identify
any change in the package that could cause the problem to disappear.

Oh well, another life mystery.

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --]