Improving face customization in notmuch.el

Improving face customization in notmuch.el

[Matt Armstrong]

Hi folks, this is my first contribution to the project.  I'm new to
the git workflow, so go easy on me.  I also know next to nothing about
elisp.  ;-)

I work for Google, which strongly supports employees contributing to
OSS projects.  Some minor legal red tape must occur because it must be
clear that Google is the legal entity that owns whatever rights (and
liabilities!) exists for the work I do.  So, I use
marmstrong@google.com and not my personal mail, and the contributor
line should say Google and not "Matt Armstrong".  Of course, given
this is a GPL project, those rights are limited and well defined.

I can, if needed, supply whatever "Contributor License Agreement" is
needed if notmuch maintains these (as the FSF does with what they
typically call "papers").

My goal here is to make the out-of-box experience better for
notmuch.el users with respect to the choice of faces.  I find that on
terminals with few colors the fonts used by notmuch are often bad
(e.g. black text on a black background).  I had a heck of a time
figuring out how some of these face choices were made.  I want to both
improve the default faces and make them easier to discover and
customize.

Trivia: I wrote https://github.com/matta/rubymail 15 years ago before
I joined Google.  RubyMail was used by "sup" after sup's author
interned at Google and learned the power of the tags+threading+search
email model.  And then, sup inspired notmuch.  If you squint hard
enough you could say I'm already a notmuch contributor, it is just
that all the code I wrote has been deleted.  ;-)

[PATCH 1/2] Add Google Inc. to AUTHORS as a contributor.

[Matt Armstrong]

By virtue of being a Google employee I'm required to contribute
software in the name of Google and not myself.  Google retains
whatever rights (and liabilities) that come from having done so, not
myself.  As long as this is clear, and my association with Google is
clear, my employer is quite supportive of my making these
contributions.
---
 AUTHORS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/AUTHORS b/AUTHORS
index 6d0f2de..51d40b4 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -28,3 +28,4 @@ ideas, inspiration, testing or feedback):
 Martin Krafft
 Keith Packard
 Jamey Sharp
+Google Inc. (by way of Matt Armstrong <marmstrong@google.com>)
-- 
2.8.0.rc3.226.g39d4020

Re: [PATCH 1/2] Add Google Inc. to AUTHORS as a contributor.

[David Bremner]

Matt Armstrong <marmstrong@google.com> writes:

> By virtue of being a Google employee I'm required to contribute
> software in the name of Google and not myself.  Google retains
> whatever rights (and liabilities) that come from having done so, not
> myself.  As long as this is clear, and my association with Google is
> clear, my employer is quite supportive of my making these
> contributions.

I've no objection to this in principle, but

1) if we start updating AUTHORS after 7 years of neglect, I think we
need to do a more systematic job. I'm tempted to script the output of
"git shortlog" as part of the release process.

In id:1469191654-5922-1-git-send-email-david@tethera.net, I've posted
the beginnings of a mailmap file. What about adding something like

Google Inc. (by way of Matt Armstrong)  <marmstrong@google.com>

to the mailmap file?

If that sounds reasonable to you, I'll leave it to you to play with the
mailmap syntax to get what you want.


2) If you think the corporate overlords are fine with a mention of
contribution in AUTHORS, that's also fine for me, but note that this is
distinct from a claim of copyright.  There are copyright headers in
every source file, so if you want to claim copyright on behalf of your
employer (which is fine, I think we're not fussed about people adding
themselves to copyright headers for anything more than 2 lines of change
or so) then you'll want to update those headers. At least, that would be
that safer option, rather than thinking such a claim is implicit.

Re: [PATCH 1/2] Add Google Inc. to AUTHORS as a contributor.

[Matt Armstrong]

David Bremner <david@tethera.net> writes:

> Matt Armstrong <marmstrong@google.com> writes:
>
>> By virtue of being a Google employee I'm required to contribute
>> software in the name of Google and not myself.

[...]

> I've no objection to this in principle, but
>
> 1) if we start updating AUTHORS after 7 years of neglect, I think we
> need to do a more systematic job.

Looking into this further, if AUTHORS file is essentially unmaintained,
I need not touch it.


> I'm tempted to script the output of "git shortlog" as part of the
> release process.
>
> In id:1469191654-5922-1-git-send-email-david@tethera.net, I've posted
> the beginnings of a mailmap file. What about adding something like
>
> Google Inc. (by way of Matt Armstrong)  <marmstrong@google.com>
>
> to the mailmap file?
>
> If that sounds reasonable to you, I'll leave it to you to play with the
> mailmap syntax to get what you want.

I think the @google.com in my email address is enough.  I don't need to
tweak my display name to mention "Google Inc." via .mailcap.


> 2) If you think the corporate overlords are fine with a mention of
> contribution in AUTHORS, that's also fine for me, but note that this is
> distinct from a claim of copyright.  There are copyright headers in
> every source file, so if you want to claim copyright on behalf of your
> employer (which is fine, I think we're not fussed about people adding
> themselves to copyright headers for anything more than 2 lines of change
> or so) then you'll want to update those headers. At least, that would be
> that safer option, rather than thinking such a claim is implicit.

