bug#54115: 27.2; `face-remap-set-base' bug with only two args

bug#54115: 27.2; `face-remap-set-base' bug with only two args

[Drew Adams]

(I see the same problem (same code) in an Emacs 28 pretest snapshot.)

emacs -Q

Eval the source-code defun of `face-remap-set-base'.

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

Evaluate (face-remap-set-base 'variable-pitch 'default).

Debugger entered--Lisp error: (wrong-type-argument listp default)
* car(default)
* (eq (car specs) face)
* (and (eq (car specs) face) (null (cdr specs)))
* (or (null specs) (and (eq (car specs) face) (null (cdr specs))))
* (if (or (null specs) (and (eq (car specs) face) (null (cdr specs)))) (face-remap-reset-base face) (make-local-variable 'face-remapping-alist) (let ((entry (assq face face-remapping-alist))) (if entry (setcar (last entry) specs) (setq face-remapping-alist (cons (list face specs) face-remapping-alist)))) (force-mode-line-update))
* (closure (t) (face &rest specs) "Set the base remapping of FACE in the current buff..." (while (and (consp specs) (not (null (car specs))) (null (cdr specs))) (setq specs (car specs))) (if (or (null specs) (and (eq (car specs) face) (null (cdr specs)))) (face-remap-reset-base face) (make-local-variable 'face-remapping-alist) (let ((entry (assq face face-remapping-alist))) (if entry (setcar (last entry) specs) (setq face-remapping-alist (cons (list face specs) face-remapping-alist)))) (force-mode-line-update)))(variable-pitch default)
* apply((closure (t) (face &rest specs) "Set the base remapping of FACE in the current buff..." (while (and (consp specs) (not (null (car specs))) (null (cdr specs))) (setq specs (car specs))) (if (or (null specs) (and (eq (car specs) face) (null (cdr specs)))) (face-remap-reset-base face) (make-local-variable 'face-remapping-alist) (let ((entry (assq face face-remapping-alist))) (if entry (setcar (last entry) specs) (setq face-remapping-alist (cons (list face specs) face-remapping-alist)))) (force-mode-line-update))) (variable-pitch default))
* face-remap-set-base(variable-pitch default)
  (progn (face-remap-set-base 'variable-pitch 'default))
  eval((progn (face-remap-set-base 'variable-pitch 'default)) t)
  elisp--eval-last-sexp(nil)
  eval-last-sexp(nil)
  funcall-interactively(eval-last-sexp nil)
  call-interactively(eval-last-sexp nil nil)
  command-execute(eval-last-sexp)

Should the test perhaps be this:

(if (or (null specs) (eq specs face)) ; default

instead of this?

(if (or (null specs)
	(and (eq (car specs) face)
        (null (cdr specs)))) ; default


In GNU Emacs 27.2 (build 1, x86_64-w64-mingw32)
 of 2021-03-26 built on CIRROCUMULUS
Repository revision: deef5efafb70f4b171265b896505b92b6eef24e6
Repository branch: HEAD
Windowing system distributor 'Microsoft Corp.', version 10.0.19043
System Description: Microsoft Windows 10 Pro (v10.0.2009.19043.1526)

bug#54115: 27.2; `face-remap-set-base' bug with only two args

[Eli Zaretskii]

> From: Drew Adams <drew.adams@oracle.com>
> Date: Tue, 22 Feb 2022 17:47:33 +0000
> 
> (I see the same problem (same code) in an Emacs 28 pretest snapshot.)
> 
> emacs -Q
> 
> Eval the source-code defun of `face-remap-set-base'.
> 
> M-x debug-on-entry face-remap-set-base
> 
> Evaluate (face-remap-set-base 'variable-pitch 'default).
> 
> Debugger entered--Lisp error: (wrong-type-argument listp default)
> * car(default)
> * (eq (car specs) face)
> * (and (eq (car specs) face) (null (cdr specs)))

How is this different from bug#46973?

bug#54115: [External] : Re: bug#54115: 27.2; `face-remap-set-base' bug with only two args

[Drew Adams]

> > (I see the same problem (same code) in an Emacs 28 pretest snapshot.)
> >
> > emacs -Q
> >
> > Eval the source-code defun of `face-remap-set-base'.
> >
> > M-x debug-on-entry face-remap-set-base
> >
> > Evaluate (face-remap-set-base 'variable-pitch 'default).
> >
> > Debugger entered--Lisp error: (wrong-type-argument listp default)
> > * car(default)
> > * (eq (car specs) face)
> > * (and (eq (car specs) face) (null (cdr specs)))
> 
> How is this different from bug#46973?

1. I'd forgotten about that bug filing.  Thx.
2. That's a doc bug.  This is a behavior bug.

So far, there's been no improvement of the doc
or correction of the behavior.

Feel free to merge the two bugs, if you like.

And please let us know how to pass a single
face for SPECS.  Clearly passing a list with
only one face doesn't work.

Labeling this "wishlist" doesn't help anyone
trying to use `face-remap-set-base' to set
the base remapping of FACE to another (single)
face.
___

