GNU bug report logs - #19903
24.4; Emacs fails to save enriched buffer with error message `wrong-type-argument symbolp "bold"'

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 19903 in the body.
You can then email your comments to 19903 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: Jorge <jorge13515 <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.4; Emacs fails to save enriched buffer with error message
 `wrong-type-argument symbolp "bold"'
Date: Thu, 19 Feb 2015 15:15:40 -0200

[Message part 1 (text/plain, inline)]

Download the attached files to the same directory.  In GNU Bash, with that
directory current, run
emacs -Q --script bug_script.el &> bug_output.txt

I ran the above command for emacs versions 24.4.1 and 24.4.90.  The output
is in `bug_output_24.4.1.txt' and `bug_output_24.4.90.txt' respectively.  The
bug manifests in both tested Emacs versions.
To compose the initial draft of this bug report, I ran
emacs -Q -l bug_script.el
and then called `report-emacs-bug' with the enriched buffer that fails to be
saved as the current buffer.

So, the input file (`enriched_bug_orig.txt') is in enriched mode, and consists
of one `0' followed by newlines.  I expected Emacs to write the buffer to
`enriched_bug_edited.txt', make the `0' bold, and save the buffer.  In
practice, Emacs does make the `0' bold, but then fails to save the buffer.  The
second line of bug_script.el is `(toggle-debug-on-error)' so Emacs should have
provided useful information.

In GNU Emacs 24.4.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.12.2) of
 2015-01-15 on jorge-Notebook-HP-G42
Windowing system distributor `The X.Org Foundation', version 11.0.11600000
System Description: Ubuntu 14.10

Configured using:
 `configure --prefix=/usr/local/emacs'

Important settings:
  value of $LC_MONETARY: pt_BR.UTF-8
  value of $LC_NUMERIC: pt_BR.UTF-8
  value of $LC_TIME: pt_BR.UTF-8
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Text

Minor modes in effect:
  enriched-mode: t
  tooltip-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  use-hard-newlines: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
C-x o C-x o C-x C-f ~ / D e s k <tab> t r a b a l h
o <tab> <return> C-x o M-x r e p o r t - e m <tab>
<return> C-g C-x o C-x 3 C-x o C-x C-f C-g C-x b <return>
C-f C-SPC C-e C-b M-w C-x o M-x r e p o r t - e m <tab>
<return>

Recent messages:
Note: file is write protected
Saving file /home/jorge/tmp/emacs_enriched_bug/enriched_bug_edited.txt...
Enriched: encoding document...
Wrote /home/jorge/tmp/emacs_enriched_bug/enriched_bug_edited.txt
Saving file /home/jorge/tmp/emacs_enriched_bug/enriched_bug_edited.txt...
Enriched: encoding document...
Entering debugger...
Enriched: decoding document...
Indenting...
Quit [2 times]

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message format-spec rfc822 mml
mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util
help-fns mail-prsvr mail-utils help-mode easymenu debug disp-table
enriched cus-start cus-load time-date tooltip electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel x-win x-dnd tool-bar dnd
fontset image regexp-opt fringe tabulated-list newcomment lisp-mode
prog-mode register page menu-bar rfn-eshadow timer select scroll-bar
mouse jit-lock font-lock syntax facemenu font-core frame cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev
minibuffer nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote make-network-process
dbusbind gfilenotify dynamic-setting system-font-setting
font-render-setting move-toolbar gtk x-toolkit x multi-tty emacs)

Memory information:
((conses 16 83013 6903)
 (symbols 48 18640 0)
 (miscs 40 67 174)
 (strings 32 11108 4144)
 (string-bytes 1 292848)
 (vectors 16 9828)
 (vector-slots 8 391654 14920)
 (floats 8 72 382)
 (intervals 56 263 1)
 (buffers 960 16)
 (heap 1024 41562 939))

[enriched_bug_orig.txt (text/plain, attachment)]

[bug_script.el (text/x-emacs-lisp, attachment)]

[bug_output_24.4.1.txt (text/plain, attachment)]

[bug_output_24.4.90.txt (text/plain, attachment)]

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

From: Ivan Shmakov <ivan <at> siamics.net>
To: 19903 <at> debbugs.gnu.org, Jorge <jorge13515 <at> gmail.com>
Subject: Re: bug#19903: 24.4;
 wrong-type-argument symbolp "bold" during enriched-encode 
Date: Thu, 19 Feb 2015 18:48:01 +0000

