bug#8396: 24.0.50; why use options (vars) instead of faces for apropos?

bug#8396: 24.0.50; why use options (vars) instead of faces for apropos?

[Drew Adams]

Throughout apropos.el, we use `defcustom's instead of `defface's for
customizing the faces used.  Why?  Apropos should have its own faces,
not variables that can be assigned to only existing faces (that have
nothing to do with apropos).
 
Try, for instance, C-u C-x = on the bold text in *Apropos*.  You'll see
this:
 
There are text properties here:
  button               (t)
  category             apropos-symbol-button
  face                 bold    <====== WHAT'S THAT ABOUT?
  skip                 t
 
That doesn't help a user understand how to change the face used here.
S?he shouldn't think that s?he can only customize face `bold' to take
care of this.  And there is nothing to indicate to the user that there
is a customizable variable (`apropos-symbol-face') that is relevant
for this.  The user should have a real apropos face to customize.
 
There is no reason to avoid creating faces for the needs of apropos (or
anything else, for that matter.)  Faces are customizable by design -
there is no reason to resort to adding customizable variables when what
is wanted is customizing the appearance (faces).
 

In GNU Emacs 24.0.50.1 (i386-mingw-nt5.1.2600)
 of 2011-03-21 on 3249CTO
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (4.5) --no-opt --cflags
-Ic:/imagesupport/include'
 

bug#8396: 24.0.50; why use options (vars) instead of faces for apropos?

[Chong Yidong]

"Drew Adams" <drew.adams@oracle.com> writes:

> Throughout apropos.el, we use `defcustom's instead of `defface's for
> customizing the faces used.  Why?

Bad historical design.  I don't see a good way to revert this, though.

bug#8396: 24.0.50; why use options (vars) instead of faces for apropos?

[Drew Adams]

> > Throughout apropos.el, we use `defcustom's instead of `defface's for
> > customizing the faces used.  Why?
> 
> Bad historical design.  I don't see a good way to revert this, though.

Surely this is not the first time Emacs has transitioned from variables to
faces. ;-)

Here's one suggestion: Start using `defface's and their resulting faces.
Declare the corresponding options obsolete.

If deemed absolutely necessary, the code could, for now, use either the option
value (if non-nil) or the face.  Eventually, we would remove the options.

Something like this, perhaps: (use-a-face (or the-face-option 'the-face))

E.g.:

(defcustom apropos-symbol-face nil
  "DOC MENTIONING IT IS OBSOLETE and to use face `apropos-symbol-face' instead."
  :group 'apropos :type '(choice (const :tag "None" nil) face))

(define-button-type 'apropos-symbol
  'face (or apropos-symbol-face 'apropos-symbol-face)
  'help-echo "mouse-2, RET: Display more help on this symbol"
  'follow-link t
  'action #'apropos-symbol-button-display-help)

Note the :type change for the option.

The current :type of `face' is incorrect - in all of the apropos.el face options
(another bug).  The doc strings for these face options, and some of the code
that uses them, expect that the value can be nil, meaning to use no face.  This
probably dates from ancient defvars, before defcustom. 

But the defcustom :type is `face' for each of them, which precludes using nil as
the value.  You cannot edit the value to `nil' and then use that, because `nil'
is not a face.  And if you use (setq apropos-symbol-face nil) then Customize
shows a type mismatch.

Note too this comment in the code of `apropos-print', which cries out for the
fix this bug requests:

;; Can't use default, since user may have changed the variable!
;; Just say `no' to variables containing faces!

And note the bugged code of `apropos-describe-plist', which in fact tests
whether variable `apropos-symbol-face' is nil (which it cannot be without
mismatching its :type).

In sum, please consider biting the bullet and getting rid of these face options.

bug#8396: 24.0.50; why use options (vars) instead of faces for apropos?

[Chong Yidong]

"Drew Adams" <drew.adams@oracle.com> writes:

> Throughout apropos.el, we use `defcustom's instead of `defface's for
> customizing the faces used.  Why?

Fixed now in trunk.