bug#46973: 26.3; (elisp) `Face Remapping'

bug#46973: 26.3; (elisp) `Face Remapping'

[Drew Adams]

I'm not sure this is a bug, but the doc seems unclear to me - it seems
to contradict what I see.

Everywhere in this node, SPECS seems to be the same thing, modulo the
special indication of SPECS containing a nil value for
`face-remap-set-base'.  And that meaning seems to be (everywhere) that
SPECS can be "either a list of face names, or a property list of
attribute/value pairs."

But if I do something like this, where SPECS (being a &rest arg) is the list `(variable-pitch)', which is a list of face names:

(add-hook 'some-mode-hook
          (lambda ()
            (face-remap-set-base 'default 'variable-pitch)))

I get an error saying that `variable-pitch' isn't a list.
(And trying '(variable-pitch) instead gives the same error.)

I'm probably missing something - what is it?  Or is it possible that
`face-remap-set-base' has a different meaning of SPECS?

Feel free to close, if this isn't a (doc) bug, but I'd appreciate an
explanation, so I understand better.  Thx.


In GNU Emacs 26.3 (build 1, x86_64-w64-mingw32)
 of 2019-08-29
Repository revision: 96dd0196c28bc36779584e47fffcca433c9309cd
Windowing system distributor `Microsoft Corp.', version 10.0.19041
Configured using:
 `configure --without-dbus --host=x86_64-w64-mingw32
 --without-compress-install 'CFLAGS=-O2 -static -g3''

bug#46973: 26.3; (elisp) `Face Remapping'

[Eli Zaretskii]

