From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Martin Marshall Newsgroups: gmane.emacs.bugs Subject: bug#68487: [PATCH] Make jump commands usable for all skeletons Date: Sun, 25 Feb 2024 20:26:22 -0500 Organization: The Marshall Firm, LLC Message-ID: <875xycyq0x.fsf@martinmarshall.com> References: <877ckawckc.fsf@martinmarshall.com> <875xz2y46o.fsf@martinmarshall.com> <87il316y4w.fsf@martinmarshall.com> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="13835"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 68487@debbugs.gnu.org To: Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Mon Feb 26 02:28:07 2024 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1rePmp-0003Jb-0w for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 26 Feb 2024 02:28:07 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rePmU-0003E7-Gf; Sun, 25 Feb 2024 20:27:47 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rePmM-0003DY-2L for bug-gnu-emacs@gnu.org; Sun, 25 Feb 2024 20:27:39 -0500 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1rePmL-0004eC-Mb for bug-gnu-emacs@gnu.org; Sun, 25 Feb 2024 20:27:37 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1rePmk-0007pl-Bv for bug-gnu-emacs@gnu.org; Sun, 25 Feb 2024 20:28:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Martin Marshall Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 26 Feb 2024 01:28:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 68487 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 68487-submit@debbugs.gnu.org id=B68487.170891087830096 (code B ref 68487); Mon, 26 Feb 2024 01:28:02 +0000 Original-Received: (at 68487) by debbugs.gnu.org; 26 Feb 2024 01:27:58 +0000 Original-Received: from localhost ([127.0.0.1]:41346 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rePmf-0007pJ-8L for submit@debbugs.gnu.org; Sun, 25 Feb 2024 20:27:58 -0500 Original-Received: from mail-yw1-f169.google.com ([209.85.128.169]:61743) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rePmc-0007os-Ol for 68487@debbugs.gnu.org; Sun, 25 Feb 2024 20:27:56 -0500 Original-Received: by mail-yw1-f169.google.com with SMTP id 00721157ae682-608959cfcbfso24930877b3.3 for <68487@debbugs.gnu.org>; Sun, 25 Feb 2024 17:27:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=martinmarshall-com.20230601.gappssmtp.com; s=20230601; t=1708910784; x=1709515584; darn=debbugs.gnu.org; h=mime-version:message-id:date:references:organization:in-reply-to :subject:cc:to:from:from:to:cc:subject:date:message-id:reply-to; bh=6CwdxYwX+BMKHxtnYeQ5iPf88ydSq4uUw/23c1saNpI=; b=mLH4kga1lMJNq8yUK6TnMuIcy+KVLPu6CcM9X00N3Ldq+CTY3AR5WkqlUF0RQilVNc 5jh5n742AcXfEiR91PhT7t8B8RLXLa/t5N0cixAl3F6yn2T6ZgQNGbFcY+N+GVqsP5l5 VZvojPhjMI8pednl7wl/Ahe/Cbka75op+DaOKgAgd2WF2kaNYBzs/SEC68gAJdx8XnqK 78KIbK2Zo6mD/edhI9q5SYPf9jsQ8yU7KyeGi57w4/3Fr8vW5R4h3+Y+3dcB7nuaZ0Au qjvKFFjwdHCwv+M7K2O9mbTY54Ba/N9amFdJnZy+RuJ0MZsd9eJ+QN2jPXv3qRLd5I8j qU9A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708910784; x=1709515584; h=mime-version:message-id:date:references:organization:in-reply-to :subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=6CwdxYwX+BMKHxtnYeQ5iPf88ydSq4uUw/23c1saNpI=; b=CnqZvYS6sEJkTjcIFQy4I+Au14vQC77d1UcR5j6m14JqF76h0zStFL98GxjhNdycc0 XFWKFg9AYm0GBFlPAi2ZbPyBsebr2V726or4C3HLAP8GzW/+7OS/WZU62rDWMmoipUZK eYJqFVxLZPZXd+6+iECnZvAQdlImtHOpQwrK9f8ycMOoiSqtjx29Vof8OLdsbOjHRRKu Kkrxw89DH4qfNrOZWjjXej2OeEOystwWwk15UE5fuuFUGW/FI6pbGNCoL+UZlNg0GCsQ v5pZRou25fqZK+bfY4ZlQxAMCdM9y2O//o+vh5Ky/PLHURDoTWnxe/bRTzyBEeVqF8HP 453Q== X-Gm-Message-State: AOJu0Yz7q404g55d9KYfTbXlvbQ09outiD+HwwDX5jMCYJwEECKtvaIz /i5fxRUW3pRa3QJxfxKNVEI88TMyDvbOmnqvr1EAwbvysyb95oT7xAh52yIyuRz1eSwS1HLQ/VI = X-Google-Smtp-Source: AGHT+IE7vqugEsyp99qrnubpfykf0uF7ZzeP430yFVab+6bhaPXxTaXksB3hifgzsgymi8Uvb2Gguw== X-Received: by 2002:a81:b64f:0:b0:608:8a6b:b213 with SMTP id h15-20020a81b64f000000b006088a6bb213mr5874911ywk.33.1708910783695; Sun, 25 Feb 2024 17:26:23 -0800 (PST) Original-Received: from vader (68-252-220-225.lightspeed.tukrga.sbcglobal.net. [68.252.220.225]) by smtp.gmail.com with ESMTPSA id i184-20020a0dc6c1000000b00607e72b478csm960386ywd.133.2024.02.25.17.26.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 25 Feb 2024 17:26:23 -0800 (PST) In-Reply-To: (Stefan Monnier's message of "Wed, 07 Feb 2024 12:13:11 -0500") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:280650 Archived-At: Finally got around to looking at this again. Stefan Monnier writes: > 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. That sounds right. But outside code also needs a way to trigger population of `expand-pos` from `expand-list`. I tried to do this with some of the new changes copied below. > 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. Changing `expand-abbrev-hook` to expect `expand-list` to actually be a list (as you did with your patch) makes sense to me. > IOW my reading of the code suggests the code would work just as well > with the patch below. Yes, I applied your patch and added more changes to separate functionality between (a) expansion and (b) populating `expand-pos` with the marks that the "expand-jump" commands use. I also removed some more functions that either became obsolete because of the changes from your patch or were already not being used anywhere. These changes make expand.el much more compact and easier to understand, not to mention the improved functionality. Still a work in progress though. What do you think? -- Best regards, Martin Marshall diff --git a/lisp/expand.el b/lisp/expand.el index f32ab101224..56329dd9805 100644 --- a/lisp/expand.el +++ b/lisp/expand.el @@ -331,60 +331,43 @@ expand-abbrev-hook (let ((p (point))) (setq expand-point nil) ;; don't expand if the preceding char isn't a word constituent - (if (and (eq (char-syntax (preceding-char)) - ?w) - (expand-do-expansion)) - (progn - ;; expand-point tells us if we have inserted the text - ;; ourself or if it is the hook which has done the job. - (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))) - (run-hooks 'expand-expand-hook) + (if (eq (char-syntax (preceding-char)) ?w) + (progn + (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)) + (startpos (point))) + (cond (text + (insert text) + (setq expand-point (point)))) + (if jump-args + (setq expand-list (nreverse + (mapcar (lambda (offset) + (+ startpos -1 offset)) + (cdr jump-args))))) + (if position + (backward-char position)) + (if hook + (funcall hook)) + (if expand-point + (indent-region p expand-point nil)) + (unless hook + (expand-do-expansion))) t) nil)) nil)) (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)) ;;;###autoload (defun expand-jump-to-previous-slot () @@ -415,38 +398,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 - (progn - (let ((i (1- (length expand-pos)))) - (while (>= i 0) - (set-marker (aref expand-pos i) nil) - (setq i (1- i)))) - (setq expand-pos nil)))) - (defun expand-in-literal () "Test if we are in a comment or in a string." (save-excursion @@ -477,8 +428,9 @@ expand-list-to-markers ;; 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))) + (when skeleton-positions + (setq expand-list skeleton-positions) + (expand-do-expansion))) (add-hook 'skeleton-end-hook (function expand-skeleton-end-hook))