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

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

[Gemini Lasswell]

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"

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: |


In GNU Emacs 26.0.50.10 (x86_64-apple-darwin15.6.0, NS appkit-1404.47 Version 10.11.6 (Build 15G1004))
 of 2016-10-22 built on rainbow.local
Repository revision: f577b8efec324afc80aa4308efbfa40dcce30bbf
Windowing system distributor 'Apple', version 10.3.1404
Recent messages:
Result: (backward-delete-char-untabify (min ada-indent (current-column)))
Go...
Result: (setq str (skeleton-read (quote "[block name]: ") nil nil))
Go...
Result: (backward-delete-char-untabify (min ada-indent (current-column)))
Go...
Result: (backward-delete-char-untabify (min ada-indent (current-column)))
Go...
Result: "Foo"
Go...
Symbol's value as variable is void: |
Go...
Adjusting case ... Done
edebug-signal: Symbol’s value as variable is void: |
Configured using:
 'configure --with-ns --disable-ns-self-contained'

Configured features:
JPEG RSVG IMAGEMAGICK NOTIFY ACL GNUTLS LIBXML2 ZLIB TOOLKIT_SCROLL_BARS
NS

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

Major mode: Lisp Interaction

Minor modes in effect:
  diff-auto-refine-mode: t
  ivy-mode: t
  buffer-face-mode: t
  yas-global-mode: t
  yas-minor-mode: t
  pyvenv-mode: t
  shell-dirtrack-mode: t
  nameless-mode: t
  beacon-mode: t
  rainbow-mode: t
  column-enforce-mode: t
  volatile-highlights-mode: t
  region-state-mode: t
  ws-butler-global-mode: t
  ws-butler-mode: t
  show-smartparens-global-mode: t
  show-smartparens-mode: t
  smartparens-global-mode: t
  smartparens-global-strict-mode: t
  smartparens-strict-mode: t
  smartparens-mode: t
  which-key-mode: t
  modalka-mode: t
  recentf-mode: t
  global-auto-revert-mode: t
  winner-mode: t
  display-time-mode: t
  savehist-mode: t
  override-global-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  size-indication-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rfc822 mml mml-sec epa derived
epg gnus-util rmail rmail-loaddefs mailabbrev gmm-utils mailheader
skeleton edebug vc-git diff-mode colir flx dired dired-loaddefs counsel
esh-util swiper ivy delsel ffap face-remap guess-style smtpmail sendmail
mail-utils yasnippet highlight-indentation flymake company help-fns
radix-tree elpy pyvenv elpy-refactor smartparens-python python tramp-sh
tramp tramp-compat tramp-loaddefs trampver shell pcomplete format-spec
json map grep compile files-x cus-edit virtualenvwrapper gud comint
nameless lisp-mnt ace-window avy beacon smex ido deft debbugs
soap-client mm-decode mm-bodies mm-encode warnings rng-xsd rng-dt
rng-util xsd-regexp xml rainbow-mode ansi-color color s hydra lv
column-enforce-mode etags xref project volatile-highlights region-state
ws-butler smartparens-config smartparens thingatpt dash which-key
modalka quail smart-mode-line-dark-theme smart-mode-line advice
rich-minority whiteboard-theme classic-theme recentf tree-widget
wid-edit autorevert filenotify winner ring time cus-start cus-load
savehist cap-words superword subword use-package diminish bind-key
easy-mmode finder-inf edmacro kmacro info network-stream starttls
url-http tls gnutls mail-parse rfc2231 rfc2047 rfc2045 mm-util
ietf-drums mail-prsvr url-gw nsm subr-x puny url-cache url-auth url
url-proxy url-privacy url-expand url-methods url-history url-cookie
url-domsuf url-util mailcap cl package epg-config url-handlers url-parse
auth-source cl-seq eieio eieio-core cl-macs eieio-loaddefs
password-cache url-vars seq byte-opt gv bytecomp byte-compile cl-extra
help-mode easymenu cconv cl-loaddefs pcase cl-lib time-date mule-util
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel term/ns-win ns-win ucs-normalize term/common-win 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
term/tty-colors frame cl-generic cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932
hebrew greek romanian slovak czech european ethiopic indian cyrillic
chinese charscript case-table epa-hook jka-cmpr-hook help simple abbrev
obarray 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 kqueue
cocoa ns multi-tty make-network-process emacs)

Memory information:
((conses 16 527832 168555)
 (symbols 48 40887 0)
 (miscs 40 872 673)
 (strings 32 94990 175020)
 (string-bytes 1 2776740)
 (vectors 16 62134)
 (vector-slots 8 1093296 27939)
 (floats 8 924 1055)
 (intervals 56 11124 3330)
 (buffers 976 15))

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

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

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

>> 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.

I mean, not a symbol...

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

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

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> 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)

There weren't any comments, and this does seem to fix the test cases, so
I've applied this now.

But like I said, I don't really know this area, so if this is the wrong
thing to do, feel free to revert.

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