bug#73552: obsolete (and broken) face attribute :reverse-video

["Gerd Möllmann" <gerd.moellmann@gmail.com>]

Mattias Engdegård <mattias.engdegard@gmail.com> writes:

> The face attribute keyword :reverse-video has been broken in defface
> for a long time, and it's not used very much, so I added a compiler
> warning and a notice to NEWS.
>
> This keyword along with :bold and :italic have been obsolete for
> longer than that and aren't even mentioned in any documentation (on
> purpose) but in the internal face-and-display machinery they still
> partly work.
>
> I'm not quite sure why :reverse-video doesn't work in defface. Demo, in *scratch*:
>
>   (defface myface '((t :reverse-video t)) "my face")
>
>   (insert (propertize "bon bon" 'font-lock-face 'myface) ?\n)
>   => bon bon  ; not inverted
>
>   (insert (propertize "non non" 'font-lock-face '(:reverse-video t)) ?\n)
>   => non non  ; inverted
>
> I first blamed a simple 14 years old mistake in `custom-fix-face-spec`
> but fixing that doesn't help, not sure why. But it means that we
> probably don't need to fix it, and indeed there are precious few
> packages using :reverse-video. One is make-mode.el; outside Emacs, I
> only found one (yaml-mode).
>
> While :bold and :italic are obsolete as well, they are used a lot more
> (and actually seem to work) so I'm not warning about them now.
>
> To summarise:
> - Should we fix `custom-fix-face-spec`? Maybe, or just stop pretending that it handles :reverse-video.
> - Should we bother to fix what other mechanism preventing :reverse-video from working? Probably no.
> - Should we remove :reverse-video from the display machinery? Yes, but maybe not now?
> - Should we remove :bold and :italic? Yes, but definitely not now.

FWIW I'm pretty sure that I added :italic, :bold, :reverse-video to what
later became Emacs 21 only for backwards compatibility with the old
pre-21 face implementation. At least on the surface, because 21 faces
work completely differently. (I also seem to remeber that reverse-video
was a synonym for inverse-video in the old code, maybe even
undocumented. Too lazy to look that up ATM, though :-).)