unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#59960: 30.0.50; Eshell's $* variable (used to define aliases) doesn't work as expected
@ 2022-12-11  6:13 Jim Porter
  2022-12-11  8:04 ` Eli Zaretskii
  0 siblings, 1 reply; 7+ messages in thread
From: Jim Porter @ 2022-12-11  6:13 UTC (permalink / raw)
  To: 59960

[-- Attachment #1: Type: text/plain, Size: 1525 bytes --]

This bug report originally comes from a Reddit post[1] where a user 
found that aliasing the Eshell built-in "mv" and "cp" commands didn't 
work properly. I'll show a slightly simpler example though. Starting 
from "emacs -Q -f eshell":

   ~ $ alias my-echo "echo A $* Z"
   ~ $ my-echo b c d
   ("A"
    ("b" "c" "d")
    "Z")

   ~ $ echo A b c d Z
   ("A" "b" "c" "d" "Z")

As you can see above, the "$*" special variable inserts the list of 
arguments passed to the alias *as a sublist*. That might be what you 
want sometimes, but more likely, you'd want to insert the arguments 
in-place, like with ",@" in Emacs Lisp backquote forms. (Note: for 
external processes, this isn't an issue because Eshell flattens all the 
arguments to external commands.)

Attached is a draft patch to fix this. It adds a new "$@expr" variable 
expansion to Eshell which lets you splice a list in-place:

   ~ $ alias my-echo 'echo A $@* Z'
   ~ $ my-echo b c d
   ("A" "b" "c" "d" "Z")

Currently, this only works in simple situations like the above. It 
should work for any variable, but likely won't work with Eshell for 
loops, inside double quotes, etc. It would be nice to get this working 
in a wider variety of places in Eshell command forms before merging. 
Still, even what's here is already pretty useful, I think. I'm posting 
this now mainly to get feedback on whether this is the right direction 
to go in the first place.

[1] 
https://old.reddit.com/r/emacs/comments/xs2ofo/eshell_aliases_for_mv_and_cp_using_their_elisp/

[-- Attachment #2: 0001-Support-completion-of-variables-with-the-length-oper.patch --]
[-- Type: text/plain, Size: 1354 bytes --]

From 326f0d11a48daef8f9d935de9da22e1b165bfddf Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sat, 10 Dec 2022 20:52:28 -0800
Subject: [PATCH 1/2] Support completion of variables with the length operator
 in Eshell

These are forms like '$#VARIABLE'.

* lisp/eshell/esh-var.el (eshell-complete-variable-reference): Support
the length operator.
---
 lisp/eshell/esh-var.el | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 57ea42f4933..5824da6dc0e 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -751,12 +751,13 @@ eshell-index-value
 
 (defun eshell-complete-variable-reference ()
   "If there is a variable reference, complete it."
-  (let ((arg (pcomplete-actual-arg)) index)
-    (when (setq index
-		(string-match
-		 (concat "\\$\\(" eshell-variable-name-regexp
-			 "\\)?\\'") arg))
-      (setq pcomplete-stub (substring arg (1+ index)))
+  (let ((arg (pcomplete-actual-arg)))
+    (when (string-match
+           (rx "$" (? "#")
+               (? (group (regexp eshell-variable-name-regexp)))
+               string-end)
+           arg)
+      (setq pcomplete-stub (substring arg (match-beginning 1)))
       (throw 'pcomplete-completions (eshell-variables-list)))))
 
 (defun eshell-variables-list ()
-- 
2.25.1


[-- Attachment #3: 0002-Add-support-for-the-splice-operator-in-Eshell.patch --]
[-- Type: text/plain, Size: 15238 bytes --]

From 597b41576907d5b6233e02cff166a83644b9f9a5 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Tue, 8 Nov 2022 22:49:23 -0800
Subject: [PATCH 2/2] Add support for the "splice operator" in Eshell

This allows splicing lists in-place in argument lists, which is
particularly important when defining aliases using the '$*' special
variable.

* lisp/eshell/esh-var.el (eshell-parse-variable): Add support for the
splice operator.
(eshell-interpolate-variable): Let 'eshell-parse-variable' handle
adding 'eshell-escape-arg'.
(eshell-complete-variable-reference): Handle the splice operator.

* lisp/eshell/esh-cmd.el (eshell-rewrite-named-command): Handle
'eshell-splice-args'.

* lisp/eshell/esh-arg.el (eshell-splice-args): New function.

* lisp/eshell/em-cmpl.el (eshell-complete-parse-arguments): Remove
'eshell-splice-args' sigils in Eshell command forms so that we can
perform completion on splice-expansions.

* lisp/eshell/em-unix.el (eshell-complete-host-reference): Don't try
to complete arguments containing "$@".

* test/lisp/eshell/esh-var-tets.el (esh-var-test/interp-var-splice)
(esh-var-test/interp-var-splice-concat): New tests.

* doc/misc/eshell.texi (Aliases): Expand documentation and use '$@*'.
(Dollars Expansion): Explain the splice operator.

* etc/NEWS: Announce this change.
---
 doc/misc/eshell.texi              | 62 ++++++++++++++++++++++---------
 etc/NEWS                          | 10 +++++
 lisp/eshell/em-cmpl.el            | 26 ++++++++-----
 lisp/eshell/em-unix.el            | 12 ++++--
 lisp/eshell/esh-arg.el            |  5 +++
 lisp/eshell/esh-cmd.el            | 31 ++++++++++++----
 lisp/eshell/esh-var.el            | 12 ++++--
 test/lisp/eshell/esh-var-tests.el | 30 +++++++++++++++
 8 files changed, 144 insertions(+), 44 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 1383b412ce7..fa1d16f7913 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1028,23 +1028,25 @@ Aliases
 @section Aliases
 
 @vindex $*
-@findex eshell-expand-history-references
+@findex eshell-read-aliases-list
 Aliases are commands that expand to a longer input line.  For example,
-@command{ll} is a common alias for @code{ls -l}, and would be defined
-with the command invocation @kbd{alias ll 'ls -l $*'}; with this defined,
-running @samp{ll foo} in Eshell will actually run @samp{ls -l foo}.
-Aliases defined (or deleted) by the @command{alias} command are
-automatically written to the file named by @code{eshell-aliases-file},
-which you can also edit directly.  After doing so, use @w{@kbd{M-x
-eshell-read-aliases-list}} to load the edited aliases.
+@command{ll} is a common alias for @code{ls -l}.  To define this alias
+in Eshell, you can use the command invocation @kbd{alias ll 'ls -l
+$@@*'}; with this defined, running @samp{ll foo} in Eshell will
+actually run @samp{ls -l foo}.  Aliases defined (or deleted) by the
+@command{alias} command are automatically written to the file named by
+@code{eshell-aliases-file}, which you can also edit directly.  After
+doing so, use @w{@kbd{M-x eshell-read-aliases-list}} to load the
+edited aliases.
 
 @vindex $1, $2, @dots{}
 Note that unlike aliases in Bash, arguments must be handled
-explicitly.  Typically the alias definition would end in @samp{$*} to
-pass all arguments along.  More selective use of arguments via
-@samp{$1}, @samp{$2}, etc., is also possible.  For example,
-@kbd{alias mcd 'mkdir $1 && cd $1'} would cause @kbd{mcd foo} to
-create and switch to a directory called @samp{foo}.
+explicitly.  In most cases, you should end the alias definition with
+@samp{$@@*} to pass all arguments along, splicing them into your
+argument list (@pxref{Dollars Expansion}).  More selective use of
+arguments via @samp{$1}, @samp{$2}, etc., is also possible.  For
+example, @kbd{alias mcd 'mkdir $1 && cd $1'} would cause @kbd{mcd foo}
+to create and switch to a directory called @samp{foo}.
 
 @node History
 @section History
@@ -1307,12 +1309,36 @@ Dollars Expansion
 number.  For example, @samp{$@var{var}[: 0]} will return the first
 element of a colon-delimited string.
 
+@cindex length operator, in variable expansion
 @item $#@var{expr}
-Expands to the length of the result of @var{expr}, an expression in
-one of the above forms.  For example, @samp{$#@var{var}} returns the
-length of the variable @var{var} and @samp{$#@var{var}[0]} returns the
-length of the first element of @var{var}.  Again, signals an error if
-the result of @var{expr} is not a string or a sequence.
+This is the @dfn{length operator}.  It expands to the length of the
+result of @var{expr}, an expression in one of the above forms.  For
+example, @samp{$#@var{var}} returns the length of the variable
+@var{var} and @samp{$#@var{var}[0]} returns the length of the first
+element of @var{var}.  Again, signals an error if the result of
+@var{expr} is not a string or a sequence.
+
+@cindex splice operator, in variable expansion
+@item $@@@var{expr}
+This is the @dfn{splice operator}.  It ``splices'' the elements of
+@var{expr} (an expression of one of the above forms) into the
+resulting list of arguments, much like the @samp{,@@} marker in Emacs
+Lisp (@pxref{Backquote, , , elisp, The Emacs Lisp Reference Manual}).
+The elements of @var{expr} become arguments at the same level as the
+other arguments around it.  For example, if @var{numbers} is the list
+@code{(1 2 3)}, then:
+
+@example
+@group
+~ $ echo 0 $numbers
+(0
+ (1 2 3))
+@end group
+@group
+~ $ echo 0 $@@numbers
+(0 1 2 3)
+@end group
+@end example
 
 @end table
 
diff --git a/etc/NEWS b/etc/NEWS
index 3338c06f037..ab1dcc59922 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -71,6 +71,16 @@ switches for shortlogs, such as the one produced by 'C-x v L'.
 You can now configure how to display the "*buffer-selection*" buffer
 using this new option.  (Or set 'display-buffer-alist' directly.)
 
+** Eshell
+
++++
+*** New splice operator for Eshell dollar expansions.
+Dollar expansions in Eshell now let you splice the elements of the
+expansion in-place using '$@expr'.  This makes it easier to fill lists
+of arguments into a command, such as when defining alises.  For more
+information, see the "(eshell) Dollars Expansion" node in the Eshell
+manual.
+
 +++
 *** 'eshell-read-aliases-list' is now an interactive command.
 After manually editing 'eshell-aliases-file', you can use
diff --git a/lisp/eshell/em-cmpl.el b/lisp/eshell/em-cmpl.el
index ac82e3f225c..2c721eb9e31 100644
--- a/lisp/eshell/em-cmpl.el
+++ b/lisp/eshell/em-cmpl.el
@@ -342,17 +342,23 @@ eshell-complete-parse-arguments
 	(setq pos (1+ pos))))
     (setq posns (cdr posns))
     (cl-assert (= (length args) (length posns)))
-    (let ((a args)
-	  (i 0)
-	  l)
+    (let ((a args) (i 0) new-start)
       (while a
-	(if (and (consp (car a))
-		 (eq (caar a) 'eshell-operator))
-	    (setq l i))
-	(setq a (cdr a) i (1+ i)))
-      (and l
-	   (setq args (nthcdr (1+ l) args)
-		 posns (nthcdr (1+ l) posns))))
+        ;; Remove any top-level `eshell-splice-args' sigils.  These
+        ;; are meant to be rewritten and can't actually be called.
+        (when (and (consp (car a))
+                   (eq (caar a) 'eshell-splice-args))
+          (setcar a (cadar a)))
+        ;; If there's an unreplaced `eshell-operator' sigil, consider
+        ;; the token after it the new start of our arguments.
+        (when (and (consp (car a))
+                   (eq (caar a) 'eshell-operator))
+          (setq new-start i))
+        (setq a (cdr a)
+              i (1+ i)))
+      (when new-start
+	(setq args (nthcdr (1+ new-start) args)
+	      posns (nthcdr (1+ new-start) posns))))
     (cl-assert (= (length args) (length posns)))
     (when (and args (eq (char-syntax (char-before end)) ? )
 	       (not (eq (char-before (1- end)) ?\\)))
diff --git a/lisp/eshell/em-unix.el b/lisp/eshell/em-unix.el
index 4b5e4dd53ed..3f7ec618a33 100644
--- a/lisp/eshell/em-unix.el
+++ b/lisp/eshell/em-unix.el
@@ -786,10 +786,14 @@ 'eshell-complete-hostname
 
 (defun eshell-complete-host-reference ()
   "If there is a host reference, complete it."
-  (let ((arg (pcomplete-actual-arg))
-	index)
-    (when (setq index (string-match "@[a-z.]*\\'" arg))
-      (setq pcomplete-stub (substring arg (1+ index))
+  (let ((arg (pcomplete-actual-arg)))
+    (when (string-match
+           (rx ;; Match an "@", but not immediately following a "$".
+               (or string-start (not "$")) "@"
+               (group (* (any "a-z.")))
+               string-end)
+           arg)
+      (setq pcomplete-stub (substring arg (match-beginning 1))
 	    pcomplete-last-completion-raw t)
       (throw 'pcomplete-completions (pcomplete-read-host-names)))))
 
diff --git a/lisp/eshell/esh-arg.el b/lisp/eshell/esh-arg.el
index cfec04e183d..18dbaf4b14f 100644
--- a/lisp/eshell/esh-arg.el
+++ b/lisp/eshell/esh-arg.el
@@ -242,6 +242,7 @@ eshell-resolve-current-argument
   "If there are pending modifications to be made, make them now."
   (when eshell-current-argument
     (when eshell-arg-listified
+      (message "arg is %S" eshell-current-argument)
       (setq eshell-current-argument
             (append (list 'eshell-concat eshell-current-quoted)
                     eshell-current-argument))
@@ -348,6 +349,10 @@ eshell-operator
   "A stub function that generates an error if a floating operator is found."
   (error "Unhandled operator in input text"))
 
+(defsubst eshell-splice-args (&rest _args)
+  "A stub function that generates an error if a floating splice is found."
+  (error "Splice operator is not permitted in this context"))
+
 (defsubst eshell-looking-at-backslash-return (pos)
   "Test whether a backslash-return sequence occurs at POS."
   (and (eq (char-after pos) ?\\)
diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index 4a41bbe8fa1..a06d077ae5d 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -477,14 +477,29 @@ eshell-rewrite-initial-subcommand
 
 (defun eshell-rewrite-named-command (terms)
   "If no other rewriting rule transforms TERMS, assume a named command."
-  (let ((sym (if eshell-in-pipeline-p
-		 'eshell-named-command*
-	       'eshell-named-command))
-	(cmd (car terms))
-	(args (cdr terms)))
-    (if args
-	(list sym cmd `(list ,@(cdr terms)))
-      (list sym cmd))))
+  (let* ((sym (if eshell-in-pipeline-p
+		  'eshell-named-command*
+	        'eshell-named-command))
+         (splice-p nil)
+         ;; Group each term like ((list term-1) (list term-2) ...),
+         ;; splicing in `eshell-splice-args' terms.  This lets us
+         ;; apply spliced terms correctly below.
+         (grouped-terms (mapcar (lambda (i)
+                                  (if (eq (car-safe i) 'eshell-splice-args)
+                                      (progn
+                                        (setq splice-p t)
+                                        (cadr i))
+                                    `(list ,i)))
+                                terms)))
+    (cond
+     (splice-p
+      `(let ((terms (nconc ,@grouped-terms)))
+         (,sym (car terms) (cdr terms))))
+     ;; If no terms are spliced, use a simpler command form.
+     ((cdr terms)
+      (list sym (car terms) `(list ,@(cdr terms))))
+     (t
+      (list sym (car terms))))))
 
 (defvar eshell-command-body)
 (defvar eshell-test-body)
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 5824da6dc0e..ffc6a5c9145 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -320,10 +320,9 @@ eshell-interpolate-variable
   "Parse a variable interpolation.
 This function is explicit for adding to `eshell-parse-argument-hook'."
   (when (and (eq (char-after) ?$)
-	     (/= (1+ (point)) (point-max)))
+             (/= (1+ (point)) (point-max)))
     (forward-char)
-    (list 'eshell-escape-arg
-	  (eshell-parse-variable))))
+    (eshell-parse-variable)))
 
 (defun eshell/define (var-alias definition)
   "Define a VAR-ALIAS using DEFINITION."
@@ -453,6 +452,8 @@ eshell-parse-variable
 process any indices that come after the variable reference."
   (let* ((get-len (when (eq (char-after) ?#)
 		    (forward-char) t))
+         (splice (when (eq (char-after) ?@)
+                   (forward-char) t))
 	 value indices)
     (setq value (eshell-parse-variable-ref get-len)
 	  indices (and (not (eobp))
@@ -465,6 +466,9 @@ eshell-parse-variable
       (setq value `(length ,value)))
     (when eshell-current-quoted
       (setq value `(eshell-stringify ,value)))
+    (setq value `(eshell-escape-arg ,value))
+    (when splice
+      (setq value `(eshell-splice-args ,value)))
     value))
 
 (defun eshell-parse-variable-ref (&optional modifier-p)
@@ -753,7 +757,7 @@ eshell-complete-variable-reference
   "If there is a variable reference, complete it."
   (let ((arg (pcomplete-actual-arg)))
     (when (string-match
-           (rx "$" (? "#")
+           (rx "$" (? (or "#" "@"))
                (? (group (regexp eshell-variable-name-regexp)))
                string-end)
            arg)
diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el
index 96fde026a54..fead90bef4d 100644
--- a/test/lisp/eshell/esh-var-tests.el
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -131,6 +131,18 @@ esh-var-test/interp-var-length-alist
     (eshell-command-result-equal "echo $#eshell-test-value" 1)
     (eshell-command-result-equal "echo $#eshell-test-value[foo]" 3)))
 
+(ert-deftest esh-var-test/interp-var-splice ()
+  "Splice-interpolate list variable"
+  (let ((eshell-test-value '(1 2 3)))
+    (eshell-command-result-equal "echo a $@eshell-test-value z"
+                                 '("a" 1 2 3 "z"))))
+
+(ert-deftest esh-var-test/interp-var-splice-concat ()
+  "Splice-interpolate list variable"
+  (let ((eshell-test-value '(1 2 3)))
+    (eshell-command-result-equal "echo a$'eshell-test-value'z"
+                                 '("a1" 2 "3z"))))
+
 (ert-deftest esh-var-test/interp-lisp ()
   "Interpolate Lisp form evaluation"
   (eshell-command-result-equal "+ $(+ 1 2) 3" 6))
@@ -197,6 +209,9 @@ esh-var-test/interp-concat-cmd-external
    (eshell-match-command-output "echo ${echo hi}-${*echo there}"
                                 "hi-there\n")))
 
+\f
+;; Quoted variable interpolation
+
 (ert-deftest esh-var-test/quoted-interp-var ()
   "Interpolate variable inside double-quotes"
   (eshell-command-result-equal "echo \"$user-login-name\""
@@ -324,6 +339,21 @@ esh-var-test/quoted-interp-concat-cmd
   (eshell-command-result-equal "echo \"${echo \\\"foo\nbar\\\"} baz\""
                                "foo\nbar baz"))
 
+\f
+;; Interpolating commands
+
+(ert-deftest esh-var-test/command-interp ()
+  "Interpolate a variable as a command name"
+  (let ((eshell-test-value "printnl"))
+    (eshell-command-result-equal "$eshell-test-value hello there"
+                                 "hello\nthere\n")))
+
+(ert-deftest esh-var-test/command-interp-splice ()
+  "Interpolate a splice variable as a command name with arguments"
+  (let ((eshell-test-value '("printnl" "hello" "there")))
+    (eshell-command-result-equal "$@eshell-test-value"
+                                 "hello\nthere\n")))
+
 \f
 ;; Interpolated variable conversion
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-12-16  6:10 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-11  6:13 bug#59960: 30.0.50; Eshell's $* variable (used to define aliases) doesn't work as expected Jim Porter
2022-12-11  8:04 ` Eli Zaretskii
2022-12-12  4:36   ` Jim Porter
2022-12-12  6:49     ` Jim Porter
2022-12-12 12:27     ` Eli Zaretskii
2022-12-13  1:04       ` Jim Porter
2022-12-16  6:10         ` Jim Porter

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).