The doc says this:

"If SPECS is empty or a single face 'eq' to FACE, call 'face-remap-reset-base'
to use the normal definition of FACE as the base remapping"

But SPECS is specifies with &rest, which means
that it's necessarily a _list_ (empty or not).
There's no way that SPECS can be a single face.
SPECS can be a list that contains only a single
face, but it can't be a face.

The doc string apparently gets mixed up from
the fact that the code in some cases resets
SPEC to the single face it contains.  But as
an argument SPECS is always a list.

bug#54115: [External] : Re: bug#54115: 27.2; `face-remap-set-base' bug with only two args

[Eli Zaretskii]

> From: Drew Adams <drew.adams@oracle.com>
> CC: "54115@debbugs.gnu.org" <54115@debbugs.gnu.org>
> Date: Tue, 22 Feb 2022 18:45:11 +0000
> 
> > > (I see the same problem (same code) in an Emacs 28 pretest snapshot.)
> > >
> > > emacs -Q
> > >
> > > Eval the source-code defun of `face-remap-set-base'.
> > >
> > > M-x debug-on-entry face-remap-set-base
> > >
> > > Evaluate (face-remap-set-base 'variable-pitch 'default).
> > >
> > > Debugger entered--Lisp error: (wrong-type-argument listp default)
> > > * car(default)
> > > * (eq (car specs) face)
> > > * (and (eq (car specs) face) (null (cdr specs)))
> > 
> > How is this different from bug#46973?
> 
> 1. I'd forgotten about that bug filing.  Thx.
> 2. That's a doc bug.  This is a behavior bug.

Then how is it different from bug#45264?

> So far, there's been no improvement of the doc
> or correction of the behavior.

So which is it -- a doc bug or a behavior bug?

> And please let us know how to pass a single
> face for SPECS.

You can't.  A face spec doesn't allow single faces.  See defface.

bug#54115: [External] : Re: bug#54115: 27.2; `face-remap-set-base' bug with only two args

[Drew Adams]

> > > How is this different from bug#46973?
> >
> > 1. I'd forgotten about that bug filing.  Thx.
> > 2. That's a doc bug.  This is a behavior bug.
> 
> Then how is it different from bug#45264?
> 
> > So far, there's been no improvement of the doc
> > or correction of the behavior.
> 
> So which is it -- a doc bug or a behavior bug?

I won't argue.  It seems to me that following the
doc/description, the behavior observed is not what
is described.  Is there a doc bug, a behavior bug,
or both?

> > And please let us know how to pass a single
> > face for SPECS.
> 
> You can't.  A face spec doesn't allow single faces.  See defface.

For defface we say one thing about what a face
spec is.  But here we say something different.
We say here:

 "SPECS, should form either a list of face names,
  or a property list of attribute/value pairs."

(And note that that doesn't allow for a mix of
the two: some face names and some attribute/value
pairs.)

Maybe there's some unspoken, special meaning of
"forms" intended here, other than meaning only
that SPECS, which is (by &rest) the list of
evaluated args after the first, should _be_ a
list of face names or a property list...

IOW, AFAICT we say that elements of list SPECS
can be face names.  And in particular SPECS can
be a list with just one element, which is a face
name.

Do you see the doc/behavior problem now?  If this
is a misunderstanding of what the doc _wants_ to
say, then please correct it to say what's needed.
If you need help with the wording, please speak up.

Further adding to the confusion is the fact that
when introducing "the following functions" (such
as `face-remap-set-base') we talk about
`face-remapping-alist' entries of the form:

(FACE RELATIVE-SPEC-1 RELATIVE-SPEC-2 ... BASE-SPEC)

and we say that each of those elements except FACE
can be "either a face name, or a property list of
attribute/value pairs".  (The comma is better
removed, BTW.)

A careful reading reminds us that this is about the
_alist_ entries, and not about the SPECS we see in
the descriptions of those "following functions".
Nevertheless, we've introduced things with "SPEC" in
their name (RELATIVE-SPEC-N) which can be face names.

So in some places (e.g. defface) we say that a face
SPEC can't be a face name.  In other places we say
it can.

And again, in this particular place we pretty clearly
suggest that SPECS elements can be face names.

Again, I don't claim to correctly understand what the
behavior is or should be.  I'm just reading the doc
and then trying to use the function as it seems to
say it can be used, and getting an error.

Please clarify.  If you do that here (bug thread) and
you would like help rewording, to make the doc more
clear, let me know.  One way or another, I think you
should be able to see what the misunderstanding is
and clarify things.  Thank you.

bug#54115: 27.2; `face-remap-set-base' bug with only two args

[Lars Ingebrigtsen]

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

> Evaluate (face-remap-set-base 'variable-pitch 'default).

Fixed in bug#46973.

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