Thanks for thinking about this David.

The good news: in looking into this further, the issue of leaving (c)
claims in code is secondary.  The primary thing I'm supposed to do is
send patches with my work email.

If I become a significant author in some file, adding a (c) Google makes
sense if that the normal thing to do in notmuch.  I'll keep that in
mind.

Google's open source lawyers tell me that regardless of what (c) marks
appear in code, the author of code (e.g. significant patches) retains
copyright unless other arrangements are made, such as those you see in
FSF projects (https://www.gnu.org/licenses/why-assign.en.html).  When it
comes to a real legal investigation things like the git commit log come
into play.  A "(c) Google Inc." is nice to have, especially if a project
typically maintains them with some rigor, but it isn't required or
particularly significant.

Re: [PATCH 1/2] Add Google Inc. to AUTHORS as a contributor.

[David Bremner]

Matt Armstrong <marmstrong@google.com> writes:
>
> Thanks for thinking about this David.
>
> The good news: in looking into this further, the issue of leaving (c)
> claims in code is secondary.  The primary thing I'm supposed to do is
> send patches with my work email.
>
> If I become a significant author in some file, adding a (c) Google makes
> sense if that the normal thing to do in notmuch.  I'll keep that in
> mind.

Yeah, for example most people creating a new file will put their name in
the copyright header. Or maybe rewriting a significant amount.
But it's an individual judgement call.

>
> Google's open source lawyers tell me that regardless of what (c) marks
> appear in code, the author of code (e.g. significant patches) retains
> copyright unless other arrangements are made, such as those you see in
> FSF projects (https://www.gnu.org/licenses/why-assign.en.html).

Never argue with a happy lawyer, that's motto :). It sounds like we're
good to go; I merged patch 2/2 after moving the NEWS. Then I wanted to
shorten the summary line of the commit, which got a bit more intrusive
than I wanted, hopefully it conveys the intent of the existing message.

d

Re: [PATCH 1/2] Add Google Inc. to AUTHORS as a contributor.

[Matt Armstrong]

David Bremner <david@tethera.net> writes:

> Never argue with a happy lawyer, that's motto :). It sounds like we're
> good to go; I merged patch 2/2 after moving the NEWS. Then I wanted to
> shorten the summary line of the commit, which got a bit more intrusive
> than I wanted, hopefully it conveys the intent of the existing message.

Thanks David, especially for your patience with how to best handle the
copyright questions.

Re: [PATCH 1/2] Add Google Inc. to AUTHORS as a contributor.

[Nicolas Petton]

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

Matt Armstrong <marmstrong@google.com> writes:

> Looking into this further, if AUTHORS file is essentially unmaintained,
> I need not touch it.

The AUTHORS file is not unmaintained, why are you saying that?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: [PATCH 1/2] Add Google Inc. to AUTHORS as a contributor.

[Matt Armstrong]

Nicolas Petton <nicolas@petton.fr> writes:

> [ Unknown signature status ]
> Matt Armstrong <marmstrong@google.com> writes:
>
>> Looking into this further, if AUTHORS file is essentially unmaintained,
>> I need not touch it.
>
> The AUTHORS file is not unmaintained, why are you saying that?

I was going by David's related comment where he said the file had not
changed in seven years:
id:1469191654-5922-1-git-send-email-david@tethera.net.  So, it is not
maintained as an authoritative list of copyright holders, as is typical
in fastidiously maintained GNU software (i.e. those projects fully
subscribing to the FSF standards).  See
https://www.gnu.org/prep/maintain/html_node/Recording-Contributors.html

Personally, I think this is fine.  In the era of reliable DVCS that keep
revision history forever and have commands like git-annotate, AUTHORS
files in the GNU tradition are probably obsolete.  But, I'm not a
lawyer.  :-)

Re: [PATCH 1/2] Add Google Inc. to AUTHORS as a contributor.

[Nicolas Petton]

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

Matt Armstrong <marmstrong@google.com> writes:

>> The AUTHORS file is not unmaintained, why are you saying that?
>
> I was going by David's related comment where he said the file had not
> changed in seven years:
> id:1469191654-5922-1-git-send-email-david@tethera.net.  So, it is not
> maintained as an authoritative list of copyright holders

The only AUTHORS file I know about is etc/AUTHORS, which was last
updated on the emacs-25 branch by commit d08afa1 one week ago.

Cheers,
Nico

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: [PATCH 1/2] Add Google Inc. to AUTHORS as a contributor.

[Matt Armstrong]

Nicolas Petton <nicolas@petton.fr> writes:

> [ Unknown signature status ]
> Matt Armstrong <marmstrong@google.com> writes:
>
>>> The AUTHORS file is not unmaintained, why are you saying that?
>>
>> I was going by David's related comment where he said the file had not
>> changed in seven years:
>> id:1469191654-5922-1-git-send-email-david@tethera.net.  So, it is not
>> maintained as an authoritative list of copyright holders
>
> The only AUTHORS file I know about is etc/AUTHORS, which was last
> updated on the emacs-25 branch by commit d08afa1 one week ago.

Are you thinking of emacs?  This thread is about notmuch, not emacs.

Re: [PATCH 1/2] Add Google Inc. to AUTHORS as a contributor.

[Nicolas Petton]

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

Matt Armstrong <marmstrong@google.com> writes:

>> The only AUTHORS file I know about is etc/AUTHORS, which was last
>> updated on the emacs-25 branch by commit d08afa1 one week ago.
>
> Are you thinking of emacs?  This thread is about notmuch, not emacs.

Oh!  I did not even see that this wasn't on emacs-devel.  Sorry for the
noise.

Cheers,
Nico

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

[PATCH 2/2] emacs: express notmuch-search-line-faces in terms of two new faces: notmuch-search-flagged-face and notmuch-search-unread-face.

[Matt Armstrong]

This makes it easier to find the relevant face by customizing
notmuch-faces.  I plan to do the same to the other alists of faces
found elsewhere.
---
 NEWS             |  7 +++++++
 emacs/notmuch.el | 39 ++++++++++++++++++++++++++++++++-------
 2 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/NEWS b/NEWS
index ab661d4..49f8bb5 100644
--- a/NEWS
+++ b/NEWS
@@ -37,6 +37,13 @@ Respect charset of MIME parts when reading them
 
   Fix previous assumption that everyone uses UTF-8.
 
+Face customization is easier
+
+  New faces `notmuch-search-flagged-face` and
+  `notmuch-search-unread-face` are used by default by
+  `notmuch-search-line-faces`. Customize `notmuch-faces` to modify
+  them.
+
 Notmuch 0.22 (2016-04-26)
 =========================
 
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 8acdef3..43d56f7 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -311,6 +311,26 @@ there will be called at other points of notmuch execution."
   :group 'notmuch-search
   :group 'notmuch-faces)
 
+(defface notmuch-search-flagged-face
+  '((t
+     (:weight bold)))
+  "Face used in search mode face for flagged threads.
+
+This face is the default value for the \"flagged\" tag in
+`notmuch-search-line-faces`."
+  :group 'notmuch-search
+  :group 'notmuch-faces)
+
+(defface notmuch-search-unread-face
+  '((t
+     (:foreground "blue")))
+  "Face used in search mode for unread threads.
+
+This face is the default value for the \"unread\" tag in
+`notmuch-search-line-faces`."
+  :group 'notmuch-search
+  :group 'notmuch-faces)
+
 (defun notmuch-search-mode ()
   "Major mode displaying results of a notmuch search.
 
@@ -654,9 +674,12 @@ of the result."
 		  (goto-char (point-min))
 		  (forward-line (1- notmuch-search-target-line)))))))))
 
