bug#24779: 26.0.50; Incorrect Edebug spec for define-skeleton

[Lars Ingebrigtsen]

Gemini Lasswell <gazally@runbox.com> writes:

> Edebug fails to instrument a large number of the skeletons included in
> Emacs, and some of those which it succeeds in instrumenting do not work
> under the debugger. I have not tried every skeleton in Emacs, but among
> those I tried which had problems are sgml-tag, rul-if, rul-function,
> m2-begin, sh-case, and texinfo-insert-quotation.
>
> To reproduce this behavior using the skeletons in ada-stmt.el, start
> Emacs, with -Q or otherwise, and open lisp/progmodes/ada-stmt.el from
> the Emacs source tree. Then:
>
> C-s ada-array RET
> C-u C-M-x 
>
> Result: Invalid read syntax: "Head of list form must be a symbol or
> lambda expression"

(define-skeleton ada-array
  "Insert array type definition.
Prompt for component type and index subtypes."
  ()
  "array (" ("index definition: " str ", " ) -2 ") of " _ ?\;)

It's complaining about "index definition", which is indeed not a
string.  The following patch fixes this, but I have no idea what's
intended here:

diff --git a/lisp/skeleton.el b/lisp/skeleton.el
index bce73d6bfe..6a8ec7993a 100644
--- a/lisp/skeleton.el
+++ b/lisp/skeleton.el
@@ -105,7 +105,7 @@ skeleton-point
 (defvar skeleton-regions)
 
 (def-edebug-spec skeleton-edebug-spec
-  ([&or null stringp (stringp &rest stringp) [[&not atom] def-form]]
+  ([&or null stringp (stringp &rest stringp) [[&not atom] sexp]]
    &rest &or "n" "_" "-" ">" "@" "&" "!" "resume:"
    ("quote" def-form) skeleton-edebug-spec def-form))
 ;;;###autoload

> To reproduce an example of a skeleton that can be instrumented but then
> produces an error while debugging, continue with:
>
> C-s ada-declare-block RET
> C-u C-M-x
> C-x b ada RET
> M-x ada-mode RET
> M-x ada-declare-block RET
> g g Foo RET g g g
>
> Result: Symbol's value as variable is void: |

I'm able to reproduce that, too, and adding "|" to the list of strings
in the spec seems to fix this.  I know nothing about any of this,
though, so...  Is this the right thing to do?

diff --git a/lisp/skeleton.el b/lisp/skeleton.el
index bce73d6bfe..67fc4aae15 100644
--- a/lisp/skeleton.el
+++ b/lisp/skeleton.el
@@ -105,8 +105,8 @@ skeleton-point
 (defvar skeleton-regions)
 
 (def-edebug-spec skeleton-edebug-spec
-  ([&or null stringp (stringp &rest stringp) [[&not atom] def-form]]
-   &rest &or "n" "_" "-" ">" "@" "&" "!" "resume:"
+  ([&or null stringp (stringp &rest stringp) [[&not atom] sexp]]
+   &rest &or "n" "_" "-" ">" "@" "&" "!" "|" "resume:"
    ("quote" def-form) skeleton-edebug-spec def-form))
 ;;;###autoload
 (defmacro define-skeleton (command documentation &rest skeleton)

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