Subject: Re: bug#28033: [PATCH] Add new face 'header-line-highlight'

From: Eli Zaretskii <[email-masked]>
Date: Sat, 12 Aug 2017 10:20:03 +0300

> From: Alex <[email-masked]>
> Cc: [email-masked]
> Date: Fri, 11 Aug 2017 16:10:02 -0600
> 
> > Thanks, this makes sense.  But please add some of this rationale to
> > the documentation.
> 
> I'm not sure exactly what you're looking for, but I added a brief
> explanation to the doc.

That's what I was looking for (although I made minor wording changes
in the actual commit).

> > Also, it is preferable to have the first line of a NEWS item be a full
> > sentence, if possible.  In this case, I would just say
> >
> >   ** New face 'header-line-highlight'.
> >
> > and then follow that by the details.
> 
> Sure. It seems that a lot of nearby entries don't follow that style,
> though...

Yes, something to work on.  Patches welcome.

I've pushed the changes.  A couple of minor comments for the future:

 . Please mention the bug number in the log message.
 . The order of the references to various parts of the changes in the
   log message should assume the reading order of top to bottom, so
   the log message might need some minor reordering.  In this case,
   your original order:

> * doc/emacs/display.texi (Standard Faces):
> * etc/NEWS: Document the face.
> * lisp/emacs-lisp/tabulated-list.el (tabulated-list-init-header):
> * lisp/info.el (Info-fontify-node): Use the face.
> * lisp/faces.el: Define the face.

   refers to "the face" before it was defined.  I've reordered it to
   put the reference to the lisp/faces.el change before all the rest.

Thanks again for working on this.