Subject: Re: bug#68487: [PATCH] Make jump commands usable for all skeletons

From: Stefan Monnier <[email-masked]>
Date: Sun, 28 Jan 2024 14:45:37 -0500

> diff --git a/lisp/skeleton.el b/lisp/skeleton.el
> index 89cb11b0fe2..24d6ef15e74 100644
> --- a/lisp/skeleton.el
> +++ b/lisp/skeleton.el
> @@ -31,6 +31,8 @@
>  
>  ;;; Code:
>  
> +(require 'expand)
> +
>  (eval-when-compile (require 'cl-lib))
>  
>  ;; page 1:	statement skeleton language definition & interpreter
> @@ -139,7 +141,14 @@ define-skeleton
>  This is a way of overriding the use of a highlighted region.")
>         (interactive "*P\nP")
>         (atomic-change-group
> -         (skeleton-proxy-new ',skeleton str arg)))))
> +         (skeleton-proxy-new ',skeleton str arg))
> +       (if expand-in-progress-p
> +           ;; `expand-abbrev-hook' will set the markers in this case.
> +           (setq expand-list skeleton-positions)
> +         (setq expand-index 0
> +	       expand-pos (expand-list-to-markers skeleton-positions)
> +               expand-list nil))
> +       t)))
>  
>  ;;;###autoload
>  (defun skeleton-proxy-new (skeleton &optional str arg)

I don't think we want such a tight dependency between `skeleton.el` and
`expand.el` [ Partly to avoid the kind of circular dependencies you
just found yourself in, but also more generally.  ]

My suggestion would be to move that code to the `skeleton-end-hook`, but
I see that's where the code started, so I'm obviously missing something:
what make you decide to move the code out of the `skeleton-end-hook` and
into `define-skeleton`?

        Stefan