>>>>> Jorge  <jorge13515 <at> gmail.com> writes:

[…]

 > (print (emacs-version))
 > (toggle-debug-on-error)
 > (find-file "enriched_bug_orig.txt")
 > (write-file "enriched_bug_edited.txt")
 > (facemenu-set-face "bold" 1 2)
 > (save-buffer 0)

	Could you please try and report if the error is still signalled
	if the code above is changed to use the 'bold symbol instead of
	a "bold" string?

[…]

 > Enriched: encoding document...
 > Debugger entered--Lisp error: (wrong-type-argument symbolp "bold")
 >   internal-get-lisp-face-attribute("bold" :foreground nil)
 >   face-attribute("bold" :foreground)
 >   enriched-face-ans("bold")
 >   enriched-encode-other-face(nil "bold")

	Unless I be mistaken, faces are ought to be symbols, not
	strings, so this error appears entirely reasonable.

[…]

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jorge <jorge13515 <at> gmail.com>
Cc: 19903-done <at> debbugs.gnu.org
Subject: Re: bug#19903: 24.4; Emacs fails to save enriched buffer with error
 message	`wrong-type-argument symbolp "bold"'
Date: Fri, 20 Feb 2015 10:27:13 +0200

> Date: Thu, 19 Feb 2015 15:15:40 -0200
> From: Jorge <jorge13515 <at> gmail.com>
> 
> Download the attached files to the same directory.  In GNU Bash, with that
> directory current, run
> emacs -Q --script bug_script.el &> bug_output.txt
> 
> I ran the above command for emacs versions 24.4.1 and 24.4.90.  The output
> is in `bug_output_24.4.1.txt' and `bug_output_24.4.90.txt' respectively.  The
> bug manifests in both tested Emacs versions.
> To compose the initial draft of this bug report, I ran
> emacs -Q -l bug_script.el
> and then called `report-emacs-bug' with the enriched buffer that fails to be
> saved as the current buffer.
> 
> So, the input file (`enriched_bug_orig.txt') is in enriched mode, and consists
> of one `0' followed by newlines.  I expected Emacs to write the buffer to
> `enriched_bug_edited.txt', make the `0' bold, and save the buffer.  In
> practice, Emacs does make the `0' bold, but then fails to save the buffer.  The
> second line of bug_script.el is `(toggle-debug-on-error)' so Emacs should have
> provided useful information.
> [...]
> (print (emacs-version))
> (toggle-debug-on-error)
> (find-file "enriched_bug_orig.txt")
> (write-file "enriched_bug_edited.txt")
> (facemenu-set-face "bold" 1 2)  <<<<<<<<<<<<<<<<<<<<<<<
> (save-buffer 0)

