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

[Eli Zaretskii <eliz@gnu.org>]

Ping! Ping!  Martin, could you please respond?

> Cc: 68487@debbugs.gnu.org, monnier@iro.umontreal.ca
> Date: Thu, 18 Apr 2024 11:58:40 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> Ping!  Martin, did you have time to make any progress in this matter?
> 
> > Cc: 68487@debbugs.gnu.org, monnier@iro.umontreal.ca
> > Date: Sat, 06 Apr 2024 11:56:53 +0300
> > From: 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
> > > 
> > 
> > 
> > 
> > 
> 
> 
> 
>