Customizing faces with `defcustom'

Customizing faces with `defcustom'

[martin rudalics]

A number of customization groups like matching, view, ispell, ibuffer,
use `(defcustom ... :type 'face)' for customizing faces.  However, the
associated tags are not assgined a face like `custom-variable-tag'.  As
a consequence, the associated customization buffers become less
intelligible.

Is there any reason not to use `defface' in these cases?

Re: Customizing faces with `defcustom'

[Richard M. Stallman]

    A number of customization groups like matching, view, ispell, ibuffer,
    use `(defcustom ... :type 'face)' for customizing faces.  

These are not faces, they are face-valued variables.

    However, the
    associated tags are not assgined a face like `custom-variable-tag'.

I don't understand that sentence at all, sorry.

Re: Customizing faces with `defcustom'

[martin rudalics]

 >     However, the
 >     associated tags are not assgined a face like `custom-variable-tag'.
 >
 > I don't understand that sentence at all, sorry.
 >
 >

Consider, for example, the customization buffer for `matching':

(1) `match' stands out in `custom-face-tag-face', by default scaled by
     1.2 and bold.

(2) `list-matching-lines-default-context-lines' is highlighted in
     `custom-variable-tag-face', by default scaled by 1.2 and bold.

Then come the options

(3) `list-matching-lines-face' and
     `list-matching-lines-buffer-name-face' both in default face.

This makes it more difficult to spot the latter and makes you believe
that these options are part of the text explaining
`list-matching-lines-default-context-lines'.  Shouldn't it be possible
to give _all_ "tags" of custom - variables (including face-valued ones),
faces, groups - a similar appearance with respect to height, pitch, and
the like.

Re: Customizing faces with `defcustom'

[Richard M. Stallman]

    Consider, for example, the customization buffer for `matching':

[Made by M-x customize-group RET matching RET]

    (1) `match' stands out in `custom-face-tag-face', by default scaled by
	 1.2 and bold.

    (2) `list-matching-lines-default-context-lines' is highlighted in
	 `custom-variable-tag-face', by default scaled by 1.2 and bold.


    Then come the options

    (3) `list-matching-lines-face' and
	 `list-matching-lines-buffer-name-face' both in default face.

That is a real bug.  These are variables, and their names should be
highlighted like all other variable names.

