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

From: Martin Marshall <[email-masked]>
Date: Mon, 15 Jan 2024 15:45:23 -0500

Dear Emacs Maintainers,

I noticed the following item in the Emacs TODO file:

> ** Improve the "code snippets" support
> Consolidate skeleton.el, tempo.el, and expand.el (any other?) and then
> advertise/use/improve it.

To that end, here's a patch which allows using expand.el's
`expand-jump-to-next-slot' ("C-x a n") and
`expand-jump-to-previous-slot' ("C-x a p") commands with all
skeletons.

In the current Emacs release, an expanded skeleton adds the locations
of `@' symbols to `skeleton-positions' list.  One could theoretically
convert these positions to markers and write commands for navigating
to the locations.  Fortunately, expand.el already implements this
behavior.  The only problem is that it's limited to skeletons being
expanded as abbrevs.  Skeletons invoked by a keybinding, menu entry,
or "M-x" can't use expand.el's jumping commands.

This patch changes that by updating `define-skeleton', so that
skeleton commands will update the list of markers in `expand-pos'
whenever called outside of `expand-abbrev'.

What do you think?

-- 
Best regards,
Martin Marshall
>From f5dbc75b7d8c06ebed6ad0dae6c06b6957d7852b Mon Sep 17 00:00:00 2001
From: Martin Marshall <[email-masked]>
Date: Mon, 15 Jan 2024 15:09:01 -0500
Subject: [PATCH] Make jump commands usable for all skeletons

* etc/NEWS: Announce availability of jump commands for all skeletons
* lisp/expand.el (expand-in-progress-p, expand-abbrev-hook): Add global
variable to indicate when the 'expand-abbrev-hook' function is running.
(expand-skeleton-end-hook, skeleton-end-hook): Remove the
'expand-skeleton-end-hook' function and move its code to 'define-skeleton'.
* lisp/skeleton.el (expand): Require the expand.el library.
(define-skeleton): Populate 'expand-pos' for use by expand.el's jump commands,
or if 'expand-abbrev-hook' is runnning, copy the positions for handling by
that function.
---
 etc/NEWS         | 13 +++++++++++++
 lisp/expand.el   | 14 +++++---------
 lisp/skeleton.el | 11 ++++++++++-
 3 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index bce33f96aee..ef30d4e9219 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -147,6 +147,19 @@ compositing manager, Emacs will now redisplay such a frame even though
 'frame-visible-p' returns nil or 'icon' for it.  This can happen, for
 example, as part of preview for iconified frames.
 
+** Consolidation of tempo.el, skeleton.el, and expand.el
+
+---
+*** All skeleton templates can now use expand.el "jump" commands.
+Previously, skeleton templates invoked by "M-x", a keybinding, or a
+menu entry, could not use expand.el's jump commands
+('expand-jump-to-next-slot' and 'expand-jump-to-previous-slot').  A
+skeleton could only use expand.el navigation if added as an abbrev
+using 'expand-add-abbrev' and only when invoked using such an abbrev.
+
+The expand.el jump commands now work on all skeletons, regardless of
+invocation method.
+
 ---
 ** New user option 'menu-bar-close-window'.
 When non-nil, selecting "Close" from the "File" menu or clicking
diff --git a/lisp/expand.el b/lisp/expand.el
index f32ab101224..ff2ae226d5f 100644
--- a/lisp/expand.el
+++ b/lisp/expand.el
@@ -286,6 +286,9 @@ expand-add-abbrevs
 
 (defvar expand-list nil "Temporary variable used by the Expand package.")
 
+(defvar expand-in-progress-p nil
+  "If non-nil, `expand-abbrev-hook' is expanding an abbrev.")
+
 (defvar-local expand-pos nil
   "If non-nil, store a vector with position markers defined by the last expansion.")
 
@@ -326,6 +329,7 @@ expand-abbrev-hook
 See `expand-add-abbrevs'.  Value is non-nil if expansion was done."
   ;; Expand only at the end of a line if we are near a word that has
   ;; an abbrev built from expand-add-abbrev.
+  (setq expand-in-progress-p t)
   (if (and (eolp)
 	   (not (expand-in-literal)))
       (let ((p (point)))
@@ -348,6 +352,7 @@ expand-abbrev-hook
 		    (setq expand-index 0
 			  expand-pos (expand-list-to-markers expand-list)
 			  expand-list nil)))
+              (setq expand-in-progress-p nil)
 	      (run-hooks 'expand-expand-hook)
 	      t)
 	  nil))
@@ -473,15 +478,6 @@ expand-list-to-markers
 	    loop (1- loop)))
     v))
 
-;; integration with skeleton.el
-;; Used in `skeleton-end-hook' to fetch the positions for  @ skeleton tags.
-;; See `skeleton-insert'.
-(defun expand-skeleton-end-hook ()
-  (if skeleton-positions
-      (setq expand-list skeleton-positions)))
-
-(add-hook 'skeleton-end-hook (function expand-skeleton-end-hook))
-
 (provide 'expand)
 
 (run-hooks 'expand-load-hook)
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)
-- 
2.39.2