bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Drew Adams
Subject: bug#8396: 24.0.50; why use options (vars) instead of faces for apropos?
Date: Sun, 24 Apr 2011 13:07:28 -0700

> > 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.






reply via email to

[Prev in Thread] Current Thread [Next in Thread]