I think this is a consequence of the following definition for
widget type `face'.

    (define-widget 'face 'symbol
      "A Lisp face name (with sample)."
      :format "%t: (%{sample%}) %v"
      :tag "Face"
      :value 'default
      :sample-face-get 'widget-face-sample-face-get

I think that this :sample-face-get definition is affecting the
appearance of the variable's name.  The code in the definition of the
`face' widget was adapted from the `color' widget.  I tried this
example

    (defcustom foocolor "black"
      "Foo"
      :type 'color)

and saw the same problem occur.

I think it is a bug that the :sample-face-get of the type has an
effect on display of the variable name.  I would expect it to affect
only where the value is displayed.  But I have never understood the
widget code, so I have no idea how this happens.

Can anyone figure out how to fix that?

Re: Customizing faces with `defcustom'

[martin rudalics]

 >     Consider, for example, the customization buffer for `matching':
 >
 > [Made by M-x customize-group RET matching RET]
 >
 >     (1) `match' stands out in `custom-face-tag-face', by default scaled by
 > 	 1.2 and bold.
 >
 >     (2) `list-matching-lines-default-context-lines' is highlighted in
 > 	 `custom-variable-tag-face', by default scaled by 1.2 and bold.
 >
 >
 >     Then come the options
 >
 >     (3) `list-matching-lines-face' and
 > 	 `list-matching-lines-buffer-name-face' both in default face.
 >
 > That is a real bug.  These are variables, and their names should be
 > highlighted like all other variable names.
 >
 > I think this is a consequence of the following definition for
 > widget type `face'.
 >
 >     (define-widget 'face 'symbol
 >       "A Lisp face name (with sample)."
 >       :format "%t: (%{sample%}) %v"
 >       :tag "Face"
 >       :value 'default
 >       :sample-face-get 'widget-face-sample-face-get
 >
 > I think that this :sample-face-get definition is affecting the
 > appearance of the variable's name.  The code in the definition of the
 > `face' widget was adapted from the `color' widget.  I tried this
 > example
 >
 >     (defcustom foocolor "black"
 >       "Foo"
 >       :type 'color)
 >
 > and saw the same problem occur.
 >
 > I think it is a bug that the :sample-face-get of the type has an
 > effect on display of the variable name.  I would expect it to affect
 > only where the value is displayed.  But I have never understood the
 > widget code, so I have no idea how this happens.
 >
 > Can anyone figure out how to fix that?
 >


The solution appears deceptively simple:

*** cus-edit.el	Sat Nov  5 20:44:24 2005
--- cus-edit.el	Fri Nov 11 18:32:56 2005
***************
*** 3397,3403 ****

   (define-widget 'face 'symbol
     "A Lisp face name (with sample)."
!   :format "%t: (%{sample%}) %v"
     :tag "Face"
     :value 'default
     :sample-face-get 'widget-face-sample-face-get
--- 3397,3403 ----

   (define-widget 'face 'symbol
     "A Lisp face name (with sample)."
!   :format "%{%t%}: (%{sample%}) %v"
     :tag "Face"
     :value 'default
     :sample-face-get 'widget-face-sample-face-get

*** wid-edit.el	Sat Nov  5 20:44:42 2005
--- wid-edit.el	Fri Nov 11 18:34:24 2005
***************
*** 3575,3581 ****
   ;; Fixme: match
   (define-widget 'color 'editable-field
     "Choose a color name (with sample)."
!   :format "%t: %v (%{sample%})\n"
     :size 10
     :tag "Color"
     :value "black"
--- 3575,3581 ----
   ;; Fixme: match
   (define-widget 'color 'editable-field
     "Choose a color name (with sample)."
!   :format "%{%t%}: %v (%{sample%})\n"
     :size 10
     :tag "Color"
     :value "black"

Re: Customizing faces with `defcustom'

[Richard M. Stallman]

I will install your fix in cus-edit.el and wid-edit.el
in a week or so if no one reports a problem with it.

Thanks for fixing it.

Re: Customizing faces with `defcustom'

[martin rudalics]

> I will install your fix in cus-edit.el and wid-edit.el
> in a week or so if no one reports a problem with it.
> 

There is one problem: When I now open a customization buffer for
`basic-faces' and attempt to show the face of `mode-line' I get a
"Lisp nesting exceeds `max-lisp-eval-depth'" error.  Increasing
`max-lisp-eval-depth' from 300 to 303 solves the problem for me.
Anyway such behavior does not appear reasonable.

Re: Customizing faces with `defcustom'

[martin rudalics]

The problem reported in my last mail is obviously not related to the
change.  I did not byte-compile `cus-edit'.  Opening a customization
buffer for `basic-faces' and attempting to show the face of `mode-line'
always gets me a "Lisp nesting exceeds `max-lisp-eval-depth'" provided
`cus-edit.el' is not byte-compiled.

Re: Customizing faces with `defcustom'

[Richard M. Stallman]

    The problem reported in my last mail is obviously not related to the
    change.  I did not byte-compile `cus-edit'.  Opening a customization
    buffer for `basic-faces' and attempting to show the face of `mode-line'
    always gets me a "Lisp nesting exceeds `max-lisp-eval-depth'" provided
    `cus-edit.el' is not byte-compiled.

Lots of programs would give this result if they were not compiled.
Don't worry about that.

Re: Customizing faces with `defcustom'

[Juri Linkov]

> I will install your fix in cus-edit.el and wid-edit.el
> in a week or so if no one reports a problem with it.

This patch fixed the appearance for most face options in the
Customization buffer, but not all.

In the customization group `cpp' there is one face that still is
displayed incorrectly:

cpp-known-face: Hide Value 'invisible
   State: STANDARD. (mismatch)

It seems the wrong appearance is caused by the invalid value of this
face variable.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: Customizing faces with `defcustom'

[martin rudalics]

 > In the customization group `cpp' there is one face that still is
 > displayed incorrectly:
 >
 > cpp-known-face: Hide Value 'invisible
 >    State: STANDARD. (mismatch)
 >
 > It seems the wrong appearance is caused by the invalid value of this
 > face variable.

It's not related to the earlier problem.  I see three possibilities to
handle this:

1. Defface `invisible'

2. Replace the reference to invisible by a reference to an existing face
    like, for example, `shadow'

3. Define a new face say `cpp-invisible' and use that instead

Re: Customizing faces with `defcustom'

[Per Abrahamsen]

The cpp "faces" can be either true faces, or the special keyword
"invisible". 

2005-11-30  Per Abrahamsen  <abraham@dina.kvl.dk>

	* progmodes/cpp.el (cpp-face): New widget.
	(cpp-known-face, cpp-unknown-face, cpp-edit-list): Use it.

*** cpp.el.~1.28.~	2003-10-24 15:35:06.000000000 +0200
--- cpp.el	2005-11-30 10:11:27.000000000 +0100
***************
*** 57,70 ****
    :type 'file
    :group 'cpp)
  
  (defcustom cpp-known-face 'invisible
    "*Face used for known cpp symbols."
!   :type 'face
    :group 'cpp)
  
  (defcustom cpp-unknown-face 'highlight
    "*Face used for unknown cpp symbols."
!   :type 'face
    :group 'cpp)
  
  (defcustom cpp-face-type 'light
--- 57,75 ----
    :type 'file
    :group 'cpp)
  
+ 
+ (define-widget 'cpp-face 'lazy
+   "Either a face or the special symbol 'invisible'."
+   :type '(choice (const invisible) (face)))
+ 
  (defcustom cpp-known-face 'invisible
    "*Face used for known cpp symbols."
!   :type 'cpp-face
    :group 'cpp)
  
  (defcustom cpp-unknown-face 'highlight
    "*Face used for unknown cpp symbols."
!   :type 'cpp-face
    :group 'cpp)
  
  (defcustom cpp-face-type 'light
***************
*** 93,102 ****
  1. Face used for text that is `ifdef' the macro.
  2. Face used for text that is `ifndef' the macro.
  3. t, nil, or `both' depending on what text may be edited."
!   :type '(repeat (list string face face
! 		       (choice (const t)
! 			       (const nil)
! 			       (const both))))
    :group 'cpp)
  
  (defvar cpp-overlay-list nil)
--- 98,109 ----
  1. Face used for text that is `ifdef' the macro.
  2. Face used for text that is `ifndef' the macro.
  3. t, nil, or `both' depending on what text may be edited."
!   :type '(repeat (list (string :tag "Macro") 
!                        (cpp-face :tag "True") 
!                        (cpp-face :tag "False")
! 		       (choice (const :tag "True branch writable" t)
! 			       (const :tag "false branch writeable" nil)
! 			       (const :tag "Both branches writeable" both))))
    :group 'cpp)
  
  (defvar cpp-overlay-list nil)

Re: Customizing faces with `defcustom'

[Per Abrahamsen]

martin rudalics <rudalics@gmx.at> writes:

> It's not related to the earlier problem.  I see three possibilities to
> handle this:

My patch should handle that.

Re: Customizing faces with `defcustom'

[Juri Linkov]

>> It's not related to the earlier problem.  I see three possibilities to
>> handle this:
>
> My patch should handle that.

Since `invisible' is a special feature of cpp.el and it has nothing to do
with faces (i.e. it uses overlays to hide text blocks), then it seems
your patch is a better way to handle this than to use a fake face.

Your patch has one problem.  It changes the default values of elements
of `cpp-edit-list'.  Currently the default value is the face `default':

Cpp Edit List: Hide Value
INS DEL List:
            String: 
            Face: (sample) default
            Face: (sample) default
            Choice: Value Menu t

But your patch changes the default to the value `invisible':

Cpp Edit List: Hide Value
INS DEL List:
            Macro: 
            True: Choice: Value Menu invisible
            False: Choice: Value Menu invisible
            Choice: Value Menu True branch writable

I think it's better to keep old defaults.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: Customizing faces with `defcustom'

[martin rudalics]

 > My patch should handle that.

It probably still needs some fixing:

cpp-face-default-list: Hide Value nil
    State: CHANGED outside Customize; operating on it here may be unreliable. (mismatch)

Alist of faces you can choose from for cpp conditionals. Hide Rest
Each element has the form (STRING . FACE), where STRING
serves as a name (for `cpp-highlight-buffer' only)
and FACE is either a face (a symbol)
or a cons cell (background-color . COLOR).


cpp-face-light-list: Hide Value
INS DEL Cons-cell:
             String: light gray
             Choice: Value Menu Cons-cell:
             background-color
             String: light gray
...
INS
    State: CHANGED outside Customize; operating on it here may be unreliable.

Alist of names and faces to be used for light backgrounds.


cpp-face-none-list: Hide Value
'(("default" . default)
   ("invisible" . invisible))
    State: STANDARD. (mismatch)

Alist of names and faces available even if you don't use a window system.

Re: Customizing faces with `defcustom'

[Juri Linkov]

>> My patch should handle that.
>
> It probably still needs some fixing:
>
> cpp-face-default-list: Hide Value nil
>    State: CHANGED outside Customize; operating on it here may be unreliable. (mismatch)

Please note that there are more cases that need fixing.  All they can be
displayed in `emacs -Q' with `M-x customize-rogue RET'.

> cpp-face-none-list: Hide Value
> '(("default" . default)
>   ("invisible" . invisible))
>    State: STANDARD. (mismatch)

I want to remind also there are more mismatched face variables:

locate-header-face: Hide Value nil
   State: STANDARD. (mismatch)

delphi-other-face: Hide Value nil
   State: STANDARD. (mismatch)

...

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: Customizing faces with `defcustom'

[martin rudalics]

 > I want to remind also there are more mismatched face variables:
 >
 > locate-header-face: Hide Value nil
 >    State: STANDARD. (mismatch)
 >
 > delphi-other-face: Hide Value nil
 >    State: STANDARD. (mismatch)
 >
 > ...
 >

Richard's last proposal (2005-09-12 `delphi-other-face mismatch') was:

 > In that case, maybe the `face' custom type should accept nil
 > as an alternative.  That way, the existing default for
 > delphi-other-face would be valid, it would still work the same
 > as now, and it would not require changing the :type spec.
 >
 > Does anyone see any problems with that?

That closed the thread then.

Re: Customizing faces with `defcustom'

[Richard M. Stallman]

We don't have to fix all these rogue-by-default variables before the
release, but it would be nice to fix some more.

Meanwhile, this seems to fix cpp.el.  But it needs another fix,
because it doesn't know that some colors are available on ttys.


*** cpp.el	15 Dec 2005 20:52:23 -0500	1.33
--- cpp.el	25 Dec 2005 13:57:53 -0500	
***************
*** 189,195 ****
     '(("default" . default)
       ("invisible" . invisible))
     "Alist of names and faces available even if you don't use a window system."
!   :type '(repeat (cons string face))
    :group 'cpp)
  
  (defvar cpp-face-all-list
--- 189,195 ----
     '(("default" . default)
       ("invisible" . invisible))
     "Alist of names and faces available even if you don't use a window system."
!   :type '(repeat (cons string cpp-face))
    :group 'cpp)
  
  (defvar cpp-face-all-list

Re: Customizing faces with `defcustom'

[Richard M. Stallman]

    I want to remind also there are more mismatched face variables:

    locate-header-face: Hide Value nil
       State: STANDARD. (mismatch)

    delphi-other-face: Hide Value nil
       State: STANDARD. (mismatch)

I fixed those.  Which others are there?

Re: Customizing faces with `defcustom'

[Richard M. Stallman]

     > In that case, maybe the `face' custom type should accept nil
     > as an alternative.  That way, the existing default for
     > delphi-other-face would be valid, it would still work the same
     > as now, and it would not require changing the :type spec.
     >
     > Does anyone see any problems with that?

    That closed the thread then.

We never did that, and I think we fixed other such problems by
adding nil as an explicit alternative, so I fixed these two in that
way also.

Re: Customizing faces with `defcustom'

[martin rudalics]

 > I fixed those.  Which others are there?

The `makefile-shell' oddity

(defface makefile-shell
   ()
   ;;'((((class color) (min-colors 88) (background light)) (:background  "seashell1"))
   ;;  (((class color) (min-colors 88) (background dark)) (:background  "seashell4")))
   "Face to use for additionally highlighting Shell commands in Font-Lock mode."
   :group 'faces
   :group 'makefile
   :version "22.1")


Also I'd remove the makefile faces from the `faces' group - currently
they are the only progmode members to figure here - and move `button',
`show-paren-match', and `show-paren-mismatch' from `faces' to
`basic-faces'.

Re: Customizing faces with `defcustom'

[Richard M. Stallman]

    The `makefile-shell' oddity

    (defface makefile-shell
       ()
       ;;'((((class color) (min-colors 88) (background light)) (:background  "seashell1"))
       ;;  (((class color) (min-colors 88) (background dark)) (:background  "seashell4")))
       "Face to use for additionally highlighting Shell commands in Font-Lock mode."
       :group 'faces

It's not invalid, but it is not very useful.
We need someone who uses this mode to suggest what that
font should look like.  Is there some standard font-lock face
that would be good to inherit from, as a good default?

    Also I'd remove the makefile faces from the `faces' group - currently

I agree; I did that.

Re: Customizing faces with `defcustom'

[Juri Linkov]

>     Also I'd remove the makefile faces from the `faces' group - currently
>
> I agree; I did that.

There is still the last face in the group `faces' that doesn't belong to
this group: `display-time-mail-face'.  Currently it is in two groups:
`faces' and `display-time'.  So we could remove it from `faces'.

However, there is another similar face that is used to highlight text in
the mode line: `Buffer-menu-buffer'.  We could create a new group
`modeline-faces' and add all modeline-related faces to this new group.
The faces that could be added to `modeline-faces' are:

Buffer-menu-buffer
display-time-mail-face
mode-line-highlight
mode-line-inactive
mode-line

Or alternatively, add `Buffer-menu-buffer' and `display-time-mail-face' to
the existing group `modeline' (where three latter faces from the list above
already belong to), and add the parent group `faces' to the group `modeline'.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: Customizing faces with `defcustom'

[Juri Linkov]

>     I want to remind also there are more mismatched face variables:
>
>     locate-header-face: Hide Value nil
>        State: STANDARD. (mismatch)
>
>     delphi-other-face: Hide Value nil
>        State: STANDARD. (mismatch)
>
> I fixed those.  Which others are there?

I can find only one other face that has the nil value by default.
It is `glasses-face'.  Currently it uses `:type 'symbol', so it
has no problem.  But it would be more appropriate to use `:type 'face'
and add nil as an explicit alternative.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: Customizing faces with `defcustom'

[Juri Linkov]

>      > In that case, maybe the `face' custom type should accept nil
>      > as an alternative.  That way, the existing default for
>      > delphi-other-face would be valid, it would still work the same
>      > as now, and it would not require changing the :type spec.
>      >
>      > Does anyone see any problems with that?
>
>     That closed the thread then.
>
> We never did that, and I think we fixed other such problems by
> adding nil as an explicit alternative, so I fixed these two in that
> way also.

Other such cases also use a tag like "Default" or "None" for the nil face
value, because displaying "nil" in the customization buffer is not
user-friendly.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: Customizing faces with `defcustom'

[martin rudalics]

 > However, there is another similar face that is used to highlight text in
 > the mode line: `Buffer-menu-buffer'.  We could create a new group
 > `modeline-faces' and add all modeline-related faces to this new group.
 > The faces that could be added to `modeline-faces' are:
 >
 > Buffer-menu-buffer
 > display-time-mail-face
 > mode-line-highlight
 > mode-line-inactive
 > mode-line
 >

The question is whether the corresponding faces should be simultaneously
removed from the `basic-faces' group.  If they are removed where should
`header-line' - which inherits from `mode-line' - go to?

 > Or alternatively, add `Buffer-menu-buffer' and `display-time-mail-face' to
 > the existing group `modeline' (where three latter faces from the list above
 > already belong to), and add the parent group `faces' to the group `modeline'.
 >

`Buffer-menu-buffer' should definitely figure in the modeline group.
When I first tried to customize the modeline, `Buffer-menu-buffer' was
hard to spot and very undescriptive.  Since `display-time' is already
part of the modeline group I'd simply remove `display-time-mail-face'

Re: Customizing faces with `defcustom'

[Juri Linkov]

>> However, there is another similar face that is used to highlight text in
>> the mode line: `Buffer-menu-buffer'.  We could create a new group
>> `modeline-faces' and add all modeline-related faces to this new group.
>> The faces that could be added to `modeline-faces' are:
>>
>> Buffer-menu-buffer
>> display-time-mail-face
>> mode-line-highlight
>> mode-line-inactive
>> mode-line
>
> The question is whether the corresponding faces should be simultaneously
> removed from the `basic-faces' group.  If they are removed where should
> `header-line' - which inherits from `mode-line' - go to?

In this case I think the corresponding faces should not be removed from
the `basic-faces' group.  Only their group `modeline' could be replaced
with `modeline-faces'.

>> Or alternatively, add `Buffer-menu-buffer' and `display-time-mail-face' to
>> the existing group `modeline' (where three latter faces from the list above
>> already belong to), and add the parent group `faces' to the group `modeline'.
>
> `Buffer-menu-buffer' should definitely figure in the modeline group.
> When I first tried to customize the modeline, `Buffer-menu-buffer' was
> hard to spot and very undescriptive.  Since `display-time' is already
> part of the modeline group I'd simply remove `display-time-mail-face'

I agree.  Unless a new special face group `modeline-faces' will be created,
`display-time-mail-face' should be removed from the group `modeline'.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: Customizing faces with `defcustom'

[Richard M. Stallman]

    There is still the last face in the group `faces' that doesn't belong to
    this group: `display-time-mail-face'.  Currently it is in two groups:
    `faces' and `display-time'.  So we could remove it from `faces'.

Yes, I think we should.

    However, there is another similar face that is used to highlight text in
    the mode line: `Buffer-menu-buffer'.  We could create a new group
    `modeline-faces' and add all modeline-related faces to this new group.
    The faces that could be added to `modeline-faces' are:

    Buffer-menu-buffer
    display-time-mail-face
    mode-line-highlight
    mode-line-inactive
    mode-line

This seems ok, but please call it mode-line-faces.

Re: Customizing faces with `defcustom'

[Richard M. Stallman]

    I can find only one other face that has the nil value by default.
    It is `glasses-face'.  Currently it uses `:type 'symbol', so it
    has no problem.  But it would be more appropriate to use `:type 'face'
    and add nil as an explicit alternative.

Thanks.  I fixed that.

Re: Customizing faces with `defcustom'

[Richard M. Stallman]

    Other such cases also use a tag like "Default" or "None" for the nil face
    value, because displaying "nil" in the customization buffer is not
    user-friendly.

Please add that if you would like to.

Re: Customizing faces with `defcustom'

[Richard M. Stallman]

     > Buffer-menu-buffer
     > display-time-mail-face
     > mode-line-highlight
     > mode-line-inactive
     > mode-line
     >

    The question is whether the corresponding faces should be simultaneously
    removed from the `basic-faces' group.

I am not sure what you mean by "corresponding faces" in this context.
The face `mode-line' -- not something that "corresponds" to it,
but the very same symbol -- is in the `basic-faces' group
as well as the modeline group.

Please don't change the `basic-faces' group.

Re: Customizing faces with `defcustom'

[Richard M. Stallman]

    `Buffer-menu-buffer' should definitely figure in the modeline group.
    When I first tried to customize the modeline, `Buffer-menu-buffer' was
    hard to spot and very undescriptive.

What is the face `Buffer-menu-buffer' used for?
Where does it appear?  Its documentation is quite unclear,
and I would like to clear it up, but first I need to understand.
(Maybe its name should be changed too.)

Re: Customizing faces with `defcustom'

[Juri Linkov]

>     `Buffer-menu-buffer' should definitely figure in the modeline group.
>     When I first tried to customize the modeline, `Buffer-menu-buffer' was
>     hard to spot and very undescriptive.
>
> What is the face `Buffer-menu-buffer' used for?  Where does it appear?
> Its documentation is quite unclear, and I would like to clear it up, but
> first I need to understand.  (Maybe its name should be changed too.)

The main purpose of `Buffer-menu-buffer' is to highlight buffer names in
the *Buffer List* buffer (since it is defined in buff-menu.el).  But the
same face is also used to highlight buffer names in the mode line.

There should be a special separate face for the mode line.  What would
be a good name for it?  `mode-line-buffer'?  This is too specific.
`mode-line-bold'?  This name might become untrue when customized to
something else.

There are modes that could use this new face.  For example, info.el
highlights in bold only the Info buffer name which is not quite important.
But the Info node name in the mode line (which is more important than
the Info buffer name) is not highlighted.  This new face could be used
to highlighted the Info node name in the mode line as well.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: Customizing faces with `defcustom'

[martin rudalics]

 > I am not sure what you mean by "corresponding faces" in this context.
 > The face `mode-line' -- not something that "corresponds" to it,
 > but the very same symbol -- is in the `basic-faces' group
 > as well as the modeline group.

In fact I meant the faces `mode-line-highlight', `mode-line-inactive',
and `mode-line'.  And I wrongly suspected that Juri wanted to _move_
them to a mode-line-faces group.

Re: Customizing faces with `defcustom'

[martin rudalics]

 > What is the face `Buffer-menu-buffer' used for?
 > Where does it appear?  Its documentation is quite unclear,
 > and I would like to clear it up, but first I need to understand.
 > (Maybe its name should be changed too.)

Capitalized face / variable names are confusing, in particular with
`custom-buffer-sort-alphabetically' non-nil.  However, it's non-trivial
to detect that this very face is used in the modeline at all.

Re: Customizing faces with `defcustom'

[Richard M. Stallman]

    The main purpose of `Buffer-menu-buffer' is to highlight buffer names in
    the *Buffer List* buffer (since it is defined in buff-menu.el).  But the
    same face is also used to highlight buffer names in the mode line.

It seems like bad design to use the same face for both.

    There should be a special separate face for the mode line.  What would
    be a good name for it?  `mode-line-buffer'?  This is too specific.

`mode-line-buffer' would be a good name for a face used only to
highlight buffer names.  But if we want something a little
more general, how about `mode-line-emphasis'?

    There are modes that could use this new face.  For example, info.el
    highlights in bold only the Info buffer name which is not quite important.
    But the Info node name in the mode line (which is more important than
    the Info buffer name) is not highlighted.

Let's use `mode-line-emphasis' for that too.

Re: Customizing faces with `defcustom'

[Richard M. Stallman]

     > What is the face `Buffer-menu-buffer' used for?
     > Where does it appear?  Its documentation is quite unclear,
     > and I would like to clear it up, but first I need to understand.
     > (Maybe its name should be changed too.)

    Capitalized face / variable names are confusing, in particular with
    `custom-buffer-sort-alphabetically' non-nil.

Yes, I think it should be renamed to buffer-menu-buffer.
But first let's change the mode line not to use it.

Re: Customizing faces with `defcustom'

[Richard M. Stallman]

    In fact I meant the faces `mode-line-highlight', `mode-line-inactive',
    and `mode-line'.  And I wrongly suspected that Juri wanted to _move_
    them to a mode-line-faces group.

They should be in the new mode-line-faces group,
but some or all can be in basic-faces too.

Re: Customizing faces with `defcustom'

[Juri Linkov]

>     There should be a special separate face for the mode line.  What would
>     be a good name for it?  `mode-line-buffer'?  This is too specific.
>
> `mode-line-buffer' would be a good name for a face used only to
> highlight buffer names.  But if we want something a little
> more general, how about `mode-line-emphasis'?

I now think the best name would be the same as the variable name whose
propertized value is used in the mode-line to display buffer information.
I mean the variable `mode-line-buffer-identification'.  However, it is
too long for the face name, and the suffix `-identification' or even a
shorter one `-id' doesn't make the purpose of the face name clearer.

Selecting a good face name will also set up a rule for naming more
mode line faces that could be added after the release, e.g.
(existing variable vs. new face name):

mode-line-mule-info variable - mode-line-mule face
mode-line-modified variable - mode-line-modified face
mode-line-frame-identification variable - mode-line-frame face
mode-line-position variable - mode-line-position face
vc-mode variable - mode-line-vc-mode face
mode-line-modes variable - mode-line-modes face
...

So taking this into account, for now a good face name for buffer
information in the mode line would be `mode-line-buffer'.

In the patch below I added a new group `mode-line-faces'
with the following faces belonging to it:

mode-line
mode-line-inactive
mode-line-highlight
mode-line-buffer (new face)
display-time-mail-face

Index: lisp/faces.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/faces.el,v
retrieving revision 1.345
diff -c -r1.345 faces.el
*** lisp/faces.el	11 Dec 2005 11:08:34 -0000	1.345
--- lisp/faces.el	30 Dec 2005 06:27:47 -0000
***************
*** 1932,1937 ****
--- 1932,1943 ----
    "Basic face for highlighting."
    :group 'basic-faces)
  
+ (defgroup mode-line-faces nil
+   "Faces used in the mode line."
+   :group 'modeline
+   :group 'faces
+   :version "22.1")
+ 
  (defface mode-line-highlight
    '((((class color) (min-colors 88))
       :box (:line-width 2 :color "grey40" :style released-button))
***************
*** 1939,1945 ****
       :inherit highlight))
    "Basic mode line face for highlighting."
    :version "22.1"
!   :group 'modeline
    :group 'basic-faces)
  
  (defface region
--- 1945,1951 ----
       :inherit highlight))
    "Basic mode line face for highlighting."
    :version "22.1"
!   :group 'mode-line-faces
    :group 'basic-faces)
  
  (defface region
***************
*** 2014,2020 ****
       :inverse-video t))
    "Basic mode line face for selected window."
    :version "21.1"