-(defcustom notmuch-search-line-faces '(("unread" :weight bold)
-				       ("flagged" :foreground "blue"))
-  "Tag/face mapping for line highlighting in notmuch-search.
+(defcustom notmuch-search-line-faces
+  '(("unread" 'notmuch-search-unread-face)
+    ("flagged" 'notmuch-search-flagged-face))
+  "Alist of tags to faces for line highlighting in notmuch-search.
+Each element looks like (TAG . FACE).
+A thread with TAG will have FACE applied.
 
 Here is an example of how to color search results based on tags.
  (the following text would be placed in your ~/.emacs file):
@@ -665,10 +688,12 @@ Here is an example of how to color search results based on tags.
                                    (\"deleted\" . (:foreground \"red\"
 						  :background \"blue\"))))
 
-The attributes defined for matching tags are merged, with earlier
-attributes overriding later. A message having both \"deleted\"
-and \"unread\" tags with the above settings would have a green
-foreground and blue background."
+The FACE must be a face name (a symbol or string), a property
+list of face attributes, or a list of these.  The faces for
+matching tags are merged, with earlier attributes overriding
+later. A message having both \"deleted\" and \"unread\" tags with
+the above settings would have a green foreground and blue
+background."
   :type '(alist :key-type (string) :value-type (custom-face-edit))
   :group 'notmuch-search
   :group 'notmuch-faces)
-- 
2.8.0.rc3.226.g39d4020

Re: [PATCH 2/2] emacs: express notmuch-search-line-faces in terms of two new faces: notmuch-search-flagged-face and notmuch-search-unread-face.

[David Bremner]

Matt Armstrong <marmstrong@google.com> writes:

> This makes it easier to find the relevant face by customizing
> notmuch-faces.  I plan to do the same to the other alists of faces
> found elsewhere.
> ---
>  NEWS             |  7 +++++++
>  emacs/notmuch.el | 39 ++++++++++++++++++++++++++++++++-------
>  2 files changed, 39 insertions(+), 7 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index ab661d4..49f8bb5 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -37,6 +37,13 @@ Respect charset of MIME parts when reading them
>  
>    Fix previous assumption that everyone uses UTF-8.
>  
> +Face customization is easier
> +
> +  New faces `notmuch-search-flagged-face` and
> +  `notmuch-search-unread-face` are used by default by
> +  `notmuch-search-line-faces`. Customize `notmuch-faces` to modify
> +  them.
> +

you actually want the first group of NEWS in the file, which is still
UNRELEASED.

d