bug#20386: 25.0.50; error when calling sgml-attributes and sgml-tag

bug#20386: 25.0.50; error when calling sgml-attributes and sgml-tag

[Matt Lundin]

Within html-mode, calling sgml-tag (C-c C-o) and entering a tag that
prompts for attributes (e.g., "h3") results in the error/backtrace
included below. A similar error occurs when calling sgml-attributes
within an existing tag (e.g., "<h3></h3>").

As far as I can tell, this error began with commit
84e0b7dad6f1a8e53261f9b96f5a9080fea681a4.

Debugger entered--Lisp error: (void-variable alist)
  (completing-read "Attribute: " alist)
  eval((completing-read "Attribute: " alist))
  skeleton-read((completing-read "Attribute: " alist))
  sgml-attributes("h3" t)
  (setq v2 (sgml-attributes "h3" t))
  eval((setq v2 (sgml-attributes "h3" t)))
  skeleton-internal-1((quote (setq v2 (sgml-attributes "h3" t))) nil nil)
  skeleton-internal-list((("") (quote (setq v2 (sgml-attributes "h3" t))) 62 (cond ((string= "![" "h3") (backward-char) (quote (("") " [ " _ " ]]"))) ((and (eq v2 t) sgml-xml-mode (member "h3" sgml-empty-tags)) (quote (("") -1 " />"))) ((or (and (eq v2 t) (not sgml-xml-mode)) (string-match "^[/!?]" "h3")) nil) ((symbolp v2) (if (eq v2 t) (setq v2 nil)) (quote (("") v2 _ v2 "</" (identity (quote "h3")) 62 >))) ((eq (car v2) t) (cons (quote ("")) (cdr v2))) (t (append (quote (("") (car v2))) (cdr v2) (quote (resume: (car v2) _ "</" (identity (quote "h3")) 62 >)))))) "")
  skeleton-internal-1((("") (quote (setq v2 (sgml-attributes "h3" t))) 62 (cond ((string= "![" "h3") (backward-char) (quote (("") " [ " _ " ]]"))) ((and (eq v2 t) sgml-xml-mode (member "h3" sgml-empty-tags)) (quote (("") -1 " />"))) ((or (and (eq v2 t) (not sgml-xml-mode)) (string-match "^[/!?]" "h3")) nil) ((symbolp v2) (if (eq v2 t) (setq v2 nil)) (quote (("") v2 _ v2 "</" (identity (quote "h3")) 62 >))) ((eq (car v2) t) (cons (quote ("")) (cdr v2))) (t (append (quote (("") (car v2))) (cdr v2) (quote (resume: (car v2) _ "</" (identity (quote "h3")) 62 >)))))) t nil)
  skeleton-internal-1((\` (("") (quote (setq v2 (sgml-attributes (\, str) t))) 62 (cond ((string= "![" (\, str)) (backward-char) (quote (("") " [ " _ " ]]"))) ((and (eq v2 t) sgml-xml-mode (member (\, str) sgml-empty-tags)) (quote (("") -1 " />"))) ((or (and (eq v2 t) (not sgml-xml-mode)) (string-match "^[/!?]" (\, str))) nil) ((symbolp v2) (if (eq v2 t) (setq v2 nil)) (quote (("") v2 _ v2 "</" (identity (quote ...)) 62 >))) ((eq (car v2) t) (cons (quote ("")) (cdr v2))) (t (append (quote (("") (car v2))) (cdr v2) (quote (resume: (car v2) _ "</" (identity ...) 62 >))))))) nil nil)
  skeleton-internal-list(((funcall (or skeleton-transformation-function (quote identity)) (setq sgml-tag-last (completing-read (if (> (length sgml-tag-last) 0) (format "Tag (default %s): " sgml-tag-last) "Tag: ") sgml-tag-alist nil nil nil (quote sgml-tag-history) sgml-tag-last))) 60 str | (("") -1 (quote (undo-boundary)) (identity "&lt;")) | (\` (("") (quote (setq v2 (sgml-attributes (\, str) t))) 62 (cond ((string= "![" (\, str)) (backward-char) (quote (("") " [ " _ " ]]"))) ((and (eq v2 t) sgml-xml-mode (member (\, str) sgml-empty-tags)) (quote (("") -1 " />"))) ((or (and (eq v2 t) (not sgml-xml-mode)) (string-match "^[/!?]" (\, str))) nil) ((symbolp v2) (if (eq v2 t) (setq v2 nil)) (quote (("") v2 _ v2 "</" (identity ...) 62 >))) ((eq (car v2) t) (cons (quote ("")) (cdr v2))) (t (appen
 d (quote (... ...)) (cdr v2) (quote (resume: ... _ "</" ... 62 >)))))))) nil)
  #[nil "\302\b	\"\207" [skeleton str skeleton-internal-list] 3]()
  funcall(#[nil "\302\b	\"\207" [skeleton str skeleton-internal-list] 3])
  (let ((completion-ignore-case (quote t))) (funcall (quote #[nil "\302\b	\"\207" [skeleton str skeleton-internal-list] 3])))
  eval((let ((completion-ignore-case (quote t))) (funcall (quote #[nil "\302\b	\"\207" [skeleton str skeleton-internal-list] 3]))))
  skeleton-insert(((funcall (or skeleton-transformation-function (quote identity)) (setq sgml-tag-last (completing-read (if (> (length sgml-tag-last) 0) (format "Tag (default %s): " sgml-tag-last) "Tag: ") sgml-tag-alist nil nil nil (quote sgml-tag-history) sgml-tag-last))) 60 str | (("") -1 (quote (undo-boundary)) (identity "&lt;")) | (\` (("") (quote (setq v2 (sgml-attributes (\, str) t))) 62 (cond ((string= "![" (\, str)) (backward-char) (quote (("") " [ " _ " ]]"))) ((and (eq v2 t) sgml-xml-mode (member (\, str) sgml-empty-tags)) (quote (("") -1 " />"))) ((or (and (eq v2 t) (not sgml-xml-mode)) (string-match "^[/!?]" (\, str))) nil) ((symbolp v2) (if (eq v2 t) (setq v2 nil)) (quote (("") v2 _ v2 "</" (identity ...) 62 >))) ((eq (car v2) t) (cons (quote ("")) (cdr v2))) (t (append (quot
 e (... ...)) (cdr v2) (quote (resume: ... _ "</" ... 62 >)))))))) nil nil)
  skeleton-proxy-new(((funcall (or skeleton-transformation-function (quote identity)) (setq sgml-tag-last (completing-read (if (> (length sgml-tag-last) 0) (format "Tag (default %s): " sgml-tag-last) "Tag: ") sgml-tag-alist nil nil nil (quote sgml-tag-history) sgml-tag-last))) 60 str | (("") -1 (quote (undo-boundary)) (identity "&lt;")) | (\` (("") (quote (setq v2 (sgml-attributes (\, str) t))) 62 (cond ((string= "![" (\, str)) (backward-char) (quote (("") " [ " _ " ]]"))) ((and (eq v2 t) sgml-xml-mode (member (\, str) sgml-empty-tags)) (quote (("") -1 " />"))) ((or (and (eq v2 t) (not sgml-xml-mode)) (string-match "^[/!?]" (\, str))) nil) ((symbolp v2) (if (eq v2 t) (setq v2 nil)) (quote (("") v2 _ v2 "</" (identity ...) 62 >))) ((eq (car v2) t) (cons (quote ("")) (cdr v2))) (t (append (q
 uote (... ...)) (cdr v2) (quote (resume: ... _ "</" ... 62 >)))))))) nil nil)
  sgml-tag(nil nil)
  funcall-interactively(sgml-tag nil nil)
  call-interactively(sgml-tag nil nil)
  command-execute(sgml-tag)




In GNU Emacs 25.0.50.2 (i686-pc-linux-gnu, GTK+ Version 3.16.2)
 of 2015-04-19 on archeee
Repository revision: 62e170072e6000b30c37792227dc34e71a31d797
Windowing system distributor `The X.Org Foundation', version 11.0.11701000
Configured using:
 `configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
 --localstatedir=/var --mandir=/usr/share/man
 --pdfdir=/usr/share/doc/emacs/pdf --with-sound=alsa --without-gconf
 --with-x-toolkit=gtk3 --with-xft 'CFLAGS=-march=i686 -mtune=generic -O2
 -pipe -fstack-protector-strong --param=ssp-buffer-size=4'
 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro
 CPPFLAGS=-D_FORTIFY_SOURCE=2'

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS NOTIFY
ACL GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB

Important settings:
  value of $LC_CTYPE: en_US.UTF-8
  value of $LC_MESSAGES: C
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Messages

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: 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
  buffer-read-only: t
  line-number-mode: t

Recent messages:
/home/matt/test.html locked by matt@archeee (pid 3893): (s, q, p, ?)? 
Please type q, s, or p; or ? for help
/home/matt/test.html locked by matt@archeee (pid 3893): (s, q, p, ?)? 
Please type q, s, or p; or ? for help
Quit
next-line: End of buffer
Entering debugger...
Making completion list...
Quit
Type "q" in help window to restore its previous buffer, C-M-v to scroll help.

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message dired 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 skeleton
cus-start cus-load sgml-mode mule-util time-date tooltip eldoc 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 elisp-mode
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 cl-preloaded 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 8 90059 9879)
 (symbols 24 19527 0)
 (miscs 20 73 257)
 (strings 16 14655 4308)
 (string-bytes 1 364073)
 (vectors 8 10754)
 (vector-slots 4 408319 9084)
 (floats 8 96 170)
 (intervals 28 428 20)
 (buffers 520 16)
 (heap 1024 24434 747))

bug#20386: 25.0.50; error when calling sgml-attributes and sgml-tag

[Dmitry Gutov]

On 04/20/2015 10:15 PM, Matt Lundin wrote:

> As far as I can tell, this error began with commit
> 84e0b7dad6f1a8e53261f9b96f5a9080fea681a4.

Yep, it's a fallout from making the file use lexical binding.

There are a lot forms like

   (skeleton-read '(completing-read
                    "Attribute: "
                    alist))

I wonder if using a backtick and a comma in each:

   (skeleton-read `(completing-read
                    "Attribute: "
                    ',alist))

is the right thing to do.

bug#20386: 25.0.50; error when calling sgml-attributes and sgml-tag

[Stefan Monnier]

> There are a lot forms like
>
>   (skeleton-read '(completing-read
>                    "Attribute: "
>                    alist))
>
> I wonder if using a backtick and a comma in each:
>
>   (skeleton-read `(completing-read
>                    "Attribute: "
>                    ',alist))
>
> is the right thing to do.

Yes, that looks right, thank you.  Better would be to let skeleton-read
take a function as argument and use `funcall' instead of `eval'.


        Stefan

bug#20386: 25.0.50; error when calling sgml-attributes and sgml-tag

[Dmitry Gutov]

On 04/27/2015 08:00 AM, Stefan Monnier wrote:

> Yes, that looks right, thank you.  Better would be to let skeleton-read
> take a function as argument and use `funcall' instead of `eval'.

Do you mean it would be able to accept both forms and functions? 
Otherwise, it's a breaking API change.

bug#20386: 25.0.50; error when calling sgml-attributes and sgml-tag

[Stefan Monnier]

>> Yes, that looks right, thank you.  Better would be to let skeleton-read
>> take a function as argument and use `funcall' instead of `eval'.
> Do you mean it would be able to accept both forms and functions?

Right, with the "forms" alternative deprecated.

> Otherwise, it's a breaking API change.

Indeed,


        Stefan

bug#20386: 25.0.50; error when calling sgml-attributes and sgml-tag

[Harald Hanche-Olsen]

Since I get bitten by this bug regularly, and little has happened to it, 
I took it upon myself to suggest a patch. I include it below.

I believe the patch is sufficiently small and trivial to not be “legally 
significant” (I don't have any signed papers on file).

One caveat:
This will break skeleton-read for code that calls it with a symbol, 
expecting the symbol to be evaluated, if the symbol also has a function 
binding.

If this is undesirable, replace (functionp prompt) in the patched 
skeleton-read with (and (functionp prompt) (not (symbolp prompt))).

– Harald

 From 1d8cc12f73b6b7fe7907073e6c34b558cc4f726b Mon Sep 17 00:00:00 2001
From: Harald Hanche-Olsen <hanche@math.ntnu.no>
Date: Thu, 23 Jul 2015 18:09:44 +0200
Subject: [PATCH] Fix bug #20386

---
  lisp/skeleton.el            | 17 ++++++++++-------
  lisp/textmodes/sgml-mode.el | 14 +++++++++-----
  2 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/lisp/skeleton.el b/lisp/skeleton.el
index d23488b..2c017dd 100644
--- a/lisp/skeleton.el
+++ b/lisp/skeleton.el
@@ -277,7 +277,8 @@ When done with skeleton, but before going back to 
`_'-point call
  (defun skeleton-read (prompt &optional initial-input recursive)
    "Function for reading a string from the minibuffer within skeletons.

-PROMPT must be a string or a form that evaluates to a string.
+PROMPT must be a string or a function that evaluates to a string.
+It may also be a form that evaluates to a string (deprecated).
  It may contain a `%s' which will be replaced by `skeleton-subprompt'.
  If non-nil second arg INITIAL-INPUT or variable `input' is a string or
  cons with index to insert before reading.  If third arg RECURSIVE is 
non-nil
@@ -306,12 +307,14 @@ automatically, and you are prompted to fill in the 
variable parts.")))
  	;; before point.
          (save-excursion (insert "\n")))
      (unwind-protect
-	(setq prompt (if (stringp prompt)
-			 (read-string (format prompt skeleton-subprompt)
-				      (setq initial-input
-					    (or initial-input
-						(symbol-value 'input))))
-		       (eval prompt)))
+	(setq prompt (cond ((stringp prompt)
+                            (read-string (format prompt skeleton-subprompt)
+                                         (setq initial-input
+                                               (or initial-input
+                                                   (symbol-value 
'input)))))
+                           ((functionp prompt)
+                            (funcall prompt))
+                           (t (eval prompt))))
        (or eolp
  	  (delete-char 1))))
    (if (and recursive
diff --git a/lisp/textmodes/sgml-mode.el b/lisp/textmodes/sgml-mode.el
index 2ffe9c1..6a14b52 100644
--- a/lisp/textmodes/sgml-mode.el
+++ b/lisp/textmodes/sgml-mode.el
@@ -759,9 +759,10 @@ If QUIET, do not print a message when there are no 
attributes for TAG."
  	    (insert ?\s)
  	    (insert (funcall skeleton-transformation-function
  			     (setq attribute
-				   (skeleton-read '(completing-read
-						    "Attribute: "
-						    alist)))))
+				   (skeleton-read (lambda ()
+                                                    (completing-read
+                                                     "Attribute: "
+                                                     alist))))))
  	    (if (string= "" attribute)
  		(setq i 0)
  	      (sgml-value (assoc (downcase attribute) alist))
@@ -1177,13 +1178,16 @@ See `sgml-tag-alist' for info about attribute 
rules."
      (if (and (eq (car alist) t) (not sgml-xml-mode))
  	(when (cdr alist)
  	  (insert "=\"")
-	  (setq alist (skeleton-read '(completing-read "Value: " (cdr alist))))
+	  (setq alist (skeleton-read (lambda ()
+                                       (completing-read
+                                        "Value: " (cdr alist)))))
  	  (if (string< "" alist)
  	      (insert alist ?\")
  	    (delete-char -2)))
        (insert "=\"")
        (if (cdr alist)
-          (insert (skeleton-read '(completing-read "Value: " alist)))
+          (insert (skeleton-read (lambda ()
+                                   (completing-read "Value: " alist))))
          (when (null alist)
            (insert (skeleton-read '(read-string "Value: ")))))
        (insert ?\"))))
-- 
2.4.5

bug#20386: 25.0.50; error when calling sgml-attributes and sgml-tag

[Dmitry Gutov]

On 07/23/2015 07:28 PM, Harald Hanche-Olsen wrote:

> I believe the patch is sufficiently small and trivial to not be “legally
> significant” (I don't have any signed papers on file).

The changes look okay to me, but please resend them as an attachment, 
and make sure the patch is against the current master. I couldn't easily 
apply this one.

> One caveat:
> This will break skeleton-read for code that calls it with a symbol,
> expecting the symbol to be evaluated, if the symbol also has a function
> binding.

I think that's fine. It should be a rare case, and it can be easily 
worked around on the caller's side.

bug#20386: 25.0.50; error when calling sgml-attributes and sgml-tag

[Harald Hanche-Olsen]

[-- Attachment #1: Type: text/plain, Size: 541 bytes --]

Dmitry Gutov wrote:
>
> The changes look okay to me, but please resend them as an attachment,
> and make sure the patch is against the current master. I couldn't easily
> apply this one.

Okay. I did it that way because I wasn't sure if the bug tracker 
accepted attachments.

The attached patch is against the current master at this commit:

commit de576a55d8c47ffd2e24fca2d48ed99f298d4334
Author: Eli Zaretskii <eliz@gnu.org>
Date:   Fri Jul 24 10:47:05 2015 +0300

(Created with git format-patch.)
I hope it will work better.

– Harald

[-- Attachment #2: 0001-Fix-bug-20386.patch --]
[-- Type: text/plain, Size: 3658 bytes --]

From 196a5a1f1e1c48482b5a173add374e4c1a5471c8 Mon Sep 17 00:00:00 2001
From: Harald Hanche-Olsen <hanche@math.ntnu.no>
Date: Thu, 23 Jul 2015 18:09:44 +0200
Subject: [PATCH] Fix bug #20386

---
 lisp/skeleton.el            | 17 ++++++++++-------
 lisp/textmodes/sgml-mode.el | 14 +++++++++-----
 2 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/lisp/skeleton.el b/lisp/skeleton.el
index d23488b..2c017dd 100644
--- a/lisp/skeleton.el
+++ b/lisp/skeleton.el
@@ -277,7 +277,8 @@ When done with skeleton, but before going back to `_'-point call
 (defun skeleton-read (prompt &optional initial-input recursive)
   "Function for reading a string from the minibuffer within skeletons.
 
-PROMPT must be a string or a form that evaluates to a string.
+PROMPT must be a string or a function that evaluates to a string.
+It may also be a form that evaluates to a string (deprecated).
 It may contain a `%s' which will be replaced by `skeleton-subprompt'.
 If non-nil second arg INITIAL-INPUT or variable `input' is a string or
 cons with index to insert before reading.  If third arg RECURSIVE is non-nil
@@ -306,12 +307,14 @@ automatically, and you are prompted to fill in the variable parts.")))
 	;; before point.
         (save-excursion (insert "\n")))
     (unwind-protect
-	(setq prompt (if (stringp prompt)
-			 (read-string (format prompt skeleton-subprompt)
-				      (setq initial-input
-					    (or initial-input
-						(symbol-value 'input))))
-		       (eval prompt)))
+	(setq prompt (cond ((stringp prompt)
+                            (read-string (format prompt skeleton-subprompt)
+                                         (setq initial-input
+                                               (or initial-input
+                                                   (symbol-value 'input)))))
+                           ((functionp prompt)
+                            (funcall prompt))
+                           (t (eval prompt))))
       (or eolp
 	  (delete-char 1))))
   (if (and recursive
diff --git a/lisp/textmodes/sgml-mode.el b/lisp/textmodes/sgml-mode.el
index 2ffe9c1..6a14b52 100644
--- a/lisp/textmodes/sgml-mode.el
+++ b/lisp/textmodes/sgml-mode.el
@@ -759,9 +759,10 @@ If QUIET, do not print a message when there are no attributes for TAG."
 	    (insert ?\s)
 	    (insert (funcall skeleton-transformation-function
 			     (setq attribute
-				   (skeleton-read '(completing-read
-						    "Attribute: "
-						    alist)))))
+				   (skeleton-read (lambda ()
+                                                    (completing-read
+                                                     "Attribute: "
+                                                     alist))))))
 	    (if (string= "" attribute)
 		(setq i 0)
 	      (sgml-value (assoc (downcase attribute) alist))
@@ -1177,13 +1178,16 @@ See `sgml-tag-alist' for info about attribute rules."
     (if (and (eq (car alist) t) (not sgml-xml-mode))
 	(when (cdr alist)
 	  (insert "=\"")
-	  (setq alist (skeleton-read '(completing-read "Value: " (cdr alist))))
+	  (setq alist (skeleton-read (lambda ()
+                                       (completing-read
+                                        "Value: " (cdr alist)))))
 	  (if (string< "" alist)
 	      (insert alist ?\")
 	    (delete-char -2)))
       (insert "=\"")
       (if (cdr alist)
-          (insert (skeleton-read '(completing-read "Value: " alist)))
+          (insert (skeleton-read (lambda ()
+                                   (completing-read "Value: " alist))))
         (when (null alist)
           (insert (skeleton-read '(read-string "Value: ")))))
       (insert ?\"))))
-- 
2.4.5

bug#20386: 25.0.50; error when calling sgml-attributes and sgml-tag

[Dmitry Gutov]

On 07/24/2015 11:41 AM, Harald Hanche-Olsen wrote:

> (Created with git format-patch.)
> I hope it will work better.

Thank you, I've changed the message (please take a look, by the way, for 
future reference), and pushed it.

Someone please double-check that it fixes the problem, and if so send an 
email to 20386-done, to close this bug.

bug#20386: 25.0.50; error when calling sgml-attributes and sgml-tag

[Harald Hanche-Olsen]

Dmitry Gutov wrote:
> Thank you, I've changed the message (please take a look, by the way, for
> future reference), and pushed it.

Great. The message looks fine to me.

– Harald

bug#20386: 25.0.50; error when calling sgml-attributes and sgml-tag

[Dmitry Gutov]

On 07/24/2015 07:55 PM, Harald Hanche-Olsen wrote:

> Great. The message looks fine to me.

Please try to follow its format in future patches (see CONTRIBUTE for 
details).