GNU bug report logs - #68487
[PATCH] Make jump commands usable for all skeletons

Previous Next

Full log

Message #43 received at 68487 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Martin Marshall <law <at> martinmarshall.com>
Cc: 68487 <at> debbugs.gnu.org
Subject: Re: bug#68487: [PATCH] Make jump commands usable for all skeletons
Date: Wed, 07 Feb 2024 12:13:11 -0500

>>> There's one problem though.  The autoloaded keybindings for
>>> `expand-jump-to-next-slot' and `expand-jump-to-previous-slot' won't work
>>> the first time they're called on an expanded skeleton, unless the user
>>> has previously loaded expand.el.
>>
>> Hmm... this suggests we should try and "merge" `expand-list/pos` and
>> `skeleton-positions`?
>
> My thinking was just to initialize `expand-list/pos/index` in skeleton.el, so
> that a skeleton-command could populate `expand-pos` with locations from
> `skeleton-positions` even before expand.el has loaded.

I'm having trouble understanding the design behind `expand.el`, but IIUC
`expand-list` is basically the variable through which interaction with
other things is expected to take place, so I think it's fair to make
`skeleton.el` set `expand-list` whereas `expand-pos/index` seem like
internal vars and `skeleton.el` shouldn't touch them.

But the docstring of `expand-list` needs to be (re)written for that, first.

Ideally this should go along with the removal of the use of a vector in
`expand-list`, which not only is odd given its name but is odd because
it seems completely useless.  IOW my reading of the code suggests the
code would work just as well with the patch below.


        Stefan


diff --git a/lisp/expand.el b/lisp/expand.el
index f32ab101224..714cc5fc11a 100644
--- a/lisp/expand.el
+++ b/lisp/expand.el
@@ -337,17 +337,12 @@ expand-abbrev-hook
 	    (progn
 	      ;; expand-point tells us if we have inserted the text
 	      ;; ourself or if it is the hook which has done the job.
+	      (if (listp expand-list)
+		  (setq expand-index 0
+			expand-pos (expand-list-to-markers expand-list)
+			expand-list nil))
 	      (if expand-point
-		  (progn
-		    (if (vectorp expand-list)
-			(expand-build-marks expand-point))
-		    (indent-region p expand-point nil))
-		;; an outside function can set expand-list to a list of
-		;; markers in reverse order.
-		(if (listp expand-list)
-		    (setq expand-index 0
-			  expand-pos (expand-list-to-markers expand-list)
-			  expand-list nil)))
+		  (indent-region p expand-point nil))
 	      (run-hooks 'expand-expand-hook)
 	      t)
 	  nil))
@@ -359,12 +354,16 @@ expand-do-expansion
 	 (text (aref vect 0))
 	 (position (aref vect 1))
 	 (jump-args (aref vect 2))
-	 (hook (aref vect 3)))
+	 (hook (aref vect 3))
+         (startpos (point)))
     (cond (text
 	   (insert text)
 	   (setq expand-point (point))))
     (if jump-args
-        (funcall #'expand-build-list (car jump-args) (cdr jump-args)))
+        (setq expand-list (nreverse
+                           (mapcar (lambda (offset)
+                                     (+ startpos -1 offset))
+                                   (cdr jump-args)))))
     (if position
 	(backward-char position))
     (if hook
@@ -415,28 +414,6 @@ expand-jump-to-next-slot
 ;;;###autoload (define-key abbrev-map "p" 'expand-jump-to-previous-slot)
 ;;;###autoload (define-key abbrev-map "n" 'expand-jump-to-next-slot)
 
-(defun expand-build-list (len l)
-  "Build a vector of offset positions from the list of positions."
-  (expand-clear-markers)
-  (setq expand-list (vconcat l))
-  (let ((i 0)
-	(lenlist (length expand-list)))
-    (while (< i lenlist)
-      (aset expand-list i (- len (1- (aref expand-list i))))
-      (setq i (1+ i)))))
-
-(defun expand-build-marks (p)
-  "Transform the offsets vector into a marker vector."
-  (if expand-list
-      (progn
-	(setq expand-index 0)
-	(setq expand-pos (make-vector (length expand-list) nil))
-	(let ((i (1- (length expand-list))))
-	  (while (>= i 0)
-	    (aset expand-pos i (copy-marker (- p (aref expand-list i))))
-	    (setq i (1- i))))
-	(setq expand-list nil))))
-
 (defun expand-clear-markers ()
   "Make the markers point nowhere."
   (if expand-pos

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.