bug#61567: 30.0.50; Define display-time-time-and-date-indicator face

bug#61567: 30.0.50; Define display-time-time-and-date-indicator face

[Protesilaos Stavrou]

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

Dear maintainers,

I have written a small patch for 'time.el'.  It adds a face to the
date+time of 'M-x display-time-mode'.

Should I install this on 'master'?  Any changes to make to it?

All the best,
Protesilaos (or simply "Prot")

-- 
Protesilaos Stavrou
https://protesilaos.com

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Define-display-time-time-and-date-indicator-face.patch --]
[-- Type: text/x-patch, Size: 2013 bytes --]

From 6f9cad322b002e796dc38deaf3ad10e679c6ac57 Mon Sep 17 00:00:00 2001
Message-Id: <6f9cad322b002e796dc38deaf3ad10e679c6ac57.1676538384.git.info@protesilaos.com>
From: Protesilaos Stavrou <info@protesilaos.com>
Date: Thu, 16 Feb 2023 11:05:53 +0200
Subject: [PATCH] Define 'display-time-time-and-date-indicator' face

* etc/NEWS: Announce the new face for the 'display-time-format'.
* lisp/time.el (display-time-time-and-date-indicator): Define new face.
(display-time-string-forms): Use the new face.
---
 etc/NEWS     | 7 +++++++
 lisp/time.el | 6 ++++++
 2 files changed, 13 insertions(+)

diff --git a/etc/NEWS b/etc/NEWS
index 4fbe09e0541..e05c288dd9f 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -56,6 +56,13 @@ This allows the user to customize the prompt that is appended by
 'yes-or-no-p' when asking questions.  The default value is
 "(yes or no) ".
 
+---
+** New face for 'display-time-format'
+The 'display-time-time-and-date-indicator' applies to the time and
+date component of the 'display-time-mode'.  By default, the face has
+all its attributes unspecified to preserve the indicator's familiar
+appearance.
+
 \f
 * Editing Changes in Emacs 30.1
 
