* bug#56509: 29.0.50; Eshell variable aliases don't fully support indexing
@ 2022-07-12 2:47 Jim Porter
2022-07-12 3:06 ` bug#56509: [PATCH] " Jim Porter
2022-07-16 12:09 ` Mattias Engdegård
0 siblings, 2 replies; 6+ messages in thread
From: Jim Porter @ 2022-07-12 2:47 UTC (permalink / raw)
To: 56509
Normally in Eshell, you can index variables to get the nth element, e.g.
"echo $exec-path[0]" would print the first element in your `exec-path'
variable. However, this doesn't always work for "alias" variables. From
"emacs -Q -f eshell":
~ $ alias first 'echo $1[0]'
~ $ first (list 1 2)
(1 2)
This should return 1, since it says to print the first (well, 0th)
element of the first argument.
There's also a related issue with alias variables that reference other
alias variables:
~ $ echo $PWD
/home/jim
~ $ echo $+
/home/jim
~ $ cd ..
/home $ echo $PWD
/home
/home $ echo $+
/home/jim
The '$+' alias is mapped to '$PWD', but rather than using Eshell's
definition for '$PWD', it erroneously consults the system's '$PWD'
environment variable, which in this case is the directory that Emacs was
started from.
Patch forthcoming; just getting a bug number.
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#56509: [PATCH] 29.0.50; Eshell variable aliases don't fully support indexing
2022-07-12 2:47 bug#56509: 29.0.50; Eshell variable aliases don't fully support indexing Jim Porter
@ 2022-07-12 3:06 ` Jim Porter
2022-07-12 4:11 ` Jim Porter
2022-07-12 13:11 ` bug#56509: " Lars Ingebrigtsen
2022-07-16 12:09 ` Mattias Engdegård
1 sibling, 2 replies; 6+ messages in thread
From: Jim Porter @ 2022-07-12 3:06 UTC (permalink / raw)
To: 56509
[-- Attachment #1: Type: text/plain, Size: 1375 bytes --]
On 7/11/2022 7:47 PM, Jim Porter wrote:
> Normally in Eshell, you can index variables to get the nth element, e.g.
> "echo $exec-path[0]" would print the first element in your `exec-path'
> variable. However, this doesn't always work for "alias" variables. From
[snip]
> There's also a related issue with alias variables that reference other
> alias variables...
The attached patches fix these issues. To help make this a bit easier to
follow, the first patch just adds tests and documentation. The only code
change I made was to move the '$INSIDE_EMACS' alias from em-dirs.el to
esh-var.el. That's because, in theory, a person might choose not to use
em-dirs.el (e.g. if they wanted their own implementation of directory
handling), and it would be pretty surprising for the '$INSIDE_EMACS'
variable to go away then too.
The second patch fixes the behavior of alias variables pointing to other
alias variables. While this is technically a change in existing
behavior, I think it's what the code intended, given the pre-existing
documentation for the '$+' variable.
Finally, the third patch makes sure indexing works on all the alias
variables. I tweaked the behavior of a couple of the alias variable
functions and also added a flag to members of
`eshell-variable-aliases-list' to use the "simple" behavior of passing
the result to `eshell-apply-indices'.
[-- Attachment #2: 0001-Improve-tests-organization-for-built-in-variables.patch --]
[-- Type: text/plain, Size: 18558 bytes --]
From afbe475271d3dd1074e7431a959bfd4add86fc0b Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 3 Jul 2022 20:05:29 -0700
Subject: [PATCH 1/3] Improve tests/organization for built-in variables
* lisp/eshell/em-dirs.el (eshell-inside-emacs)
(eshell-dirs-initialize): Move 'INSIDE_EMACS' from here...
* lisp/eshell/esh-var.el (eshell-inside-emacs)
(eshell-variable-aliases-alist): ... to here, and improve doc string.
* test/lisp/eshell/eshell-tests.el (eshell-test/inside-emacs-var):
Move from here...
* test/lisp/eshell/esh-var-tests.el (esh-var-test/inside-emacs-var):
... to here.
(esh-var-test/last-arg-var-indices)
(esh-var-test/last-arg-var-split-indices): New tests.
* test/lisp/eshell/em-alias-tests.el:
* test/lisp/eshell/em-dirs-tests.el:
* test/lisp/eshell-em-script-tests.el: New files.
* doc/misc/eshell.texi (Built-ins): Fix 'cd' documentation; it works
with the directory ring, not the directory stack. Move built-in
variables documentation from here...
(Variables): ... to here, and add documentation for missing built-in
variables.
---
doc/misc/eshell.texi | 49 +++++++++++++++-----
lisp/eshell/em-dirs.el | 5 --
lisp/eshell/esh-var.el | 18 +++++---
test/lisp/eshell/em-alias-tests.el | 64 +++++++++++++++++++++++++
test/lisp/eshell/em-dirs-tests.el | 72 +++++++++++++++++++++++++++++
test/lisp/eshell/em-script-tests.el | 62 +++++++++++++++++++++++++
test/lisp/eshell/esh-var-tests.el | 25 +++++++++-
test/lisp/eshell/eshell-tests.el | 7 ---
8 files changed, 269 insertions(+), 33 deletions(-)
create mode 100644 test/lisp/eshell/em-alias-tests.el
create mode 100644 test/lisp/eshell/em-dirs-tests.el
create mode 100644 test/lisp/eshell/em-script-tests.el
diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 6a348f37dc..a72fc925c6 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -354,11 +354,11 @@ Built-ins
directory (this is the same as @kbd{cd $-}).
@item
-The command @kbd{cd =} shows the directory stack. Each line is
+The command @kbd{cd =} shows the directory ring. Each line is
numbered.
@item
-With @kbd{cd =foo}, Eshell searches the directory stack for a directory
+With @kbd{cd =foo}, Eshell searches the directory ring for a directory
matching the regular expression @samp{foo}, and changes to that
directory.
@@ -845,23 +845,40 @@ Built-ins
@end defmac
+@node Variables
+@section Variables
+Since Eshell is just an Emacs REPL@footnote{Read-Eval-Print Loop}, it
+does not have its own scope, and simply stores variables the same you
+would in an Elisp program. Eshell provides a command version of
+@code{setq} for convenience.
+
@subsection Built-in variables
Eshell knows a few built-in variables:
@table @code
+@item $PWD
@item $+
+@vindex $PWD
@vindex $+
This variable always contains the current working directory.
+@item $OLDPWD
@item $-
+@vindex $OLDPWD
@vindex $-
This variable always contains the previous working directory (the
current working directory from before the last @code{cd} command).
+When using @code{$-}, you can also access older directories in the
+directory ring via subscripting, e.g. @samp{$-[1]} refers to the
+working directory @emph{before} the previous one.
@item $_
@vindex $_
-It refers to the last argument of the last command.
+This refers to the last argument of the last command. With a
+subscript, you can access any argument of the last command. For
+example, @samp{$_[1]} refers to the second argument of the last
+command (excluding the command name itself).
@item $$
@vindex $$
@@ -870,21 +887,29 @@ Built-ins
@item $?
@vindex $?
-This variable contains the exit code of the last command (0 or 1 for
-Lisp functions, based on successful completion).
+This variable contains the exit code of the last command. If the last
+command was a Lisp function, it is 0 for successful completion or 1
+otherwise.
+
+@item $COLUMNS
+@item $LINES
+@vindex $COLUMNS
+@vindex $LINES
+These variables tell the number of columns and lines, respectively,
+that are currently visible in the Eshell window. They are both
+copied to the environment, so external commands invoked from
+Eshell can consult them to do the right thing.
+
+@item $INSIDE_EMACS
+This variable indicates to external commands that they are being
+invoked from within Emacs so they can adjust their behavior if
+necessary. Its value is @code{@var{emacs-version},eshell}.
@end table
@xref{Aliases}, for the built-in variables @samp{$*}, @samp{$1},
@samp{$2}, @dots{}, in alias definitions.
-@node Variables
-@section Variables
-Since Eshell is just an Emacs REPL@footnote{Read-Eval-Print Loop}, it
-does not have its own scope, and simply stores variables the same you
-would in an Elisp program. Eshell provides a command version of
-@code{setq} for convenience.
-
@node Aliases
@section Aliases
diff --git a/lisp/eshell/em-dirs.el b/lisp/eshell/em-dirs.el
index 5396044d8c..a3cf0b9131 100644
--- a/lisp/eshell/em-dirs.el
+++ b/lisp/eshell/em-dirs.el
@@ -168,9 +168,6 @@ eshell-dirstack
(defvar eshell-last-dir-ring nil
"The last directory that Eshell was in.")
-(defconst eshell-inside-emacs (format "%s,eshell" emacs-version)
- "Value for the `INSIDE_EMACS' environment variable.")
-
;;; Functions:
(defun eshell-dirs-initialize () ;Called from `eshell-mode' via intern-soft!
@@ -193,8 +190,6 @@ eshell-dirs-initialize
(unless (ring-empty-p eshell-last-dir-ring)
(expand-file-name
(ring-ref eshell-last-dir-ring 0))))
- t)
- ("INSIDE_EMACS" eshell-inside-emacs
t))))
(when eshell-cd-on-directory
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 17add9b668..5092d2c6a5 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -113,6 +113,9 @@
(require 'pcomplete)
(require 'ring)
+(defconst eshell-inside-emacs (format "%s,eshell" emacs-version)
+ "Value for the `INSIDE_EMACS' environment variable.")
+
(defgroup eshell-var nil
"Variable interpolation is introduced whenever the `$' character
appears unquoted in any argument (except when that argument is
@@ -151,6 +154,7 @@ eshell-variable-aliases-list
`(;; for eshell.el
("COLUMNS" ,(lambda (_indices) (window-body-width nil 'remap)) t)
("LINES" ,(lambda (_indices) (window-body-height nil 'remap)) t)
+ ("INSIDE_EMACS" eshell-inside-emacs t)
;; for eshell-cmd.el
("_" ,(lambda (indices)
@@ -160,6 +164,8 @@ eshell-variable-aliases-list
indices))))
("?" eshell-last-command-status)
("$" eshell-last-command-result)
+
+ ;; for em-alias.el and em-script.el
("0" eshell-command-name)
("1" ,(lambda (_indices) (nth 0 eshell-command-arguments)))
("2" ,(lambda (_indices) (nth 1 eshell-command-arguments)))
@@ -180,13 +186,11 @@ eshell-variable-aliases-list
compute the string value that will be returned when the variable is
accessed via the syntax `$NAME'.
-If the value is a function, call that function with two arguments: the
-list of the indices that was used in the reference, and whether the
-user is requesting the length of the ultimate element. For example, a
-reference of `$NAME[10][20]' would result in the function for alias
-`NAME' being called (assuming it were aliased to a function), and the
-arguments passed to this function would be the list `(10 20)', and
-nil.
+If the value is a function, call that function with one argument: the
+list of the indices that was used in the reference. For example, if
+`NAME' were aliased to a function, a reference of `$NAME[10][20]'
+would result in that function being called with the argument
+`((\"10\") (\"20\"))'. (For more details, see `eshell-apply-indices').
If the value is a string, return the value for the variable with that
name in the current environment. If no variable with that name exists
diff --git a/test/lisp/eshell/em-alias-tests.el b/test/lisp/eshell/em-alias-tests.el
new file mode 100644
index 0000000000..762f125a78
--- /dev/null
+++ b/test/lisp/eshell/em-alias-tests.el
@@ -0,0 +1,64 @@
+;;; em-alias-tests.el --- em-alias test suite -*- lexical-binding:t -*-
+
+;; Copyright (C) 2022 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs. If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; Tests for Eshell's alias module.
+
+;;; Code:
+
+(require 'ert)
+(require 'esh-mode)
+(require 'eshell)
+(require 'em-alias)
+
+(require 'eshell-tests-helpers
+ (expand-file-name "eshell-tests-helpers"
+ (file-name-directory (or load-file-name
+ default-directory))))
+;;; Tests:
+
+(ert-deftest em-alias-test/simple-alias ()
+ "Test a simple alias with no arguments"
+ (with-temp-eshell
+ (eshell-insert-command "alias say-hi 'echo hi'")
+ (eshell-command-result-p "say-hi" "hi\n")
+ (eshell-command-result-p "say-hi bye" "hi\n")))
+
+(ert-deftest em-alias-test/alias-arg-vars ()
+ "Test alias with $0, $1, ... variables"
+ (with-temp-eshell
+ (eshell-insert-command "alias show-args 'printnl $0 \"$1 $2\"'")
+ (eshell-command-result-p "show-args one two" "show-args\none two\n")))
+
+(ert-deftest em-alias-test/alias-all-args-var ()
+ "Test alias with the $* variable"
+ (with-temp-eshell
+ (eshell-insert-command "alias show-all-args 'printnl $*'")
+ (eshell-command-result-p "show-all-args" "\\`\\'")
+ (eshell-command-result-p "show-all-args a" "a\n")
+ (eshell-command-result-p "show-all-args a b c" "a\nb\nc\n")))
+
+(ert-deftest em-alias-test/alias-all-args-var-indices ()
+ "Test alias with the $* variable using indices"
+ (with-temp-eshell
+ (eshell-insert-command "alias add-pair '+ $*[0] $*[1]'")
+ (eshell-command-result-p "add-pair 1 2" "3\n")))
+
+;; em-alias-tests.el ends here
diff --git a/test/lisp/eshell/em-dirs-tests.el b/test/lisp/eshell/em-dirs-tests.el
new file mode 100644
index 0000000000..eb27acd208
--- /dev/null
+++ b/test/lisp/eshell/em-dirs-tests.el
@@ -0,0 +1,72 @@
+;;; em-dirs-tests.el --- em-dirs test suite -*- lexical-binding:t -*-
+
+;; Copyright (C) 2022 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs. If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; Tests for Eshell's dirs module.
+
+;;; Code:
+
+(require 'ert)
+(require 'esh-mode)
+(require 'eshell)
+(require 'em-dirs)
+
+(require 'eshell-tests-helpers
+ (expand-file-name "eshell-tests-helpers"
+ (file-name-directory (or load-file-name
+ default-directory))))
+;;; Tests:
+
+(ert-deftest em-dirs-test/pwd-var ()
+ "Test using the $PWD variable."
+ (should (equal (eshell-test-command-result "echo $PWD")
+ (expand-file-name (eshell/pwd)))))
+
+(ert-deftest em-dirs-test/short-pwd-var ()
+ "Test using the $+ (current directory) variable."
+ (should (equal (eshell-test-command-result "echo $+")
+ (expand-file-name (eshell/pwd)))))
+
+(ert-deftest em-dirs-test/oldpwd-var ()
+ "Test using the $OLDPWD variable."
+ (let (eshell-last-dir-ring-file-name)
+ (with-temp-eshell
+ (eshell-command-result-p "echo $OLDPWD"
+ "\\`\\'")
+ (ring-insert eshell-last-dir-ring "/some/path")
+ (eshell-command-result-p "echo $OLDPWD"
+ "/some/path\n"))))
+
+(ert-deftest em-dirs-test/directory-ring-var ()
+ "Test using the $- (directory ring) variable."
+ (let (eshell-last-dir-ring-file-name)
+ (with-temp-eshell
+ (eshell-command-result-p "echo $-"
+ "\\`\\'")
+ (ring-insert eshell-last-dir-ring "/some/path")
+ (ring-insert eshell-last-dir-ring "/other/path")
+ (eshell-command-result-p "echo $-"
+ "/other/path\n")
+ (eshell-command-result-p "echo $-[0]"
+ "/other/path\n")
+ (eshell-command-result-p "echo $-[1]"
+ "/some/path\n"))))
+
+;; em-dirs-tests.el ends here
diff --git a/test/lisp/eshell/em-script-tests.el b/test/lisp/eshell/em-script-tests.el
new file mode 100644
index 0000000000..a34f943402
--- /dev/null
+++ b/test/lisp/eshell/em-script-tests.el
@@ -0,0 +1,62 @@
+;;; em-script-tests.el --- em-script test suite -*- lexical-binding:t -*-
+
+;; Copyright (C) 2022 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs. If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; Tests for Eshell's script module.
+
+;;; Code:
+
+(require 'ert)
+(require 'esh-mode)
+(require 'eshell)
+(require 'em-script)
+
+(require 'eshell-tests-helpers
+ (expand-file-name "eshell-tests-helpers"
+ (file-name-directory (or load-file-name
+ default-directory))))
+;;; Tests:
+
+(ert-deftest em-script-test/source-script ()
+ "Test sourcing script with no argumentss"
+ (ert-with-temp-file temp-file :text "echo hi"
+ (with-temp-eshell
+ (eshell-command-result-p (format "source %s" temp-file)
+ "hi\n"))))
+
+(ert-deftest em-script-test/source-script-arg-vars ()
+ "Test sourcing script with $0, $1, ... variables"
+ (ert-with-temp-file temp-file :text "printnl $0 \"$1 $2\""
+ (with-temp-eshell
+ (eshell-command-result-p (format "source %s one two" temp-file)
+ (format "%s\none two\n" temp-file)))))
+
+(ert-deftest em-script-test/source-script-all-args-var ()
+ "Test sourcing script with the $* variable"
+ (ert-with-temp-file temp-file :text "printnl $*"
+ (with-temp-eshell
+ (eshell-command-result-p (format "source %s" temp-file)
+ "\\`\\'")
+ (eshell-command-result-p (format "source %s a" temp-file)
+ "a\n")
+ (eshell-command-result-p (format "source %s a b c" temp-file)
+ "a\nb\nc\n"))))
+
+;; em-script-tests.el ends here
diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el
index 3180fe7a5f..955190aee0 100644
--- a/test/lisp/eshell/esh-var-tests.el
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -487,6 +487,13 @@ esh-var-test/columns-var
(should (equal (eshell-test-command-result "echo $COLUMNS")
(window-body-width nil 'remap))))
+(ert-deftest esh-var-test/inside-emacs-var ()
+ "Test presence of \"INSIDE_EMACS\" in subprocesses"
+ (with-temp-eshell
+ (eshell-command-result-p "env"
+ (format "INSIDE_EMACS=%s,eshell"
+ emacs-version))))
+
(ert-deftest esh-var-test/last-result-var ()
"Test using the \"last result\" ($$) variable"
(with-temp-eshell
@@ -497,12 +504,26 @@ esh-var-test/last-result-var2
"Test using the \"last result\" ($$) variable twice"
(with-temp-eshell
(eshell-command-result-p "+ 1 2; + $$ $$"
- "3\n6\n")))
+ "3\n6\n")))
(ert-deftest esh-var-test/last-arg-var ()
"Test using the \"last arg\" ($_) variable"
(with-temp-eshell
(eshell-command-result-p "+ 1 2; + $_ 4"
- "3\n6\n")))
+ "3\n6\n")))
+
+(ert-deftest esh-var-test/last-arg-var-indices ()
+ "Test using the \"last arg\" ($_) variable with indices"
+ (with-temp-eshell
+ (eshell-command-result-p "+ 1 2; + $_[0] 4"
+ "3\n5\n")
+ (eshell-command-result-p "+ 1 2; + $_[1] 4"
+ "3\n6\n")))
+
+(ert-deftest esh-var-test/last-arg-var-split-indices ()
+ "Test using the \"last arg\" ($_) variable with split indices"
+ (with-temp-eshell
+ (eshell-command-result-p "concat 01:02 03:04; echo $_[0][: 1]"
+ "01:0203:04\n2\n")))
;; esh-var-tests.el ends here
diff --git a/test/lisp/eshell/eshell-tests.el b/test/lisp/eshell/eshell-tests.el
index ab5d73d479..5dc1877548 100644
--- a/test/lisp/eshell/eshell-tests.el
+++ b/test/lisp/eshell/eshell-tests.el
@@ -178,13 +178,6 @@ eshell-test/redirect-buffer-escaped
(string-replace "\\" "\\\\" bufname))))
(should (equal (buffer-string) "hi")))))
-(ert-deftest eshell-test/inside-emacs-var ()
- "Test presence of \"INSIDE_EMACS\" in subprocesses"
- (with-temp-eshell
- (eshell-command-result-p "env"
- (format "INSIDE_EMACS=%s,eshell"
- emacs-version))))
-
(ert-deftest eshell-test/escape-nonspecial ()
"Test that \"\\c\" and \"c\" are equivalent when \"c\" is not a
special character."
--
2.25.1
[-- Attachment #3: 0002-Allow-Eshell-variable-aliases-to-point-to-other-alia.patch --]
[-- Type: text/plain, Size: 3726 bytes --]
From 5f0f60ebcce61954324c172700fa8ac924b605f7 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Fri, 8 Jul 2022 18:41:07 -0700
Subject: [PATCH 2/3] Allow Eshell variable aliases to point to other aliases
In particular, this resolves an issue where '$+' referenced the real
environment variable '$PWD' instead of the Eshell variable alias of
the same name. This meant that changing directories in Eshell
wouldn't update the value of '$+'.
* lisp/eshell/esh-var.el (eshell-get-variable): Allow Eshell variable
aliaes to point to other aliases.
* test/lisp/eshell/em-dirs-tests.el (em-dirs-test/pwd-var)
(em-dirs-test/short-pwd-var): Adapt tests to check this case
(bug#56509).
---
lisp/eshell/esh-var.el | 41 +++++++++++++++----------------
test/lisp/eshell/em-dirs-tests.el | 10 +++++---
2 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 5092d2c6a5..e1535c1c5d 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -549,27 +549,26 @@ eshell-get-variable
"Get the value for the variable NAME.
INDICES is a list of index-lists (see `eshell-parse-indices').
If QUOTED is non-nil, this was invoked inside double-quotes."
- (let* ((alias (assoc name eshell-variable-aliases-list))
- (var (if alias
- (cadr alias)
- name)))
- (if (and alias (functionp var))
- (funcall var indices)
- (eshell-apply-indices
- (cond
- ((stringp var)
- (let ((sym (intern-soft var)))
- (if (and sym (boundp sym)
- (or eshell-prefer-lisp-variables
- (memq sym eshell--local-vars) ; bug#15372
- (not (getenv var))))
- (symbol-value sym)
- (getenv var))))
- ((symbolp var)
- (symbol-value var))
- (t
- (error "Unknown variable `%s'" (eshell-stringify var))))
- indices quoted))))
+ (if-let ((alias (assoc name eshell-variable-aliases-list)))
+ (let ((target (cadr alias)))
+ (cond
+ ((functionp target)
+ (funcall target indices))
+ ((symbolp target)
+ (eshell-apply-indices (symbol-value target) indices quoted))
+ (t
+ (eshell-get-variable target indices quoted))))
+ (unless (stringp name)
+ (error "Unknown variable `%s'" (eshell-stringify name)))
+ (eshell-apply-indices
+ (let ((sym (intern-soft name)))
+ (if (and sym (boundp sym)
+ (or eshell-prefer-lisp-variables
+ (memq sym eshell--local-vars) ; bug#15372
+ (not (getenv name))))
+ (symbol-value sym)
+ (getenv name)))
+ indices quoted)))
(defun eshell-apply-indices (value indices &optional quoted)
"Apply to VALUE all of the given INDICES, returning the sub-result.
diff --git a/test/lisp/eshell/em-dirs-tests.el b/test/lisp/eshell/em-dirs-tests.el
index eb27acd208..69480051e4 100644
--- a/test/lisp/eshell/em-dirs-tests.el
+++ b/test/lisp/eshell/em-dirs-tests.el
@@ -36,13 +36,15 @@
(ert-deftest em-dirs-test/pwd-var ()
"Test using the $PWD variable."
- (should (equal (eshell-test-command-result "echo $PWD")
- (expand-file-name (eshell/pwd)))))
+ (let ((default-directory "/some/path"))
+ (should (equal (eshell-test-command-result "echo $PWD")
+ (expand-file-name default-directory)))))
(ert-deftest em-dirs-test/short-pwd-var ()
"Test using the $+ (current directory) variable."
- (should (equal (eshell-test-command-result "echo $+")
- (expand-file-name (eshell/pwd)))))
+ (let ((default-directory "/some/path"))
+ (should (equal (eshell-test-command-result "echo $+")
+ (expand-file-name default-directory)))))
(ert-deftest em-dirs-test/oldpwd-var ()
"Test using the $OLDPWD variable."
--
2.25.1
[-- Attachment #4: 0003-Ensure-Eshell-variable-aliases-properly-handle-index.patch --]
[-- Type: text/plain, Size: 14869 bytes --]
From 1bdccd3c34a5355183fd0fee4cadb99dd5675151 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Wed, 6 Jul 2022 21:59:11 -0700
Subject: [PATCH 3/3] Ensure Eshell variable aliases properly handle indexing
* lisp/eshell/em-dirs.el (eshell-dirs-initialize): Properly handle
indexing for variable aliases.
* lisp/eshell/esh-var (eshell-variable-aliases-list): Properly handle
indexing for variable aliases, and add SIMPLE-FUNCTION entry for
aliases.
(eshell-get-variable): Update how variable alias functions are called.
* test/lisp/eshell/em-alias-tests.el
(em-alias-test/alias-arg-vars-indices)
(em-alias-test/alias-arg-vars-split-indices)
(em-alias-test/alias-all-args-var-split-indices):
* test/lisp/eshell/em-dirs-tests.el (em-dirs-test/pwd-var-indices)
(em-dirs-test/oldpwd-var-indices)
(em-dirs-test/directory-ring-var-indices):
* test/lisp/eshell/esh-var-tests.el
(esh-var-test/inside-emacs-var-split-indices)
(esh-var-test/last-result-var-split-indices): New tests.
(esh-var-test/last-arg-var-split-indices): Expand test to check
conversion behavior inside double quotes (bug#56509).
---
lisp/eshell/em-dirs.el | 36 ++++++++-------
lisp/eshell/esh-var.el | 73 ++++++++++++++++++------------
test/lisp/eshell/em-alias-tests.el | 23 ++++++++++
test/lisp/eshell/em-dirs-tests.el | 28 ++++++++++++
test/lisp/eshell/esh-var-tests.el | 22 ++++++++-
5 files changed, 134 insertions(+), 48 deletions(-)
diff --git a/lisp/eshell/em-dirs.el b/lisp/eshell/em-dirs.el
index a3cf0b9131..00880b9f28 100644
--- a/lisp/eshell/em-dirs.el
+++ b/lisp/eshell/em-dirs.el
@@ -175,22 +175,26 @@ eshell-dirs-initialize
(setq-local eshell-variable-aliases-list
(append
eshell-variable-aliases-list
- `(("-" ,(lambda (indices)
- (if (not indices)
- (unless (ring-empty-p eshell-last-dir-ring)
- (expand-file-name
- (ring-ref eshell-last-dir-ring 0)))
- (expand-file-name
- (eshell-apply-indices eshell-last-dir-ring indices)))))
- ("+" "PWD")
- ("PWD" ,(lambda (_indices)
- (expand-file-name (eshell/pwd)))
- t)
- ("OLDPWD" ,(lambda (_indices)
- (unless (ring-empty-p eshell-last-dir-ring)
- (expand-file-name
- (ring-ref eshell-last-dir-ring 0))))
- t))))
+ `(("-" ,(lambda (indices quoted)
+ (if (not indices)
+ (unless (ring-empty-p eshell-last-dir-ring)
+ (expand-file-name
+ (ring-ref eshell-last-dir-ring 0)))
+ ;; Apply the first index, expand the file name,
+ ;; and then apply the rest of the indices.
+ (eshell-apply-indices
+ (expand-file-name
+ (eshell-apply-indices eshell-last-dir-ring
+ (list (car indices)) quoted))
+ (cdr indices) quoted))))
+ ("+" "PWD")
+ ("PWD" ,(lambda () (expand-file-name (eshell/pwd)))
+ t t)
+ ("OLDPWD" ,(lambda ()
+ (unless (ring-empty-p eshell-last-dir-ring)
+ (expand-file-name
+ (ring-ref eshell-last-dir-ring 0))))
+ t t))))
(when eshell-cd-on-directory
(setq-local eshell-interpreter-alist
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index e1535c1c5d..cbd7942de4 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -152,59 +152,63 @@ eshell-variable-name-regexp
(defcustom eshell-variable-aliases-list
`(;; for eshell.el
- ("COLUMNS" ,(lambda (_indices) (window-body-width nil 'remap)) t)
- ("LINES" ,(lambda (_indices) (window-body-height nil 'remap)) t)
+ ("COLUMNS" ,(lambda () (window-body-width nil 'remap)) t t)
+ ("LINES" ,(lambda () (window-body-height nil 'remap)) t t)
("INSIDE_EMACS" eshell-inside-emacs t)
;; for eshell-cmd.el
- ("_" ,(lambda (indices)
+ ("_" ,(lambda (indices quoted)
(if (not indices)
(car (last eshell-last-arguments))
(eshell-apply-indices eshell-last-arguments
- indices))))
+ indices quoted))))
("?" eshell-last-command-status)
("$" eshell-last-command-result)
;; for em-alias.el and em-script.el
("0" eshell-command-name)
- ("1" ,(lambda (_indices) (nth 0 eshell-command-arguments)))
- ("2" ,(lambda (_indices) (nth 1 eshell-command-arguments)))
- ("3" ,(lambda (_indices) (nth 2 eshell-command-arguments)))
- ("4" ,(lambda (_indices) (nth 3 eshell-command-arguments)))
- ("5" ,(lambda (_indices) (nth 4 eshell-command-arguments)))
- ("6" ,(lambda (_indices) (nth 5 eshell-command-arguments)))
- ("7" ,(lambda (_indices) (nth 6 eshell-command-arguments)))
- ("8" ,(lambda (_indices) (nth 7 eshell-command-arguments)))
- ("9" ,(lambda (_indices) (nth 8 eshell-command-arguments)))
- ("*" ,(lambda (indices)
- (if (not indices)
- eshell-command-arguments
- (eshell-apply-indices eshell-command-arguments
- indices)))))
+ ("1" ,(lambda () (nth 0 eshell-command-arguments)) nil t)
+ ("2" ,(lambda () (nth 1 eshell-command-arguments)) nil t)
+ ("3" ,(lambda () (nth 2 eshell-command-arguments)) nil t)
+ ("4" ,(lambda () (nth 3 eshell-command-arguments)) nil t)
+ ("5" ,(lambda () (nth 4 eshell-command-arguments)) nil t)
+ ("6" ,(lambda () (nth 5 eshell-command-arguments)) nil t)
+ ("7" ,(lambda () (nth 6 eshell-command-arguments)) nil t)
+ ("8" ,(lambda () (nth 7 eshell-command-arguments)) nil t)
+ ("9" ,(lambda () (nth 8 eshell-command-arguments)) nil t)
+ ("*" eshell-command-arguments))
"This list provides aliasing for variable references.
-Each member defines the name of a variable, and a Lisp value used to
+Each member is of the following form:
+
+ (NAME VALUE [COPY-TO-ENVIRONMENT] [SIMPLE-FUNCTION])
+
+NAME defines the name of the variable, VALUE is a Lisp value used to
compute the string value that will be returned when the variable is
accessed via the syntax `$NAME'.
-If the value is a function, call that function with one argument: the
-list of the indices that was used in the reference. For example, if
+If VALUE is a function, its behavior depends on the value of
+SIMPLE-FUNCTION. If SIMPLE-FUNCTION is nil, call VALUE with two
+arguments: the list of the indices that was used in the reference and
+whether the variable was used within double quotes. For example, if
`NAME' were aliased to a function, a reference of `$NAME[10][20]'
-would result in that function being called with the argument
-`((\"10\") (\"20\"))'. (For more details, see `eshell-apply-indices').
+would result in that function being called with the arguments
+`((\"10\") (\"20\"))' and nil. If SIMPLE-FUNCTION is non-nil, call
+the function with no arguments and then pass its result to
+`eshell-apply-indices'.
-If the value is a string, return the value for the variable with that
+If VALUE is a string, return the value for the variable with that
name in the current environment. If no variable with that name exists
in the environment, but if a symbol with that same name exists and has
a value bound to it, return its value instead. You can prioritize
symbol values over environment values by setting
`eshell-prefer-lisp-variables' to t.
-If the value is a symbol, return the value bound to it.
+If VALUE is a symbol, return the value bound to it.
-If the value has any other type, signal an error.
+If VALUE has any other type, signal an error.
-Additionally, each member may specify if it should be copied to the
-environment of created subprocesses."
+Additionally, if COPY-TO-ENVIRONMENT is non-nil, the alias should be
+copied to the environment of created subprocesses."
:type '(repeat (list string sexp
(choice (const :tag "Copy to environment" t)
(const :tag "Use only in Eshell" nil))))
@@ -550,10 +554,19 @@ eshell-get-variable
INDICES is a list of index-lists (see `eshell-parse-indices').
If QUOTED is non-nil, this was invoked inside double-quotes."
(if-let ((alias (assoc name eshell-variable-aliases-list)))
- (let ((target (cadr alias)))
+ (let ((target (nth 1 alias)))
(cond
((functionp target)
- (funcall target indices))
+ (if (nth 3 alias)
+ (eshell-apply-indices (funcall target) indices quoted)
+ (condition-case nil
+ (funcall target indices quoted)
+ (wrong-number-of-arguments
+ (display-warning
+ :warning (concat "Function for `eshell-variable-aliases-list' "
+ "entry should accept two arguments: INDICES "
+ "and QUOTED.'"))
+ (funcall target indices)))))
((symbolp target)
(eshell-apply-indices (symbol-value target) indices quoted))
(t
diff --git a/test/lisp/eshell/em-alias-tests.el b/test/lisp/eshell/em-alias-tests.el
index 762f125a78..497159e346 100644
--- a/test/lisp/eshell/em-alias-tests.el
+++ b/test/lisp/eshell/em-alias-tests.el
@@ -47,6 +47,23 @@ em-alias-test/alias-arg-vars
(eshell-insert-command "alias show-args 'printnl $0 \"$1 $2\"'")
(eshell-command-result-p "show-args one two" "show-args\none two\n")))
+(ert-deftest em-alias-test/alias-arg-vars-indices ()
+ "Test alias with $1, $2, ... variables using indices"
+ (with-temp-eshell
+ (eshell-insert-command "alias funny-sum '+ $1[0] $2[1]'")
+ (eshell-command-result-p "funny-sum (list 1 2) (list 3 4)"
+ "5\n")))
+
+(ert-deftest em-alias-test/alias-arg-vars-split-indices ()
+ "Test alias with $0, $1, ... variables using split indices"
+ (with-temp-eshell
+ (eshell-insert-command "alias my-prefix 'echo $0[- 0]'")
+ (eshell-command-result-p "my-prefix"
+ "my\n")
+ (eshell-insert-command "alias funny-sum '+ $1[: 0] $2[: 1]'")
+ (eshell-command-result-p "funny-sum 1:2 3:4"
+ "5\n")))
+
(ert-deftest em-alias-test/alias-all-args-var ()
"Test alias with the $* variable"
(with-temp-eshell
@@ -61,4 +78,10 @@ em-alias-test/alias-all-args-var-indices
(eshell-insert-command "alias add-pair '+ $*[0] $*[1]'")
(eshell-command-result-p "add-pair 1 2" "3\n")))
+(ert-deftest em-alias-test/alias-all-args-var-split-indices ()
+ "Test alias with the $* variable using split indices"
+ (with-temp-eshell
+ (eshell-insert-command "alias add-funny-pair '+ $*[0][: 0] $*[1][: 1]'")
+ (eshell-command-result-p "add-funny-pair 1:2 3:4" "5\n")))
+
;; em-alias-tests.el ends here
diff --git a/test/lisp/eshell/em-dirs-tests.el b/test/lisp/eshell/em-dirs-tests.el
index 69480051e4..8e96cc0747 100644
--- a/test/lisp/eshell/em-dirs-tests.el
+++ b/test/lisp/eshell/em-dirs-tests.el
@@ -40,6 +40,14 @@ em-dirs-test/pwd-var
(should (equal (eshell-test-command-result "echo $PWD")
(expand-file-name default-directory)))))
+(ert-deftest em-dirs-test/pwd-var-indices ()
+ "Test using the $PWD variable with indices."
+ (let ((default-directory "/some/path/here"))
+ (should (equal (eshell-test-command-result "echo $PWD[/ 1]")
+ "some"))
+ (should (equal (eshell-test-command-result "echo $PWD[/ 1 3]")
+ '("some" "here")))))
+
(ert-deftest em-dirs-test/short-pwd-var ()
"Test using the $+ (current directory) variable."
(let ((default-directory "/some/path"))
@@ -56,6 +64,16 @@ em-dirs-test/oldpwd-var
(eshell-command-result-p "echo $OLDPWD"
"/some/path\n"))))
+(ert-deftest em-dirs-test/oldpwd-var-indices ()
+ "Test using the $OLDPWD variable with indices."
+ (let (eshell-last-dir-ring-file-name)
+ (with-temp-eshell
+ (ring-insert eshell-last-dir-ring "/some/path/here")
+ (eshell-command-result-p "echo $OLDPWD[/ 1]"
+ "some\n")
+ (eshell-command-result-p "echo $OLDPWD[/ 1 3]"
+ "(\"some\" \"here\")\n"))))
+
(ert-deftest em-dirs-test/directory-ring-var ()
"Test using the $- (directory ring) variable."
(let (eshell-last-dir-ring-file-name)
@@ -71,4 +89,14 @@ em-dirs-test/directory-ring-var
(eshell-command-result-p "echo $-[1]"
"/some/path\n"))))
+(ert-deftest em-dirs-test/directory-ring-var-indices ()
+ "Test using the $- (directory ring) variable with multiple indices."
+ (let (eshell-last-dir-ring-file-name)
+ (with-temp-eshell
+ (ring-insert eshell-last-dir-ring "/some/path/here")
+ (eshell-command-result-p "echo $-[0][/ 1]"
+ "some\n")
+ (eshell-command-result-p "echo $-[1][/ 1 3]"
+ "(\"some\" \"here\")\n"))))
+
;; em-dirs-tests.el ends here
diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el
index 955190aee0..54e701a6aa 100644
--- a/test/lisp/eshell/esh-var-tests.el
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -494,6 +494,12 @@ esh-var-test/inside-emacs-var
(format "INSIDE_EMACS=%s,eshell"
emacs-version))))
+(ert-deftest esh-var-test/inside-emacs-var-split-indices ()
+ "Test using \"INSIDE_EMACS\" with split indices"
+ (with-temp-eshell
+ (eshell-command-result-p "echo $INSIDE_EMACS[, 1]"
+ "eshell")))
+
(ert-deftest esh-var-test/last-result-var ()
"Test using the \"last result\" ($$) variable"
(with-temp-eshell
@@ -506,6 +512,16 @@ esh-var-test/last-result-var2
(eshell-command-result-p "+ 1 2; + $$ $$"
"3\n6\n")))
+(ert-deftest esh-var-test/last-result-var-split-indices ()
+ "Test using the \"last result\" ($$) variable with split indices"
+ (with-temp-eshell
+ (eshell-command-result-p
+ "string-join (list \"01\" \"02\") :; + $$[: 1] 3"
+ "01:02\n5\n")
+ (eshell-command-result-p
+ "string-join (list \"01\" \"02\") :; echo \"$$[: 1]\""
+ "01:02\n02\n")))
+
(ert-deftest esh-var-test/last-arg-var ()
"Test using the \"last arg\" ($_) variable"
(with-temp-eshell
@@ -523,7 +539,9 @@ esh-var-test/last-arg-var-indices
(ert-deftest esh-var-test/last-arg-var-split-indices ()
"Test using the \"last arg\" ($_) variable with split indices"
(with-temp-eshell
- (eshell-command-result-p "concat 01:02 03:04; echo $_[0][: 1]"
- "01:0203:04\n2\n")))
+ (eshell-command-result-p "concat 01:02 03:04; + $_[0][: 1] 5"
+ "01:0203:04\n7\n")
+ (eshell-command-result-p "concat 01:02 03:04; echo \"$_[0][: 1]\""
+ "01:0203:04\n02\n")))
;; esh-var-tests.el ends here
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* bug#56509: [PATCH] 29.0.50; Eshell variable aliases don't fully support indexing
2022-07-12 3:06 ` bug#56509: [PATCH] " Jim Porter
@ 2022-07-12 4:11 ` Jim Porter
2022-07-12 13:11 ` bug#56509: " Lars Ingebrigtsen
1 sibling, 0 replies; 6+ messages in thread
From: Jim Porter @ 2022-07-12 4:11 UTC (permalink / raw)
To: 56509
On 7/11/2022 8:06 PM, Jim Porter wrote:
> Finally, the third patch makes sure indexing works on all the alias
> variables. I tweaked the behavior of a couple of the alias variable
> functions and also added a flag to members of
> `eshell-variable-aliases-list' to use the "simple" behavior of passing
> the result to `eshell-apply-indices'.
Oh, I forgot to mention: in the third patch, I changed how many
arguments are passed to functions in `eshell-variable-aliases-list'. I
tried to add a backwards-compatible fallback for the old way in
`eshell-get-variable'. Maybe there's a better way to do this than what I
came up with, though...
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#56509: 29.0.50; Eshell variable aliases don't fully support indexing
2022-07-12 3:06 ` bug#56509: [PATCH] " Jim Porter
2022-07-12 4:11 ` Jim Porter
@ 2022-07-12 13:11 ` Lars Ingebrigtsen
1 sibling, 0 replies; 6+ messages in thread
From: Lars Ingebrigtsen @ 2022-07-12 13:11 UTC (permalink / raw)
To: Jim Porter; +Cc: 56509
Jim Porter <jporterbugs@gmail.com> writes:
> The attached patches fix these issues.
Thanks; pushed to Emacs 29.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#56509: 29.0.50; Eshell variable aliases don't fully support indexing
2022-07-12 2:47 bug#56509: 29.0.50; Eshell variable aliases don't fully support indexing Jim Porter
2022-07-12 3:06 ` bug#56509: [PATCH] " Jim Porter
@ 2022-07-16 12:09 ` Mattias Engdegård
2022-07-17 3:13 ` Jim Porter
1 sibling, 1 reply; 6+ messages in thread
From: Mattias Engdegård @ 2022-07-16 12:09 UTC (permalink / raw)
To: Lars Ingebrigtsen; +Cc: 56509, Jim Porter
The defcustom tests failed because the initial value of `eshell-variable-aliases-list` no longer corresponded to its declared type, so the type has now been updated (ea9b442b82). Please check.
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#56509: 29.0.50; Eshell variable aliases don't fully support indexing
2022-07-16 12:09 ` Mattias Engdegård
@ 2022-07-17 3:13 ` Jim Porter
0 siblings, 0 replies; 6+ messages in thread
From: Jim Porter @ 2022-07-17 3:13 UTC (permalink / raw)
To: Mattias Engdegård, Lars Ingebrigtsen; +Cc: 56509
On 7/16/2022 5:09 AM, Mattias Engdegård wrote:
> The defcustom tests failed because the initial value of `eshell-variable-aliases-list` no longer corresponded to its declared type, so the type has now been updated (ea9b442b82). Please check.
Thanks for catching that. It looks good to me now.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-07-17 3:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-12 2:47 bug#56509: 29.0.50; Eshell variable aliases don't fully support indexing Jim Porter
2022-07-12 3:06 ` bug#56509: [PATCH] " Jim Porter
2022-07-12 4:11 ` Jim Porter
2022-07-12 13:11 ` bug#56509: " Lars Ingebrigtsen
2022-07-16 12:09 ` Mattias Engdegård
2022-07-17 3:13 ` 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).