!   :group 'modeline
    :group 'basic-faces)
  
  (defface mode-line-inactive
--- 2020,2026 ----
       :inverse-video t))
    "Basic mode line face for selected window."
    :version "21.1"
!   :group 'mode-line-faces
    :group 'basic-faces)
  
  (defface mode-line-inactive
***************
*** 2030,2036 ****
       :foreground "grey80" :background "grey30"))
    "Basic mode line face for non-selected windows."
    :version "22.1"
!   :group 'modeline
    :group 'basic-faces)
  
  ;; Make `modeline' an alias for `mode-line', for compatibility.
--- 2036,2042 ----
       :foreground "grey80" :background "grey30"))
    "Basic mode line face for non-selected windows."
    :version "22.1"
!   :group 'mode-line-faces
    :group 'basic-faces)
  
  ;; Make `modeline' an alias for `mode-line', for compatibility.
***************
*** 2038,2043 ****
--- 2044,2055 ----
  (put 'modeline-inactive 'face-alias 'mode-line-inactive)
  (put 'modeline-highlight 'face-alias 'mode-line-highlight)
  
+ (defface mode-line-buffer
+   '((t (:weight bold)))
+   "Face used to highlight buffer identification parts of the mode line."
+   :version "22.1"
+   :group 'mode-line-faces)
+ 
  (defface header-line
    '((default
       :inherit mode-line)
***************
*** 2077,2083 ****
    '((((type tty)) :inherit mode-line-inactive))
    "Face used for vertical window dividers on ttys."
    :version "22.1"
-   :group 'modeline
    :group 'basic-faces)
  
  (defface minibuffer-prompt
--- 2089,2094 ----

Index: lisp/bindings.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/bindings.el,v
retrieving revision 1.155
diff -c -r1.155 bindings.el
*** lisp/bindings.el	16 Dec 2005 11:35:25 -0000	1.155
--- lisp/bindings.el	30 Dec 2005 06:27:52 -0000
***************
*** 449,455 ****
  FMT is a format specifier such as \"%12b\".  This function adds
  text properties for face, help-echo, and local-map to it."
    (list (propertize fmt
! 		    'face 'Buffer-menu-buffer
  		    'help-echo
  		    (purecopy "mouse-1: previous buffer, mouse-3: next buffer")
  		    'mouse-face 'mode-line-highlight
--- 449,455 ----
  FMT is a format specifier such as \"%12b\".  This function adds
  text properties for face, help-echo, and local-map to it."
    (list (propertize fmt
! 		    'face 'mode-line-buffer
  		    'help-echo
  		    (purecopy "mouse-1: previous buffer, mouse-3: next buffer")
  		    'mouse-face 'mode-line-highlight

Index: lisp/time.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/time.el,v
retrieving revision 1.88
diff -c -r1.88 time.el
*** lisp/time.el	11 Aug 2005 01:53:17 -0000	1.88
--- lisp/time.el	30 Dec 2005 06:27:02 -0000
***************
*** 129,135 ****
  If `display-time-use-mail-icon' is non-nil, the image's
  background color is the background of this face.  Set this to
  make the mail indicator stand out on a color display."
!   :group 'faces
    :group 'display-time
    :version "22.1"
    :type '(choice (const :tag "None" nil) face))
--- 129,135 ----
  If `display-time-use-mail-icon' is non-nil, the image's
  background color is the background of this face.  Set this to
  make the mail indicator stand out on a color display."
!   :group 'mode-line-faces
    :group 'display-time
    :version "22.1"
    :type '(choice (const :tag "None" nil) face))

Index: lisp/info.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/info.el,v
retrieving revision 1.469
diff -c -r1.469 info.el
*** lisp/info.el	23 Dec 2005 21:52:00 -0000	1.469
--- lisp/info.el	30 Dec 2005 06:27:28 -0000
***************
*** 1403,1410 ****
  		(concat " ("
  			(if Info-current-file
  			    (file-name-nondirectory Info-current-file)
! 			  " ")
! 			") " (or Info-current-node ""))))))
  \f
  ;; Go to an Info node specified with a filename-and-nodename string
  ;; of the sort that is found in pointers in nodes.
--- 1403,1414 ----
  		(concat " ("
  			(if Info-current-file
  			    (file-name-nondirectory Info-current-file)
! 			  "")
! 			") "
! 			(if Info-current-node
! 			    (propertize Info-current-node
! 					'face 'mode-line-buffer)
! 			  ""))))))
  \f
  ;; Go to an Info node specified with a filename-and-nodename string
  ;; of the sort that is found in pointers in nodes.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: Customizing faces with `defcustom'

[martin rudalics]

I appreciate your proposal but for one issue: `mode-line-buffer' would
usually highlight that part of the modeline where "previous / next
buffer" mouse clicks operate on.  Doing this differently for Info
buffers appears counterintuitive.  On the other hand, it would be
convenient to make the modeline part occupied by the Info node name
sensitive for `Info-next' / `Info-prev' (or `Info-history-forward' /
`Info-history-back').

Re: Customizing faces with `defcustom'