diff --git a/lisp/time.el b/lisp/time.el
index f04a22dfd28..beffebbfa00 100644
--- a/lisp/time.el
+++ b/lisp/time.el
@@ -139,6 +139,11 @@ (defcustom display-time-mail-face nil
   :version "22.1"
   :type '(choice (const :tag "None" nil) face))
 
+(defface display-time-time-and-date-indicator nil
+  "Face of the `display-time-format'."
+  :group 'mode-line-faces
+  :version "30.1")
+
 (defvar display-time-mail-icon
   (find-image '((:type xpm :file "letter.xpm" :ascent center)
 		(:type pbm :file "letter.pbm" :ascent center)))
@@ -179,6 +184,7 @@ (defcustom display-time-string-forms
      (format-time-string (or display-time-format
 			     (if display-time-24hr-format "%H:%M" "%-I:%M%p"))
 			 now)
+     'face 'display-time-time-and-date-indicator
      'help-echo (format-time-string "%a %b %e, %Y" now))
     load
     (if mail
-- 
2.39.1

bug#61567: 30.0.50; Define display-time-time-and-date-indicator face

[Eli Zaretskii]

> From: Protesilaos Stavrou <info@protesilaos.com>
> Date: Fri, 17 Feb 2023 08:07:40 +0200
> 
> I have written a small patch for 'time.el'.  It adds a face to the
> date+time of 'M-x display-time-mode'.
> 
> Should I install this on 'master'?

Yes, please.

> Any changes to make to it?

See below.

> * etc/NEWS: Announce the new face for the 'display-time-format'.
> * lisp/time.el (display-time-time-and-date-indicator): Define new face.
> (display-time-string-forms): Use the new face.

Please remember to mention the bug number in the log message when you
install this.

> +---
> +** New face for 'display-time-format'
> +The 'display-time-time-and-date-indicator' applies to the time and
> +date component of the 'display-time-mode'.  By default, the face has
> +all its attributes unspecified to preserve the indicator's familiar
> +appearance.

This entry is IMO too long for such a simple and minor change.  Just
saying something like

  ** New face 'display-time-time-and-date-indicator'.
  This is used for displaying the time and date components of
  'display-time-mode'.

should be enough.

> +(defface display-time-time-and-date-indicator nil
> +  "Face of the `display-time-format'."
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
"Face for `display-time-format'."

Thanks.

bug#61567: 30.0.50; Define display-time-time-and-date-indicator face

[Protesilaos Stavrou]

> From: Eli Zaretskii <eliz@gnu.org>
> Date: Fri, 17 Feb 2023 08:37:32 +0200
>
>> From: Protesilaos Stavrou <info@protesilaos.com>
>> Date: Fri, 17 Feb 2023 08:07:40 +0200
>> 
>> I have written a small patch for 'time.el'.  It adds a face to the
>> date+time of 'M-x display-time-mode'.
>> 
>> Should I install this on 'master'?
>
> Yes, please.
>
>> Any changes to make to it?
>
> See below.

> [... 30 lines elided]

Thank you for the feedback!  I made the changes and installed the patch.
Closing the bug now.

-- 
Protesilaos Stavrou
https://protesilaos.com

bug#61567: 30.0.50; Define display-time-time-and-date-indicator face

[Robert Pluim]

>>>>> On Fri, 17 Feb 2023 08:37:32 +0200, Eli Zaretskii <eliz@gnu.org> said:
    Eli>   ** New face 'display-time-time-and-date-indicator'.
    Eli>   This is used for displaying the time and date components of
    Eli>   'display-time-mode'.

    Eli> should be enough.

    >> +(defface display-time-time-and-date-indicator nil
    >> +  "Face of the `display-time-format'."
    Eli>       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    Eli> "Face for `display-time-format'."

Thatʼs a very repetitive name. How about just
`display-time-indicator-face' (although I have a vague memory of there
being a guideline to not end faces in -face)

Robert
-- 

bug#61567: 30.0.50; Define display-time-time-and-date-indicator face

[Protesilaos Stavrou]

> From: Robert Pluim <rpluim@gmail.com>
> Date: Fri, 17 Feb 2023 11:31:08 +0100
>
>>>>>> On Fri, 17 Feb 2023 08:37:32 +0200, Eli Zaretskii <eliz@gnu.org> said:
>     Eli>   ** New face 'display-time-time-and-date-indicator'.
>     Eli>   This is used for displaying the time and date components of
>     Eli>   'display-time-mode'.
>
>     Eli> should be enough.
>
>     >> +(defface display-time-time-and-date-indicator nil
>     >> +  "Face of the `display-time-format'."
>     Eli>       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>     Eli> "Face for `display-time-format'."
>
> Thatʼs a very repetitive name. How about just
> `display-time-indicator-face' (although I have a vague memory of there
> being a guideline to not end faces in -face)

Indeed, the name is awkard.  I also remember that use of "-face" is
discouraged, but could not find the resource for it.

My initial idea was to use "display-time-face", which would also be
symmetric with "display-time-mail-face".  Note though that the latter is
defined as a 'defcustom', so I wasn't sure what the right approach is.

-- 
Protesilaos Stavrou
https://protesilaos.com

bug#61567: 30.0.50; Define display-time-time-and-date-indicator face

[Robert Pluim]

>>>>> On Fri, 17 Feb 2023 12:40:22 +0200, Protesilaos Stavrou <info@protesilaos.com> said:

    >> From: Robert Pluim <rpluim@gmail.com>
    >> Date: Fri, 17 Feb 2023 11:31:08 +0100
    >> 
    >>>>>>> On Fri, 17 Feb 2023 08:37:32 +0200, Eli Zaretskii <eliz@gnu.org> said:
    Eli> ** New face 'display-time-time-and-date-indicator'.
    Eli> This is used for displaying the time and date components of
    Eli> 'display-time-mode'.
    >> 
    Eli> should be enough.
    >> 
    >> >> +(defface display-time-time-and-date-indicator nil
    >> >> +  "Face of the `display-time-format'."
    Eli> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    Eli> "Face for `display-time-format'."
    >> 
    >> Thatʼs a very repetitive name. How about just
    >> `display-time-indicator-face' (although I have a vague memory of there
    >> being a guideline to not end faces in -face)

    Protesilaos> Indeed, the name is awkard.  I also remember that use of "-face" is
    Protesilaos> discouraged, but could not find the resource for it.

I canʼt find anything in the elisp manual. But I do note that all the
font-lock faces end in '-face'

    Protesilaos> My initial idea was to use "display-time-face", which would also be
    Protesilaos> symmetric with "display-time-mail-face".  Note though that the latter is
    Protesilaos> defined as a 'defcustom', so I wasn't sure what the right approach is.

`defface' defined faces are customizable, so I donʼt think it matters.

Robert
-- 

bug#61567: 30.0.50; Define display-time-time-and-date-indicator face

[Protesilaos Stavrou]

> From: Robert Pluim <rpluim@gmail.com>
> Date: Fri, 17 Feb 2023 11:58:25 +0100

> [... 21 lines elided]
>
>     Protesilaos> Indeed, the name is awkard.  I also remember that use of "-face" is
>     Protesilaos> discouraged, but could not find the resource for it.
>
> I canʼt find anything in the elisp manual. But I do note that all the
> font-lock faces end in '-face'
>
>     Protesilaos> My initial idea was to use "display-time-face", which would also be
>     Protesilaos> symmetric with "display-time-mail-face".  Note though that the latter is
>     Protesilaos> defined as a 'defcustom', so I wasn't sure what the right approach is.
>
> `defface' defined faces are customizable, so I donʼt think it matters.

Okay.  Any thoughts on renaming the face to 'display-time-face'?  Maybe
some other name is better?

-- 
Protesilaos Stavrou
https://protesilaos.com

bug#61567: 30.0.50; Define display-time-time-and-date-indicator face

[Basil Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Robert Pluim [2023-02-17 11:58 +0100] wrote:

>>>>>> On Fri, 17 Feb 2023 12:40:22 +0200, Protesilaos Stavrou <info@protesilaos.com> said:
>
>     Protesilaos> Indeed, the name is awkard.  I also remember that use of "-face" is
>     Protesilaos> discouraged, but could not find the resource for it.

> I canʼt find anything in the elisp manual.

It's mentioned in the description of defface (C-h S defface).

-- 
Basil

bug#61567: 30.0.50; Define display-time-time-and-date-indicator face

[Robert Pluim]

>>>>> On Fri, 17 Feb 2023 13:33:12 +0200, Protesilaos Stavrou <info@protesilaos.com> said:
    Protesilaos> Okay.  Any thoughts on renaming the face to 'display-time-face'?  Maybe
    Protesilaos> some other name is better?

Iʼm bad at naming at the best of times. 'display-time-face' sounds
fine to me (or even 'display-time' if we obey the edict in the elisp
manual).

Robert
-- 

bug#61567: 30.0.50; Define display-time-time-and-date-indicator face

[Eli Zaretskii]

> From: Protesilaos Stavrou <info@protesilaos.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, 61567@debbugs.gnu.org
> Date: Fri, 17 Feb 2023 13:33:12 +0200
> 
> Okay.  Any thoughts on renaming the face to 'display-time-face'?  Maybe
> some other name is better?

Please don't end the face's name with "-face".  I suggest
display-time-date-and-time instead.

bug#61567: 30.0.50; Define display-time-time-and-date-indicator face

[Robert Pluim]

>>>>> On Fri, 17 Feb 2023 14:35:45 +0200, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Protesilaos Stavrou <info@protesilaos.com>
    >> Cc: Eli Zaretskii <eliz@gnu.org>, 61567@debbugs.gnu.org
    >> Date: Fri, 17 Feb 2023 13:33:12 +0200
    >> 
    >> Okay.  Any thoughts on renaming the face to 'display-time-face'?  Maybe
    >> some other name is better?

    Eli> Please don't end the face's name with "-face".  I suggest
    Eli> display-time-date-and-time instead.

I think we have a winner

Robert
-- 

bug#61567: 30.0.50; Define display-time-time-and-date-indicator face

[Drew Adams]

> > I canʼt find anything in the elisp manual.
> 
> It's mentioned in the description of defface (C-h S defface).

I was going to say the same thing.  Perhaps
it should _also_ be said in the doc string
(but that's already a bit long, and it has
a link to the Elisp manual doc).  IMO that
would help.
___

As for the face name: This face is intended
(only) for the mode-line, no?  That's more
important (besides respecting the guideline)
than including "-face" in the name.

Maybe something like `mode-line-datetime'?

Standard faces used generally in the mode-line
have that prefix `mode-line-'.  Other faces
used in the mode-line (e.g. for different
modes) typically have "mode-line" as suffix
or infix.

mode-line 
mode-line-buffer-id 
mode-line-emphasis 
mode-line-highlight 
mode-line-inactive

compilation-mode-line-exit
compilation-mode-line-fail 
compilation-mode-line-run 

bug#61567: 30.0.50; Define display-time-time-and-date-indicator face

[Protesilaos Stavrou]

> From: Robert Pluim <rpluim@gmail.com>
> Date: Fri, 17 Feb 2023 13:43:14 +0100
>
>>>>>> On Fri, 17 Feb 2023 14:35:45 +0200, Eli Zaretskii <eliz@gnu.org> said:
>
>     >> From: Protesilaos Stavrou <info@protesilaos.com>
>     >> Cc: Eli Zaretskii <eliz@gnu.org>, 61567@debbugs.gnu.org
>     >> Date: Fri, 17 Feb 2023 13:33:12 +0200
>     >> 
>     >> Okay.  Any thoughts on renaming the face to 'display-time-face'?  Maybe
>     >> some other name is better?
>
>     Eli> Please don't end the face's name with "-face".  I suggest
>     Eli> display-time-date-and-time instead.
>
> I think we have a winner

I made the change.  Thank you!

-- 
Protesilaos Stavrou
https://protesilaos.com