> From: Drew Adams <drew.adams@oracle.com>
> Date: Sat, 6 Mar 2021 18:16:07 +0000
> 
> But if I do something like this, where SPECS (being a &rest arg) is the list `(variable-pitch)', which is a list of face names:
> 
> (add-hook 'some-mode-hook
>           (lambda ()
>             (face-remap-set-base 'default 'variable-pitch)))
> 
> I get an error saying that `variable-pitch' isn't a list.
> (And trying '(variable-pitch) instead gives the same error.)
> 
> I'm probably missing something - what is it?  Or is it possible that
> `face-remap-set-base' has a different meaning of SPECS?

The doc string of face-remap-set-base says:

  The remaining arguments, SPECS, specify the base of the remapping.
  Each one of SPECS should be either a face name or a property list
  of face attribute/value pairs, like in a ‘face’ text property.

bug#46973: [External] : Re: bug#46973: 26.3; (elisp) `Face Remapping'

[Drew Adams]

> > But if I do something like this, where SPECS (being a &rest arg) is the
> list `(variable-pitch)', which is a list of face names:
> >
> > (add-hook 'some-mode-hook
> >           (lambda ()
> >             (face-remap-set-base 'default 'variable-pitch)))
> >
> > I get an error saying that `variable-pitch' isn't a list.
> > (And trying '(variable-pitch) instead gives the same error.)
> >
> > I'm probably missing something - what is it?  Or is it possible that
> > `face-remap-set-base' has a different meaning of SPECS?
> 
> The doc string of face-remap-set-base says:
> 
>   The remaining arguments, SPECS, specify the base of the remapping.
>   Each one of SPECS should be either a face name or a property list
>   of face attribute/value pairs, like in a ‘face’ text property.

You just cited the same doc I cited.

In `(face-remap-set-base 'default 'variable-pitch)' do
you not agree that SPECS is the list `(variable-pitch)',
which is a list of one face name?

Again, what am I missing?

 M-x debug-on-entry face-remap-set-base

Stepping through shows that SPECS is indeed the list
`(variable-pitch)'.  The first (and only) spec is the
face (name) `variable-pitch'.

To me, it looks like either a doc bug or a code bug.
The `while' loop sets SPECS to `variable-pitch'.  And
the subsequent `if' tries to take its car.

Contrast with `face-remap-add-relative', which also
has an &rest SPECS arg, and whose elements can also
each be "a face name or a property list of face
attribute/value pairs."

The code for `face-remap-add-relative' does this:

  (while (and (consp specs) (null (cdr specs)))
    (setq specs (car specs)))

which is different from what `face-remap-set-base' does:

  (while (and (consp specs)
              (not (null (car specs)))
              (null (cdr specs)))
    (setq specs (car specs)))

I'm not saying that the two should necessarily be
the same.  I am saying that the latter doesn't
seem to work for SPECS that's a list of face names.

bug#46973: 26.3; (elisp) `Face Remapping'

[Lars Ingebrigtsen]

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

> (add-hook 'some-mode-hook
>           (lambda ()
>             (face-remap-set-base 'default 'variable-pitch)))
>
> I get an error saying that `variable-pitch' isn't a list.
> (And trying '(variable-pitch) instead gives the same error.)

Looks like this regression was introduced by cece37cf09de9, which just
wanted to simplify the specs list.  But then the test after that wasn't
adjusted.  I've now fixed this in Emacs 29.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#46973: [External] : Re: bug#46973: 26.3; (elisp) `Face Remapping'

[Drew Adams]

> > (add-hook 'some-mode-hook
> >           (lambda ()
> >             (face-remap-set-base 'default 'variable-pitch)))
> >
> > I get an error saying that `variable-pitch' isn't a list.
> > (And trying '(variable-pitch) instead gives the same error.)
> 
> Looks like this regression was introduced by cece37cf09de9, which just
> wanted to simplify the specs list.  But then the test after that wasn't
> adjusted.  I've now fixed this in Emacs 29.

Thanks for fixing this.  I downloaded face-remap.el
from MASTER, and I see that you applied the fix I
suggested: (if (or (null specs) (eq specs face)).

I see from the commit log that the regression was
introduced in 2008.  Which Emacs release was that?

[And how do I find that release @ from the web
interface to the repo, here?
https://repo.or.cz/emacs.git/commit/cece37cf09de9]

bug#46973: [External] : Re: bug#46973: 26.3; (elisp) `Face Remapping'

[Eli Zaretskii]

> From: Drew Adams <drew.adams@oracle.com>
> Date: Wed, 23 Feb 2022 15:56:21 +0000
> Cc: "46973@debbugs.gnu.org" <46973@debbugs.gnu.org>
> 
> I see from the commit log that the regression was
> introduced in 2008.  Which Emacs release was that?

Emacs 23.1.

> [And how do I find that release @ from the web
> interface to the repo, here?
> https://repo.or.cz/emacs.git/commit/cece37cf09de9]

I don't think you can.

bug#46973: [External] : Re: bug#46973: 26.3; (elisp) `Face Remapping'

[Drew Adams]

> > I see from the commit log that the regression was
> > introduced in 2008.  Which Emacs release was that?
> 
> Emacs 23.1.
> 
> > [And how do I find that release @ from the web
> > interface to the repo, here?
> >
> https://urldefense.com/v3/__https://repo.or.cz/emacs.git/commit/cece37cf09de9
> __;!!ACWV5N9M2RV99hQ!efT4BhwnL86QkgYo3P5AiPsR9THcH0JRhkI6cAB1EyCKVZXlVsna4KmG
> -JIMiqYG$ ]
> 
> I don't think you can.

I see.  Thank you.

bug#46973: 26.3; (elisp) `Face Remapping'

[Mauro Aranda]

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

> [And how do I find that release @ from the web
> interface to the repo, here?
> https://repo.or.cz/emacs.git/commit/cece37cf09de9]

Not sure if this is reliable enough, but you can click in "tree" and
then look for the configure.ac (or configure.in) file.  The second
argument for AC_INIT is the Emacs version for that revision.

bug#46973: [External] : Re: bug#46973: 26.3; (elisp) `Face Remapping'

[Drew Adams]

Thanks, Mauro!

> > [And how do I find that release # from the web
> > interface to the repo, here?
> > https://repo.or.cz/emacs.git/commit/cece37cf09de9]
> 
> Not sure if this is reliable enough, but you can click in "tree" and
> then look for the configure.ac (or configure.in) file.  The second
> argument for AC_INIT is the Emacs version for that revision.

bug#46973: 26.3; (elisp) `Face Remapping'

[Eli Zaretskii]

> Resent-From: Mauro Aranda <maurooaranda@gmail.com>
> Original-Sender: "Debbugs-submit" <debbugs-submit-bounces@debbugs.gnu.org>
> Resent-CC: bug-gnu-emacs@gnu.org
> Resent-Sender: help-debbugs@gnu.org
> From: Mauro Aranda <maurooaranda@gmail.com>
> Date: Wed, 23 Feb 2022 19:10:32 -0300
> Cc: 46973@debbugs.gnu.org
> 
> Drew Adams <drew.adams@oracle.com> writes:
> 
> > [And how do I find that release @ from the web
> > interface to the repo, here?
> > https://repo.or.cz/emacs.git/commit/cece37cf09de9]
> 
> Not sure if this is reliable enough, but you can click in "tree" and
> then look for the configure.ac (or configure.in) file.  The second
> argument for AC_INIT is the Emacs version for that revision.

AFAIU, that is not reliable enough for finding out the Emacs release
where a particular change was first part of the tarball.  This just
tells you what is the _next_ release of Emacs _from_that_branch_.

bug#46973: 26.3; (elisp) `Face Remapping'

[Mauro Aranda]

Eli Zaretskii <eliz@gnu.org> writes:

>> Drew Adams <drew.adams@oracle.com> writes:
>> 
>> > [And how do I find that release @ from the web
>> > interface to the repo, here?
>> > https://repo.or.cz/emacs.git/commit/cece37cf09de9]
>> 
>> Not sure if this is reliable enough, but you can click in "tree" and
>> then look for the configure.ac (or configure.in) file.  The second
>> argument for AC_INIT is the Emacs version for that revision.
>
> AFAIU, that is not reliable enough for finding out the Emacs release
> where a particular change was first part of the tarball.  This just
> tells you what is the _next_ release of Emacs _from_that_branch_.

Ah yes, you're right.