[Richard M. Stallman]

    Selecting a good face name will also set up a rule for naming more
    mode line faces that could be added after the release, e.g.
    (existing variable vs. new face name):

    mode-line-mule-info variable - mode-line-mule face
    mode-line-modified variable - mode-line-modified face
    mode-line-frame-identification variable - mode-line-frame face
    mode-line-position variable - mode-line-position face

It is logical, but far too complicated, and not worth it.  I would
rather have just the one face, called mode-line-emphasis or
mode-line-location or mode-line-subject-matter, and use that for the
buffer name and the info node name, and for any other similar thing.

Re: Customizing faces with `defcustom'

[Juri Linkov]

>     mode-line-mule-info variable - mode-line-mule face
>     mode-line-modified variable - mode-line-modified face
>     mode-line-frame-identification variable - mode-line-frame face
>     mode-line-position variable - mode-line-position face
>
> It is logical, but far too complicated, and not worth it.  I would
> rather have just the one face, called mode-line-emphasis or
> mode-line-location or mode-line-subject-matter, and use that for the
> buffer name and the info node name, and for any other similar thing.

I think using one face for different parts of the mode line is not
useful.  Users might want to highlight the buffer name and the info
node name with different faces.  There are already several faces for
highlighting different parts of the Info header line (`info-header-node',
`info-header-xref'), so it should be natural to add a new Info face for
the node name in the mode line.

Since part of the mode line highlighted currently in bold is clearly
associated with the buffer name (it even has a keymap for switching the
buffer), a good face name should have the word `buffer' in its name.  Any
other name would be not intuitive and even useless for other parts of
the mode line since it won't allow highlighting other parts in different
faces.

