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

[Eli Zaretskii <eliz@gnu.org>]

Any progress there?

> From: martin <law@martinmarshall.com>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,  68487@debbugs.gnu.org
> Date: Thu, 21 Mar 2024 20:05:04 -0400
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Ping!  Martin, can you please respond to Stefan's comments, so we
> > could move forward with this issue?
> 
> Sorry for the long delay in responding.  I'll try to get to this by the
> end of the weekend.
> 
> 
> >> Cc: 68487@debbugs.gnu.org
> >> Date: Sat, 02 Mar 2024 23:07:18 -0500
> >> From:  Stefan Monnier via "Bug reports for GNU Emacs,
> >>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> >> 
> >> >> 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.
> >> >
> >> > Nothing (at least nothing in Emacs core) stores a vector to
> >> > `expand-list`.  So I'm curious why `expand-abbrev-hook` was written to
> >> > account for that possibility.
> >> 
> >> It's because it internally did that, tho I don't know why it did that
> >> internally since my patch seems to show that it's simpler not to.
> >> 
> >> > Changing `expand-abbrev-hook` to expect `expand-list` to actually be a
> >> > list (as you did with your patch) makes sense to me.
> >> 
> >> Should I install it, so it's kept separate from the changes you add
> >> on top (mostly for readability of the patches)?
> >> 
> >> > What do you think?
> >> 
> >> I find the patch a bit hard to read, maybe for lack of a separate
> >> description of the intended changes, or maybe because it does too much
> >> in a single step.
> >> 
> >> I have one question, tho:
> >> 
> >> >  (defun expand-do-expansion ()
> >> > -  (delete-char (- (length last-abbrev-text)))
> >> > -  (let* ((vect (symbol-value last-abbrev))
> >> > -	 (text (aref vect 0))
> >> > -	 (position (aref vect 1))
> >> > -	 (jump-args (aref vect 2))
> >> > -	 (hook (aref vect 3)))
> >> > -    (cond (text
> >> > -	   (insert text)
> >> > -	   (setq expand-point (point))))
> >> > -    (if jump-args
> >> > -        (funcall #'expand-build-list (car jump-args) (cdr jump-args)))
> >> > -    (if position
> >> > -	(backward-char position))
> >> > -    (if hook
> >> > -	(funcall hook))
> >> > -    t))
> >> > -
> >> > -(defun expand-abbrev-from-expand (word)
> >> > -  "Test if an abbrev has a hook."
> >> > -  (or
> >> > -   (and (intern-soft word local-abbrev-table)
> >> > -	(symbol-function (intern-soft word local-abbrev-table)))
> >> > -   (and (intern-soft word global-abbrev-table)
> >> > -	(symbol-function (intern-soft word global-abbrev-table)))))
> >> > -
> >> > -(defun expand-previous-word ()
> >> > -  "Return the previous word."
> >> > -  (save-excursion
> >> > -    (let ((p (point)))
> >> > -      (backward-word 1)
> >> > -      (buffer-substring p (point)))))
> >> > +  ;; 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))
> >> > +  (run-hooks 'expand-expand-hook))
> >> 
> >> Hmm... but this `expand-do-expansion` doesn't actually "do" any
> >> expansion any more, right?
> >> 
> >> >  (defun expand-skeleton-end-hook ()
> >> > -  (if skeleton-positions
> >> > -      (setq expand-list skeleton-positions)))
> >> > +  (when skeleton-positions
> >> > +    (setq expand-list skeleton-positions)
> >> > +    (expand-do-expansion)))
> >> 
> >> Here if you read the code out loud it doesn't make sense to call 
> >> `expand-do-expansion` since skeleton has already "done the expansion".
> >> 
> >> 
> >>         Stefan
> >> 
> >> 
> >> 
> >> 
> >> 
> 
> -- 
> Best regards,
> Martin Marshall
>