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

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 73552 in the body.
You can then email your comments to 73552 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
To: Emacs Bug Report <bug-gnu-emacs <at> gnu.org>
Cc: Gerd Möllmann <gerd.moellmann <at> gmail.com>,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: obsolete (and broken) face attribute :reverse-video
Date: Sun, 29 Sep 2024 12:49:44 +0200

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.

Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Gerd Möllmann <gerd.moellmann <at> gmail.com>
To: Mattias Engdegård <mattias.engdegard <at> gmail.com>
Cc: Emacs Bug Report <bug-gnu-emacs <at> gnu.org>, Eli Zaretskii <eliz <at> gnu.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: obsolete (and broken) face attribute :reverse-video
Date: Sun, 29 Sep 2024 13:31:07 +0200

Mattias Engdegård <mattias.engdegard <at> 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 :-).)

Message #11 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
To: Gerd Möllmann <gerd.moellmann <at> gmail.com>
Cc: Emacs Bug Report <bug-gnu-emacs <at> gnu.org>, Eli Zaretskii <eliz <at> gnu.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: obsolete (and broken) face attribute :reverse-video
Date: Sun, 29 Sep 2024 14:35:01 +0200

29 sep. 2024 kl. 13.31 skrev Gerd Möllmann <gerd.moellmann <at> gmail.com>:

>> I first blamed a simple 14 years old mistake in `custom-fix-face-spec`
>> but fixing that doesn't help, not sure why.

Having said that I naturally had to find the bug and it appears to be `face-spec-set-2` which only sets attributes that are present in `face-x-resources` for some reason.

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

Thanks. Given Gerd's testimony, the lack of use and general brokenness, I propose we just give up on :reverse-video, perhaps removing it from xfaces.c.

Message #14 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mattias Engdegård <mattias.engdegard <at> gmail.com>
Cc: gerd.moellmann <at> gmail.com, bug-gnu-emacs <at> gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: obsolete (and broken) face attribute :reverse-video
Date: Sun, 29 Sep 2024 15:55:02 +0300

> From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
> Date: Sun, 29 Sep 2024 14:35:01 +0200
> Cc: Emacs Bug Report <bug-gnu-emacs <at> gnu.org>,
>  Eli Zaretskii <eliz <at> gnu.org>,
>  Stefan Monnier <monnier <at> iro.umontreal.ca>
> 
> 29 sep. 2024 kl. 13.31 skrev Gerd Möllmann <gerd.moellmann <at> gmail.com>:
> 
> >> I first blamed a simple 14 years old mistake in `custom-fix-face-spec`
> >> but fixing that doesn't help, not sure why.
> 
> Having said that I naturally had to find the bug and it appears to be `face-spec-set-2` which only sets attributes that are present in `face-x-resources` for some reason.
> 
> > 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.
> 
> Thanks. Given Gerd's testimony, the lack of use and general brokenness, I propose we just give up on :reverse-video, perhaps removing it from xfaces.c.

I'm not sure I understand the proposal: we remove :reverse-video, but
keep :inverse-video (which _is_ included in face-x-resources)?  Or did
you mean to remove :inverse-video as well?

Message #17 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: gerd.moellmann <at> gmail.com, bug-gnu-emacs <at> gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: obsolete (and broken) face attribute :reverse-video
Date: Sun, 29 Sep 2024 15:41:53 +0200

29 sep. 2024 kl. 14.55 skrev Eli Zaretskii <eliz <at> gnu.org>:

> I'm not sure I understand the proposal: we remove :reverse-video, but
> keep :inverse-video (which _is_ included in face-x-resources)?  Or did
> you mean to remove :inverse-video as well?

Leave face-x-resources as it is. Keep :inverse-video.
Remove all references to :reverse-video everywhere (except the newly introduced warning).

Message #20 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mattias Engdegård <mattias.engdegard <at> gmail.com>
Cc: gerd.moellmann <at> gmail.com, bug-gnu-emacs <at> gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: obsolete (and broken) face attribute :reverse-video
Date: Sun, 29 Sep 2024 16:49:39 +0300

> From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
> Date: Sun, 29 Sep 2024 15:41:53 +0200
> Cc: gerd.moellmann <at> gmail.com,
>  bug-gnu-emacs <at> gnu.org,
>  monnier <at> iro.umontreal.ca
> 
> 29 sep. 2024 kl. 14.55 skrev Eli Zaretskii <eliz <at> gnu.org>:
> 
> > I'm not sure I understand the proposal: we remove :reverse-video, but
> > keep :inverse-video (which _is_ included in face-x-resources)?  Or did
> > you mean to remove :inverse-video as well?
> 
> Leave face-x-resources as it is. Keep :inverse-video.
> Remove all references to :reverse-video everywhere (except the newly introduced warning).