In the previous messages you said that `mode-line-buffer' is a good name
for a face used only to highlight buffer names.  So let's add this face
and also `info-mode-line-node' which by default will inherit from
`mode-line-buffer'.

BTW, while looking at `mode-line-buffer-identification-keymap', I noticed
that currently it is broken due to the recent changes in `last-buffer' which
now uses a new frame parameter `buried-buffer-list'.  This means that
mouse-1 clicked on the buffer name in the mode line uses the buffer list
from the frame parameter `buried-buffer-list', but mouse-3 uses `bury-buffer'
to put the buffer to the bottom of the global buffer list.  Either both
mouse-1 and mouse-3 should use the frame-local buffer list or the global
buffer list.  I'm not sure which is better.  Since `previous-buffer' and
`next-buffer' (that use the frame-local buffer list) currently are unfinished
(they require more changes in C after the release) they don't work reliably,
so maybe mouse-1 and mouse-3 in the mode line should use the global buffer
list.  Even though it is not convenient, it exhibits predictable behavior.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: Customizing faces with `defcustom'

[Juri Linkov]

> I appreciate your proposal but for one issue: `mode-line-buffer' would
> usually highlight that part of the modeline where "previous / next
> buffer" mouse clicks operate on.  Doing this differently for Info
> buffers appears counterintuitive.  On the other hand, it would be
> convenient to make the modeline part occupied by the Info node name
> sensitive for `Info-next' / `Info-prev' (or `Info-history-forward' /
> `Info-history-back').

Good idea.  I tried the following code and it works well.

(defvar Info-mode-line-node-keymap
  (let ((map (make-sparse-keymap)))
    (define-key map [mode-line mouse-1] 'Info-prev)
    (define-key map [mode-line mouse-3] 'Info-next)
    map)
  "Keymap to put on the Info node name in the mode line.")

(defun Info-set-mode-line ()
  (setq mode-line-buffer-identification
	(nconc (propertized-buffer-identification "%b")
	       (list
		(concat
		 " ("
		 (if Info-current-file
		     (file-name-nondirectory Info-current-file)
		   "")
		 ") "
		 (if Info-current-node
		     (propertize Info-current-node
				 'face 'info-mode-line-node
				 'help-echo
				 "mouse-1: previous node, mouse-3: next node"
				 'mouse-face 'mode-line-highlight
				 'local-map Info-mode-line-node-keymap)
		   ""))))))

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: Customizing faces with `defcustom'

[Richard M. Stallman]

    In the patch below I added a new group `mode-line-faces'
    with the following faces belonging to it:

    mode-line
    mode-line-inactive
    mode-line-highlight
    mode-line-buffer (new face)
    display-time-mail-face

Could you install that, with mode-line-buffer renamed
to mode-line-subject-matter?

Re: Customizing faces with `defcustom'

[Richard M. Stallman]

    I think using one face for different parts of the mode line is not
    useful.  Users might want to highlight the buffer name and the info
    node name with different faces.

There are an unlimited range of features that users "might want to"
use, but we certainly don't want to add them all.  So that's not a
basis for making this decision.

    In the previous messages you said that `mode-line-buffer' is a good name
    for a face used only to highlight buffer names.  So let's add this face
    and also `info-mode-line-node' which by default will inherit from
    `mode-line-buffer'.

I'd prefer to add one face rather than two.

If lots of users ask for that distinction, we can oblige them later.

Re: Customizing faces with `defcustom'

[Richard M. Stallman]

    (defvar Info-mode-line-node-keymap
      (let ((map (make-sparse-keymap)))
	(define-key map [mode-line mouse-1] 'Info-prev)
	(define-key map [mode-line mouse-3] 'Info-next)
	map)
      "Keymap to put on the Info node name in the mode line.")

    ...

It sounds good.  Please install it.

Re: Customizing faces with `defcustom'

[Richard M. Stallman]

      I'm not sure which is better.  Since `previous-buffer' and
    `next-buffer' (that use the frame-local buffer list) currently are unfinished
    (they require more changes in C after the release) they don't work reliably,
    so maybe mouse-1 and mouse-3 in the mode line should use the global buffer
    list.  Even though it is not convenient, it exhibits predictable behavior.

I agree.  Could you fix that?

Re: Customizing faces with `defcustom'

[Juri Linkov]

>     In the patch below I added a new group `mode-line-faces'
>     with the following faces belonging to it:
>
>     mode-line
>     mode-line-inactive
>     mode-line-highlight
>     mode-line-buffer (new face)
>     display-time-mail-face
>
> Could you install that, with mode-line-buffer renamed
> to mode-line-subject-matter?

I think `mode-line-subject-matter' is the worst name among all proposed
so far.  For the user wanting to customize the appearance of the buffer
name in the mode line, the face with `subject' or `matter' in its name is
the last thing to look at.

Meanwhile, I found that XEmacs uses the name `mode-line-buffer-id' to
highlight the string from the variable `mode-line-buffer-identification'
in the mode line.  It seems a good name.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: Customizing faces with `defcustom'

[Juri Linkov]

>     I think using one face for different parts of the mode line is not
>     useful.  Users might want to highlight the buffer name and the info
>     node name with different faces.
>
> There are an unlimited range of features that users "might want to"
> use, but we certainly don't want to add them all.  So that's not a
> basis for making this decision.

I remember that more mode-line faces were already proposed on emacs-devel,
and you agreed to install them, but these faces are still not in CVS.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: Customizing faces with `defcustom'

[Richard M. Stallman]

    Meanwhile, I found that XEmacs uses the name `mode-line-buffer-id' to
    highlight the string from the variable `mode-line-buffer-identification'
    in the mode line.  It seems a good name.

Ok.

Re: Customizing faces with `defcustom'

[Richard M. Stallman]

    I remember that more mode-line faces were already proposed on emacs-devel,
    and you agreed to install them, but these faces are still not in CVS.

I don't remember anything about a previous discussion, but it could be
that I reached a different conclusion this time.  I would rather not
install a lot of them.