* bug#45264: 26.3; `face-remap-set-base' seems to be bugged @ 2020-12-16 0:31 Drew Adams 2020-12-19 10:20 ` Eli Zaretskii 0 siblings, 1 reply; 6+ messages in thread From: Drew Adams @ 2020-12-16 0:31 UTC (permalink / raw) To: 45264 See https://emacs.stackexchange.com/a/62301/105 (defface foo '((t (:background "red"))) "...") (face-remap-set-base 'font-lock-keyword-face 'foo) The &rest arg SPECS is `(foo)', which is, as required, a list of (one) face. But the code actually expects `foo' itself to be a list. It raises an error, because it sets SPECS to just `foo' and then tries to take the car of it. (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)))) Is there a doc bug (both manual and doc string)? Or is there a code bug? Or am I missing something? 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.18362 Configured using: `configure --without-dbus --host=x86_64-w64-mingw32 --without-compress-install 'CFLAGS=-O2 -static -g3'' ^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#45264: 26.3; `face-remap-set-base' seems to be bugged 2020-12-16 0:31 bug#45264: 26.3; `face-remap-set-base' seems to be bugged Drew Adams @ 2020-12-19 10:20 ` Eli Zaretskii 0 siblings, 0 replies; 6+ messages in thread From: Eli Zaretskii @ 2020-12-19 10:20 UTC (permalink / raw) To: Drew Adams; +Cc: 45264 > Date: Tue, 15 Dec 2020 16:31:31 -0800 (PST) > From: Drew Adams <drew.adams@oracle.com> > > (defface foo '((t (:background "red"))) "...") > > (face-remap-set-base 'font-lock-keyword-face 'foo) > > The &rest arg SPECS is `(foo)', which is, as required, a list of > (one) face. > > But the code actually expects `foo' itself to be a list. It raises > an error, because it sets SPECS to just `foo' and then tries to > take the car of it. > > (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)))) > > Is there a doc bug (both manual and doc string)? Or is there a code > bug? Or am I missing something? I don't see anything wrong with the documentation yet. One needs to know and understand what is a "face spec", and then everything falls into its place. The &rest part is also a big hint. Can I turn the table and ask you why you thought the argument could be a list of one or more faces? The doc string says "should FORM a list of faces", it doesn't say it should BE a list of faces. And since when does &rest specify a single argument that is a list? ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <<dbc9cd7f-c4da-4137-a972-aada49e0f1f1@default>]
[parent not found: <<833602umgb.fsf@gnu.org>]
* bug#45264: 26.3; `face-remap-set-base' seems to be bugged [not found] ` <<833602umgb.fsf@gnu.org> @ 2020-12-19 18:32 ` Drew Adams 2020-12-19 18:58 ` Eli Zaretskii 0 siblings, 1 reply; 6+ messages in thread From: Drew Adams @ 2020-12-19 18:32 UTC (permalink / raw) To: Eli Zaretskii, Drew Adams; +Cc: 45264 > > Is there a doc bug (both manual and doc string)? Or is there a code > > bug? Or am I missing something? > > I don't see anything wrong with the documentation yet. One needs to > know and understand what is a "face spec", and then everything falls > into its place. The &rest part is also a big hint. > > Can I turn the table and ask you why you thought the argument could be > a list of one or more faces? The doc string says "should FORM a list > of faces", it doesn't say it should BE a list of faces. And since > when does &rest specify a single argument that is a list? The confusion is not from not understanding what a face spec is. I think I know what a face spec is. The confusion is from the doc not saying explicitly that each element of SPECS is a face spec, and NOT a face. Please consider making that explicit (clear). 1. I didn't suggest (at all) that &rest specifies a single arg that is a list. Quite the contrary - the answer I gave to that SE question explicitly made that exact point. 2. "why you thought the argument could be a list of one or more faces?" The doc string explicitly says that elements of SPECS can be face names: Each list element should be either a face name or... 3. The predicate that tests for a face is `facep', and it doesn't return non-nil for a face spec. Its doc explicitly says that it tests whether its arg is a "face name", which can be a string or a symbol. Putting #2 and #3 together, the doc for SPECS does indeed say that elements of SPECS can be faces. 4. I strongly suggest that you change the language. "FORM" as a verb here is not clear, and this is apparently not about faces as arguments; it's about face specs. Face specs can be said to define (or "form") faces, but they are not faces - they don't satisfy `facep'. ___ Another possibility is perhaps to fix the behavior, so that it does what its doc says: allow elements of SPECS to "be either a face name or a property list of...". Allow faces, not just face specs. ^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#45264: 26.3; `face-remap-set-base' seems to be bugged 2020-12-19 18:32 ` Drew Adams @ 2020-12-19 18:58 ` Eli Zaretskii 0 siblings, 0 replies; 6+ messages in thread From: Eli Zaretskii @ 2020-12-19 18:58 UTC (permalink / raw) To: Drew Adams; +Cc: 45264-done > Date: Sat, 19 Dec 2020 10:32:16 -0800 (PST) > From: Drew Adams <drew.adams@oracle.com> > Cc: 45264@debbugs.gnu.org > > The confusion is from the doc not saying explicitly > that each element of SPECS is a face spec, and NOT a > face. SPECS has no "elements". SPECS stands for arguments to the function beyond the 1st arg FACE. Each such argument is either a face name or a list of attribute/value pairs. I changed the doc string to be more clear about that. > 2. "why you thought the argument could be a list of > one or more faces?" > > The doc string explicitly says that elements of > SPECS can be face names: > > Each list element should be either a face name or... That's after it says that you should consider SPECS as "forming" a list of elements. ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <<<dbc9cd7f-c4da-4137-a972-aada49e0f1f1@default>]
[parent not found: <<<833602umgb.fsf@gnu.org>]
[parent not found: <<4efb5c63-6636-449f-8e2b-4b8533a9cc8c@default>]
[parent not found: <<83a6u9tyh1.fsf@gnu.org>]
* bug#45264: 26.3; `face-remap-set-base' seems to be bugged [not found] ` <<83a6u9tyh1.fsf@gnu.org> @ 2020-12-19 19:28 ` Drew Adams 2020-12-19 19:40 ` Eli Zaretskii 0 siblings, 1 reply; 6+ messages in thread From: Drew Adams @ 2020-12-19 19:28 UTC (permalink / raw) To: Eli Zaretskii, Drew Adams; +Cc: 45264-done > > The confusion is from the doc not saying explicitly > > that each element of SPECS is a face spec, and NOT a > > face. > > SPECS has no "elements". SPECS stands for arguments to the function > beyond the 1st arg FACE. As with any &rest, you supply zero or more actual args that correspond to SPECS. The function itself receives a single list argument that corresponds to SPECS. In the function body, variable SPECS is a list. And as the doc says, "Each list element...". > Each such argument is either a face name or > a list of attribute/value pairs. AFAICT, the function doesn't work if such an arg is a face name. See what I said about that previously, please. > I changed the doc string to be more clear about that. Great. Thanks for taking a look. Please check for the behavior bug I pointed to: If the doc is correct then the behavior seems bugged. It's not true that you can pass face names, AFAICT. > > 2. "why you thought the argument could be a list of > > one or more faces?" > > > > The doc string explicitly says that elements of > > SPECS can be face names: > > > > Each list element should be either a face name or... > > That's after it says that you should consider SPECS as "forming" a > list of elements. "Forming" a list of elements is unclear, as I said. And SPECS is, itself, from the point of view of the function, a list of elements. See above. There is _nothing_ special about this. Every &rest parameter behaves the same in this regard. BTW, the exact same misleading and inexact text is used for function `face-map-add-relative'. For functions `buffer-face-(set|toggle)', however, we instead say, as we usually do, "Each argument in SPECS should be a face, i.e., either a face name or a property list...". And we explicitly speak of SPECS as a list (singular): "if SPECS is omitted or nil..." ^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#45264: 26.3; `face-remap-set-base' seems to be bugged 2020-12-19 19:28 ` Drew Adams @ 2020-12-19 19:40 ` Eli Zaretskii 0 siblings, 0 replies; 6+ messages in thread From: Eli Zaretskii @ 2020-12-19 19:40 UTC (permalink / raw) To: Drew Adams; +Cc: 45264 > Date: Sat, 19 Dec 2020 11:28:31 -0800 (PST) > From: Drew Adams <drew.adams@oracle.com> > Cc: 45264-done@debbugs.gnu.org > > > > The confusion is from the doc not saying explicitly > > > that each element of SPECS is a face spec, and NOT a > > > face. > > > > SPECS has no "elements". SPECS stands for arguments to the function > > beyond the 1st arg FACE. > > As with any &rest, you supply zero or more actual > args that correspond to SPECS. The function itself > receives a single list argument that corresponds to > SPECS. > > In the function body, variable SPECS is a list. > And as the doc says, "Each list element...". The doc string shouldn't explain how the function's body sees SPECS, it should explain how to provide those args. > Please check for the behavior bug I pointed to: If it doesn't work according to the doc string, then yes, it's a bug. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-12-19 19:40 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-12-16 0:31 bug#45264: 26.3; `face-remap-set-base' seems to be bugged Drew Adams 2020-12-19 10:20 ` Eli Zaretskii [not found] <<dbc9cd7f-c4da-4137-a972-aada49e0f1f1@default> [not found] ` <<833602umgb.fsf@gnu.org> 2020-12-19 18:32 ` Drew Adams 2020-12-19 18:58 ` Eli Zaretskii [not found] <<<dbc9cd7f-c4da-4137-a972-aada49e0f1f1@default> [not found] ` <<<833602umgb.fsf@gnu.org> [not found] ` <<4efb5c63-6636-449f-8e2b-4b8533a9cc8c@default> [not found] ` <<83a6u9tyh1.fsf@gnu.org> 2020-12-19 19:28 ` Drew Adams 2020-12-19 19:40 ` Eli Zaretskii
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.