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

Previous Next

Full log

View this message in rfc822 format

From: Martin Marshall <law <at> martinmarshall.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 68487 <at> debbugs.gnu.org
Subject: bug#68487: [PATCH] Make jump commands usable for all skeletons
Date: Mon, 05 Feb 2024 16:46:55 -0500

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>> 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

Sorry for my late response.

I had moved that code to the skeleton command definition because I
thought I'd have to account for adding skeleton positions to
`expand-pos' before expand.el had loaded.  But since it turns out that
expand.el loads skeleton.el anyway, it seems to makes more sense to
follow your suggestion and put this change back on `skeleton-end-hook'.

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.

-- 
Best regards,
Martin Marshall

Previous Next

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