I think the use of :reverse-video in make-mode.el should be changed to
:inverse-video.  Otherwise, I'm okay with retiring this old and
half-supported alias.

Message #23 received at 73552 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Mattias Engdegård <mattias.engdegard <at> gmail.com>, 
 73552 <at> debbugs.gnu.org
Cc: Gerd Möllmann <gerd.moellmann <at> gmail.com>,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#73552: obsolete (and broken) face attribute :reverse-video
Date: Sun, 29 Sep 2024 06:55:19 -0700

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

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

I suggest fixing this in make-mode.el to avoid the warning.

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

We could start by doing s/:bold t/:weight bold/ and
s/:italic t/:slant italic/ in our tree.

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

It doesn't seem worth fixing :reverse-video at this point, indeed.

I suggest removing it, given that it's been broken for so long and that
we now have a warning instead.

> - Should we remove :bold and :italic? Yes, but definitely not now.

I'd propose adding obsoletion warnings for :bold and :italic.  I note
that they are not documented in the elisp manual.

Message #28 received at 73552-done <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: Gerd Möllmann <gerd.moellmann <at> gmail.com>,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 73552-done <at> debbugs.gnu.org
Subject: Re: bug#73552: obsolete (and broken) face attribute :reverse-video
Date: Sun, 29 Sep 2024 18:22:42 +0200

29 sep. 2024 kl. 15.49 skrev Eli Zaretskii <eliz <at> gnu.org>:

> I think the use of :reverse-video in make-mode.el should be changed to
> :inverse-video.

Oh, certainly (compiling that file now yields a warning).

The few extant uses of :reverse-video discovered so far seem to be fall-back cases for non-colour displays, decidedly a rarity these days.

>  Otherwise, I'm okay with retiring this old and
> half-supported alias.

Thank you, now done.


29 sep. 2024 kl. 15.55 skrev Stefan Kangas <stefankangas <at> gmail.com>:

> We could start by doing s/:bold t/:weight bold/ and
> s/:italic t/:slant italic/ in our tree.

Yes, maybe later. At least these still work, and are widely used by external packages.

> I'd propose adding obsoletion warnings for :bold and :italic.  I note
> that they are not documented in the elisp manual.

Wr probably should but the compilation warnings only catch constant arguments to `defface`. Other uses (themes, font-lock, computed text properties, non-constant arguments to defface, etc) aren't caught; that would require a run-time warning.

Anyway, we're done with :reverse-video, thanks everyone!

Message #31 received at 73552 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mattias Engdegård <mattias.engdegard <at> gmail.com>
Cc: gerd.moellmann <at> gmail.com, stefankangas <at> gmail.com, monnier <at> iro.umontreal.ca,
 73552 <at> debbugs.gnu.org
Subject: Re: bug#73552: obsolete (and broken) face attribute :reverse-video
Date: Sun, 29 Sep 2024 19:42:09 +0300

> From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
> Date: Sun, 29 Sep 2024 18:22:42 +0200
> Cc: 73552-done <at> debbugs.gnu.org,
>  Gerd Möllmann <gerd.moellmann <at> gmail.com>,
>  Eli Zaretskii <eliz <at> gnu.org>,
>  Stefan Monnier <monnier <at> iro.umontreal.ca>
> 
> 29 sep. 2024 kl. 15.49 skrev Eli Zaretskii <eliz <at> gnu.org>:
> 
> >  Otherwise, I'm okay with retiring this old and
> > half-supported alias.
> 
> Thank you, now done.

Thanks.  Should the warning in bytecomp.el be reworded to the effect
that :reverse-video is no longer supported?

Message #34 received at 73552 <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: gerd.moellmann <at> gmail.com, stefankangas <at> gmail.com, monnier <at> iro.umontreal.ca,
 73552 <at> debbugs.gnu.org
Subject: Re: bug#73552: obsolete (and broken) face attribute :reverse-video
Date: Sun, 29 Sep 2024 19:25:33 +0200

29 sep. 2024 kl. 18.42 skrev Eli Zaretskii <eliz <at> gnu.org>:

> Should the warning in bytecomp.el be reworded to the effect
> that :reverse-video is no longer supported?

Yes, why not -- done.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.