That's a cockpit error: the marked line should have said

  (facemenu-set-face 'bold 1 2)

instead.  Then this script will work and do what you expect.

A face is represented by its symbol, not by its string name.

I'm therefore closing this bug.

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

From: Jorge <jorge13515 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 19903-done <at> debbugs.gnu.org
Subject: Re: bug#19903: 24.4; Emacs fails to save enriched buffer with error
 message `wrong-type-argument symbolp "bold"'
Date: Fri, 20 Feb 2015 06:51:54 -0200

On Fri, Feb 20, 2015 at 6:27 AM, Eli Zaretskii <eliz <at> gnu.org> wrote:
> That's a cockpit error: the marked line should have said
>
>   (facemenu-set-face 'bold 1 2)
>
> instead.  Then this script will work and do what you expect.
Thank you very much for the clarification.  I just corrected my code.
However, it seems to me that the current behavior is not
user-friendly.  (facemenu-set-face "bold" 1 2) _does_ make the text
bold, without any warning - which makes it appear that the call was
successful - but then we get an error when we try to save the buffer.
Generally, it is best for errors to be detected as close to their
source as possible.  What do you think?

Regards

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

From: Ivan Shmakov <ivan <at> siamics.net>
To: 19903 <at> debbugs.gnu.org
Subject: Re: bug#19903: 24.4;
 wrong-type-argument symbolp "bold" during enriched-encode 
Date: Fri, 20 Feb 2015 09:12:08 +0000

>>>>> Jorge  <jorge13515 <at> gmail.com> writes:

[…]

 > However, it seems to me that the current behavior is not
 > user-friendly.  (facemenu-set-face "bold" 1 2) _does_ make the text
 > bold, without any warning – which makes it appear that the call was
 > successful – but then we get an error when we try to save the buffer.

	The facemenu-set-face command is essentially a call to
	facemenu-add-new-face, followed by a call to facemenu-add-face.
	Curiously enough, the former implements explicit support for
	string arguments:

$ git archive --format=tar  c4e2be4587ec -- lisp/facemenu.el \
      | tar -xO | nl -ba ;  # 2015-02-16 07:22:46 +0000 
…
   785	(defun facemenu-add-new-face (face)
   786	  "Add FACE (a face) to the Face menu if `facemenu-listed-faces' says so.
   787	This is called whenever you create a new face, and at other times."
   788	  (let* (name
   789		 symbol
…
   792		 function menu-val)
   793	    (if (symbolp face)
   794		(setq name (symbol-name face)
   795		      symbol face)
   796	      (setq name face
   797		    symbol (intern name)))

 > Generally, it is best for errors to be detected as close to their
 > source as possible.  What do you think?

	If string faces are rendered just the same as symbol ones,
	I’d rather wonder if either facemenu-add-face should silently
	‘intern’ all the strings it gets before use, /or/ face-attribute
	should do that.  (Or perhaps both.)

-- 
FSF associate member #7257  np. Transformation — David Modica  B6A0 230E 334A

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jorge <jorge13515 <at> gmail.com>
Cc: 19903 <at> debbugs.gnu.org
Subject: Re: bug#19903: 24.4;
 Emacs fails to save enriched buffer with error message
 `wrong-type-argument symbolp "bold"'
Date: Fri, 20 Feb 2015 12:03:15 +0200

> Date: Fri, 20 Feb 2015 06:51:54 -0200
> From: Jorge <jorge13515 <at> gmail.com>
> Cc: 19903-done <at> debbugs.gnu.org
> 
> On Fri, Feb 20, 2015 at 6:27 AM, Eli Zaretskii <eliz <at> gnu.org> wrote:
> > That's a cockpit error: the marked line should have said
> >
> >   (facemenu-set-face 'bold 1 2)
> >
> > instead.  Then this script will work and do what you expect.
> Thank you very much for the clarification.  I just corrected my code.
> However, it seems to me that the current behavior is not
> user-friendly.  (facemenu-set-face "bold" 1 2) _does_ make the text
> bold, without any warning - which makes it appear that the call was
> successful - but then we get an error when we try to save the buffer.
> Generally, it is best for errors to be detected as close to their
> source as possible.  What do you think?

facemenu-set-face calls several other functions, some of which accept
face names, some don't.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ivan Shmakov <ivan <at> siamics.net>
Cc: 19903 <at> debbugs.gnu.org
Subject: Re: bug#19903: 24.4;
 wrong-type-argument symbolp "bold" during enriched-encode
Date: Fri, 20 Feb 2015 12:07:42 +0200

> From: Ivan Shmakov <ivan <at> siamics.net>
> Date: Fri, 20 Feb 2015 09:12:08 +0000
> 
> 	If string faces are rendered just the same as symbol ones,
> 	I’d rather wonder if either facemenu-add-face should silently
> 	‘intern’ all the strings it gets before use, /or/ face-attribute
> 	should do that.  (Or perhaps both.)

Patches to do that are welcome.

Thanks.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 19903 <at> debbugs.gnu.org, Ivan Shmakov <ivan <at> siamics.net>
Subject: Re: bug#19903: 24.4;
 wrong-type-argument symbolp "bold" during enriched-encode
Date: Fri, 20 Feb 2015 12:18:38 -0500

>> If string faces are rendered just the same as symbol ones,

I think that's a misfeature.

>> I’d rather wonder if either facemenu-add-face should silently
>> ‘intern’ all the strings it gets before use, /or/ face-attribute
>> should do that.  (Or perhaps both.)

I think interning would be good, indeed, tho I would prefer if it
emitted some kind of warning instead of doing it silently, to try and
stop this bad habit.


        Stefan

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

From: Ivan Shmakov <ivan <at> siamics.net>
To: 19903 <at> debbugs.gnu.org
Subject: Re: bug#19903: 24.4;
 wrong-type-argument symbolp "bold" during enriched-encode 
Date: Fri, 20 Feb 2015 18:56:30 +0000

[Message part 1 (text/plain, inline)]

>>>>> Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

 >> If string faces are rendered just the same as symbol ones,

 > I think that's a misfeature.

	I’m not familiar with the Emacs display code, so I have no
	opinion on this.

 >> I’d rather wonder if either facemenu-add-face should silently
 >> ‘intern’ all the strings it gets before use, /or/ face-attribute
 >> should do that.  (Or perhaps both.)

 > I think interning would be good, indeed, tho I would prefer if it
 > emitted some kind of warning instead of doing it silently, to try and
 > stop this bad habit.

	Please consider the patch MIMEd.

-- 
FSF associate member #7257  np. Night Prowler — AC/DC   … 3013 B6A0 230E 334A

[Message part 2 (text/diff, inline)]

--- a/lisp/facemenu.el
+++ b/lisp/facemenu.el
@@ -702,6 +702,9 @@ defun facemenu-add-face (face &optional start end)
 text property.  Otherwise, selecting the default face would not have any
 effect.  See `facemenu-remove-face-function'."
   (interactive "*xFace: \nr")
+  (when (stringp face)
+    (warn "Face %S is a string; interning" face)
+    (setq face (intern face)))
   (cond
    ((and (eq face 'default)
          (not (eq facemenu-remove-face-function t)))
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -402,6 +402,9 @@ defun face-attribute (face attribute &optional frame inherit)
 value of `default' for INHERIT; this will resolve any unspecified or
 relative values by merging with the `default' face (which is always
 completely specified)."
+  (when (stringp face)
+    (message "Face %S is a string; interning" face)
+    (setq face (intern face)))
   (let ((value (internal-get-lisp-face-attribute face attribute frame)))
     (when (and inherit (face-attribute-relative-p attribute value))
       ;; VALUE is relative, so merge with inherited faces

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: 19903 <at> debbugs.gnu.org
Subject: Re: bug#19903: 24.4;
 wrong-type-argument symbolp "bold" during enriched-encode
Date: Fri, 20 Feb 2015 14:55:35 -0500

> +    (warn "Face %S is a string; interning" face)
[...]
> +    (message "Face %S is a string; interning" face)

Why `warn' in one and `message' in the other?

These could end up spewing a lot of warnings at times, so it's a bit
risky, but if such uses are sufficiently rare it might be OK.


        Stefan

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

From: Ivan Shmakov <ivan <at> siamics.net>
To: 19903 <at> debbugs.gnu.org
Subject: Re: bug#19903: 24.4;
 wrong-type-argument symbolp "bold" during enriched-encode 
Date: Fri, 20 Feb 2015 21:09:35 +0000

>>>>> Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

 >> + (warn "Face %S is a string; interning" face)

 >> + (message "Face %S is a string; interning" face)

 > Why `warn' in one and `message' in the other?

	By the time we hit this in ‘face-attribute’, the point at which
	the sub-setandard string-named face was introduced is presumably
	long gone, so there’s no good reason to use ‘warn’.

	On the contrary, ‘facemenu-add-face’ is likely to be invoked
	either directly by the user, or as part of some other
	interactive command.  Should the call result in a warning,
	the user would probably be able to gather enough context from
	his or her actions immediately preceding the warning.

 > These could end up spewing a lot of warnings at times, so it's a bit
 > risky, but if such uses are sufficiently rare it might be OK.

	At least so I hope.

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

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

From: Ivan Shmakov <ivan <at> siamics.net>
To: 19903 <at> debbugs.gnu.org
Subject: Re: bug#19903: 24.4;
 wrong-type-argument symbolp "bold" during enriched-encode 
Date: Sat, 21 Feb 2015 11:12:44 +0000

[Message part 1 (text/plain, inline)]

>>>>> Ivan Shmakov <ivan <at> siamics.net> writes:
>>>>> Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

 >>> + (warn "Face %S is a string; interning" face)

 >>> + (message "Face %S is a string; interning" face)

 >> Why `warn' in one and `message' in the other?

 > By the time we hit this in ‘face-attribute’, the point at which the
 > sub-setandard string-named face was introduced is presumably long
 > gone, so there’s no good reason to use ‘warn’.

	… Or, on a second thought, ‘message’, either.  Given that
	‘face-attribute’ has no idea of where the caller got this face
	from, there’s simply no way for it to provide any helpful
	message at this point.  (Say, “Face "bold", as found at position
	42 in #<buffer foo>, is a string; interning”.)

	Now, given that there’s a number of “internal” functions (such
	as ‘internal-lisp-face-p’, for instance) which accept string
	face names just fine, I wonder if it makes sense to just change
	‘internal-get-lisp-face-attribute’ accordingly?  An untested
	patch is hereby MIMEd.

[…]

-- 
FSF associate member #7257  np. Fear of the Dark — Iron Maiden B6A0 230E 334A

[Message part 2 (text/diff, inline)]

--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -3570,15 +3570,18 @@ the result will be absolute, otherwise it will be relative.  */)
 DEFUN ("internal-get-lisp-face-attribute", Finternal_get_lisp_face_attribute,
        Sinternal_get_lisp_face_attribute,
        2, 3, 0,
-       doc: /* Return face attribute KEYWORD of face SYMBOL.
-If SYMBOL does not name a valid Lisp face or KEYWORD isn't a valid
+       doc: /* Return face attribute KEYWORD of face FACE.
+FACE should be a symbol or string.
+If FACE does not name a valid Lisp face or KEYWORD isn't a valid
 face attribute name, signal an error.
-If the optional argument FRAME is given, report on face SYMBOL in that
-frame.  If FRAME is t, report on the defaults for face SYMBOL (for new
+If the optional argument FRAME is given, report on face FACE in that
+frame.  If FRAME is t, report on the defaults for face FACE (for new
 frames).  If FRAME is omitted or nil, use the selected frame.  */)
-  (Lisp_Object symbol, Lisp_Object keyword, Lisp_Object frame)
+  (Lisp_Object face, Lisp_Object keyword, Lisp_Object frame)
 {
   struct frame *f = EQ (frame, Qt) ? NULL : decode_live_frame (frame);
+  Lisp_Object symbol = (STRINGP (face) ? intern (SSDATA (face))
+			: face);
   Lisp_Object lface = lface_from_face_name (f, symbol, true), value = Qnil;
 
   CHECK_SYMBOL (symbol);

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ivan Shmakov <ivan <at> siamics.net>
Cc: 19903 <at> debbugs.gnu.org
Subject: Re: bug#19903: 24.4;
 wrong-type-argument symbolp "bold" during enriched-encode
Date: Sat, 21 Feb 2015 13:42:41 +0200

> From: Ivan Shmakov <ivan <at> siamics.net>
> Date: Sat, 21 Feb 2015 11:12:44 +0000
> 
> 	Now, given that there’s a number of “internal” functions (such
> 	as ‘internal-lisp-face-p’, for instance) which accept string
> 	face names just fine, I wonder if it makes sense to just change
> 	‘internal-get-lisp-face-attribute’ accordingly?  An untested
> 	patch is hereby MIMEd.

I don't think internal functions should cater to UI issues, unless
they are themselves interactive.

If we keep this confined to interactive functions, how many such
functions in facemenu.el will have to be changed?  If not too many,
I'm inclined to keep this there.

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

From: Ivan Shmakov <ivan <at> siamics.net>
To: 19903 <at> debbugs.gnu.org
Subject: Re: bug#19903: 24.4;
 wrong-type-argument symbolp "bold" during enriched-encode 
Date: Wed, 25 Feb 2015 06:20:36 +0000

>>>>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>>>> From: Ivan Shmakov  Date: Sat, 21 Feb 2015 11:12:44 +0000

 >> Now, given that there’s a number of “internal” functions (such as
 >> ‘internal-lisp-face-p’, for instance) which accept string face names
 >> just fine, I wonder if it makes sense to just change
 >> ‘internal-get-lisp-face-attribute’ accordingly?  An untested patch
 >> is hereby MIMEd.

 > I don't think internal functions should cater to UI issues, unless
 > they are themselves interactive.

	I’m unsure where you see an UI issue here?  The issue, as
	originally reported, is that face-attribute fails to handle
	string-named faces, which are considered perfectly valid by the
	rest of Emacs (including, say, facep and the display engine.)

	That is: the user accidentally sets 'face to a string, and is
	surprised to find that while the result is displayed just fine,
	some part of Emacs (namely, enriched-mode) breaks instantly.

	From there, we can go different ways, including:

	• bury our head in the sand and pretend there’s no issue;

	• patch one or two of the functions which can be used to add
	  such faces – to either silently (or not so) replace them with
	  the respective symbol-named ones, /or/ to signal an error;
	  this won’t prevent, however, the use of put-text-property and
	  the likes of it to achieve that same effect;

	• begin to slowly deprecate string-named faces; I guess we’d
	  rather have the display engine log the cases of their use,
	  at least once per buffer, to highlight the affected code and
	  thus ease the migration – from this undocumented feature to
	  the documented lack thereof;

	• accept string-named faces as valid and make sure that this
	  applies uniformly throughout the code; my latest patch changes
	  internal-get-lisp-face-attribute to achieve this, but I’m fine
	  with applying an earlier one instead, which similarly changes
	  face-attribute.

 > If we keep this confined to interactive functions, how many such
 > functions in facemenu.el will have to be changed?  If not too many,
 > I'm inclined to keep this there.

	I believe that facemenu-add-face is the only function which can
	be used to add a string-named face /interactively/, as it reads
	an arbitrary Lisp form for the face.  (See also #18369.)

	The original report, however, was concerned with the use of
	facemenu-add-face from Lisp, and there’re still a plenty of ways
	to hit this issue apart from using facemenu-add-face with a
	string argument.

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ivan Shmakov <ivan <at> siamics.net>
Cc: 19903 <at> debbugs.gnu.org
Subject: Re: bug#19903: 24.4;
 wrong-type-argument symbolp "bold" during enriched-encode
Date: Wed, 25 Feb 2015 18:06:50 +0200

> From: Ivan Shmakov <ivan <at> siamics.net>
> Date: Wed, 25 Feb 2015 06:20:36 +0000
> 
>  > I don't think internal functions should cater to UI issues, unless
>  > they are themselves interactive.
> 
> 	I’m unsure where you see an UI issue here?  The issue, as
> 	originally reported, is that face-attribute fails to handle
> 	string-named faces, which are considered perfectly valid by the
> 	rest of Emacs (including, say, facep and the display engine.)

Accepting strings instead of symbols is a convenience feature for
users, so it's a UI issue.

> 	From there, we can go different ways, including:
> 
> 	• bury our head in the sand and pretend there’s no issue;

I don't think anybody suggested that; I certainly didn't.  If you are
now suggesting it, then no, I don't think we should.

> 	• patch one or two of the functions which can be used to add
> 	  such faces – to either silently (or not so) replace them with
> 	  the respective symbol-named ones, /or/ to signal an error;
> 	  this won’t prevent, however, the use of put-text-property and
> 	  the likes of it to achieve that same effect;

I don't see why we need to spread this so wide.  Making facemenu.el
behave consistently sounds good enough, and will solve the original
report.

>  > If we keep this confined to interactive functions, how many such
>  > functions in facemenu.el will have to be changed?  If not too many,
>  > I'm inclined to keep this there.
> 
> 	I believe that facemenu-add-face is the only function which can
> 	be used to add a string-named face /interactively/, as it reads
> 	an arbitrary Lisp form for the face.  (See also #18369.)

Yes, but how many don't?

> 
> 	The original report, however, was concerned with the use of
> 	facemenu-add-face from Lisp, and there’re still a plenty of ways
> 	to hit this issue apart from using facemenu-add-face with a
> 	string argument.

I don't think we need to consider them.

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

From: Ivan Shmakov <ivan <at> siamics.net>
To: 19903 <at> debbugs.gnu.org
Subject: Re: bug#19903: 24.4;
 wrong-type-argument symbolp "bold" during enriched-encode 
Date: Wed, 25 Feb 2015 17:10:13 +0000

>>>>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>>>> From: Ivan Shmakov  Date: Wed, 25 Feb 2015 06:20:36 +0000

 >>> I don't think internal functions should cater to UI issues, unless
 >>> they are themselves interactive.

 >> I’m unsure where you see an UI issue here?  The issue, as originally
 >> reported, is that face-attribute fails to handle string-named faces,
 >> which are considered perfectly valid by the rest of Emacs
 >> (including, say, facep and the display engine.)

 > Accepting strings instead of symbols is a convenience feature
 > for users, so it's a UI issue.

	Could you please elaborate on this?  Specifically, does this
	apply to the interactive or non-interactive use (or both) of
	facemenu-add-face?

[…]

 >>> If we keep this confined to interactive functions, how many such
 >>> functions in facemenu.el will have to be changed?  If not too many,
 >>> I'm inclined to keep this there.

 >> I believe that facemenu-add-face is the only function which can be
 >> used to add a string-named face /interactively/, as it reads an
 >> arbitrary Lisp form for the face.  (See also #18369.)

 > Yes, but how many don't?

	One another (facemenu-set-face) uses read-face-name, which in
	turn explicitly passes user input through ‘intern’.

	Then, facemenu-set-face-from-menu uses last-command-event (when
	called interactively), assumes it’s a symbol, and uses it either
	as a face directly /or/ (should its name begin with fg: or bg:)
	as the cdr for a cons cell face.  (The facemenu-set-foreground
	and facemenu-set-background commands rely on this.)

	Per my reading of the code, no other command there accepts
	user-specified faces when used interactively.

[…]

-- 
FSF associate member #7257  np. Coming Home — Iron Maiden  B6A0 230E 334A

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ivan Shmakov <ivan <at> siamics.net>
Cc: 19903 <at> debbugs.gnu.org
Subject: Re: bug#19903: 24.4;
 wrong-type-argument symbolp "bold" during enriched-encode
Date: Wed, 25 Feb 2015 19:43:52 +0200

> From: Ivan Shmakov <ivan <at> siamics.net>
> Date: Wed, 25 Feb 2015 17:10:13 +0000
> 
> >>>>> Eli Zaretskii <eliz <at> gnu.org> writes:
> >>>>> From: Ivan Shmakov  Date: Wed, 25 Feb 2015 06:20:36 +0000
> 
>  >>> I don't think internal functions should cater to UI issues, unless
>  >>> they are themselves interactive.
> 
>  >> I’m unsure where you see an UI issue here?  The issue, as originally
>  >> reported, is that face-attribute fails to handle string-named faces,
>  >> which are considered perfectly valid by the rest of Emacs
>  >> (including, say, facep and the display engine.)
> 
>  > Accepting strings instead of symbols is a convenience feature
>  > for users, so it's a UI issue.
> 
> 	Could you please elaborate on this?  Specifically, does this
> 	apply to the interactive or non-interactive use (or both) of
> 	facemenu-add-face?

Both.

>  >>> If we keep this confined to interactive functions, how many such
>  >>> functions in facemenu.el will have to be changed?  If not too many,
>  >>> I'm inclined to keep this there.
> 
>  >> I believe that facemenu-add-face is the only function which can be
>  >> used to add a string-named face /interactively/, as it reads an
>  >> arbitrary Lisp form for the face.  (See also #18369.)
> 
>  > Yes, but how many don't?
> 
> 	One another (facemenu-set-face) uses read-face-name, which in
> 	turn explicitly passes user input through ‘intern’.
> 
> 	Then, facemenu-set-face-from-menu uses last-command-event (when
> 	called interactively), assumes it’s a symbol, and uses it either
> 	as a face directly /or/ (should its name begin with fg: or bg:)
> 	as the cdr for a cons cell face.  (The facemenu-set-foreground
> 	and facemenu-set-background commands rely on this.)
> 
> 	Per my reading of the code, no other command there accepts
> 	user-specified faces when used interactively.

So only one function needs a change?  If so, I think that's what we
should do.

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

From: Ivan Shmakov <ivan <at> siamics.net>
To: 19903 <at> debbugs.gnu.org
Subject: Re: bug#19903: 24.4;
 wrong-type-argument symbolp "bold" during enriched-encode 
Date: Wed, 25 Feb 2015 17:55:46 +0000

>>>>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>>>> From: Ivan Shmakov  Date: Wed, 25 Feb 2015 17:10:13 +0000
>>>>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>>>> From: Ivan Shmakov  Date: Wed, 25 Feb 2015 06:20:36 +0000

[…]

 >>>> I’m unsure where you see an UI issue here?  The issue, as
 >>>> originally reported, is that face-attribute fails to handle
 >>>> string-named faces, which are considered perfectly valid by the
 >>>> rest of Emacs (including, say, facep and the display engine.)

 >>> Accepting strings instead of symbols is a convenience feature
 >>> for users, so it's a UI issue.

 >> Could you please elaborate on this?  Specifically, does this apply
 >> to the interactive or non-interactive use (or both) of
 >> facemenu-add-face?

 > Both.

	Frankly, I fail to see how allowing the first of the following
	provides for any additional convenience over the second:

    M-x facemenu-add-face RET "bold" RET  (adds 'bold face);
    M-x facemenu-add-face RET bold RET    (adds "bold" one.)

	And, similarly:

   (facemenu-add-face "bold" here there)
   (facemenu-add-face 'bold  here there)

[…]

-- 
FSF associate member #7257  When the Wild Wind Blows — Iron Maiden  230E 334A

Previous Next

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