* bug#55236: 29.0.50; Surprising behaviors with Eshell expansions
@ 2022-05-03 3:41 Jim Porter
2022-05-03 3:45 ` bug#55236: [PATCH] " Jim Porter
2022-05-03 9:32 ` Michael Albinus
0 siblings, 2 replies; 5+ messages in thread
From: Jim Porter @ 2022-05-03 3:41 UTC (permalink / raw)
To: 55236
(Note: this is closely related to bug#12689, but this isn't *quite* a
fix for that bug. This bug is also somewhat broader in scope, so it felt
like a new bug was the best place for it. I'll comment on that bug
shortly with further details.)
There are several inconsistencies with how Eshell expansions are, well,
expanded. These are arguably separate bugs, but they're closely-enough
related that I think it's best to discuss them all at once so that
everyone can see the full context (especially since any changes here
could conceivably cause incompatibilities).
1. Quoted Expansions
--------------------
From "emacs -Q --eval '(eshell)'":
~ $ type-of 1
integer
~ $ type-of "1"
string
~ $ setq foo 1
1
~ $ type-of $foo
integer
~ $ type-of "$foo"
integer
Surprisingly, the last line shows that "$foo" is an integer, even though
double-quotes are used in Eshell to disable conversion to numbers. For
another example, first try this in your favorite shell (tested in dash,
bash, and zsh):
$ cat args
#!/usr/bin/env python3
import sys
print(sys.argv[1:])
$ ./args $(/bin/echo -e "foo\nbar")
['foo', 'bar']
$ ./args "$(/bin/echo -e "foo\nbar")"
['foo\nbar']
Now in "emacs -Q --eval '(eshell)'":
~ $ ./args ${/bin/echo -e "foo\nbar"}
['foo', 'bar']
~ $ ./args "${/bin/echo -e \"foo\nbar\"}"
['foo', 'bar']
Again, wrapping the Eshell expansion in double-quotes doesn't do
anything. It should probably work like other shells.
2. Converting subcommand output to numbers
------------------------------------------
From "emacs -Q --eval '(eshell)'":
~ $ type-of ${/bin/echo "1"}
integer
~ $ echo ${/bin/echo -e "1\n2"}
("1" "2")
~ $ type-of ${/bin/echo -e "1\n2"}[0]
string
Eshell converts ${SUBCOMMAND} output to a number, but only if there's a
*single* number. If there are multiple lines with numbers, you just get
a list of strings. This is somewhat inconvenient, since you might want
to add the numbers up, e.g. by typing:
~ $ apply #'+ ${/bin/echo -e "1\n2"}
Wrong type argument: number-or-marker-p, "1"
(This might seem unrelated, but it ties directly into the next issue.)
3. Concatenating expansions
---------------------------
Like in other shells, you can concatenate expansions with strings or
other expansions. From "emacs -Q --eval '(eshell)'":
~ $ echo ${*echo "1"}2
12
~ $ type-of ${*echo "1"}2
integer
That's fine (at least once you know to expect it), but there are some
odd corner cases:
~ $ setq foo "1"
1
~ $ type-of $foo
string
~ $ type-of $'foo'2
integer
Here, concatenating two strings produces an integer, since the string
"12" looks numeric. I think that's an overly-aggressive conversion.
Similarly, when the expansion returns a list, the result is surprising
(this is one of the things bug#12689 is about):
~ $ ./args ${/bin/echo -e "foo\nbar"}-baz
['("foo" "bar")-baz']
This time, the result is the list of lines from /bin/echo, converted to
a string, and then concatenated with "-baz" into a single string. That's
surprising, since doing the same sort of thing in dash/bash/zsh produces
the following:
$ ./args $(/bin/echo -e "foo\nbar")-baz
['foo', 'bar-baz']
I'll post patches for these shortly (just getting a bug number).
^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#55236: [PATCH] 29.0.50; Surprising behaviors with Eshell expansions
2022-05-03 3:41 bug#55236: 29.0.50; Surprising behaviors with Eshell expansions Jim Porter
@ 2022-05-03 3:45 ` Jim Porter
2022-05-03 4:20 ` bug#55236: [PATCH v2] " Jim Porter
2022-05-03 9:32 ` Michael Albinus
1 sibling, 1 reply; 5+ messages in thread
From: Jim Porter @ 2022-05-03 3:45 UTC (permalink / raw)
To: 55236
[-- Attachment #1: Type: text/plain, Size: 755 bytes --]
Here are the patches. Each patch corresponds to one of the headings in
the original email. Hopefully they're fairly self-explanatory since I've
added documentation to the manual as well as NEWS entries for each. I
debated whether these should count as incompatible changes, but for most
cases, I think there shouldn't be any incompatibility.
The highest-risk patch is the second one, since it converts multi-line
numeric output from subcommands into a list of numbers. However, this
helps a lot in fixing some of the problems in the third issue
(concatenating expansions), and I think it's more consistent overall
than before. However, it could make sense to provide a defcustom to opt
out of the behavior if people think that would be helpful.
[-- Attachment #2: 0001-Eshell-variable-expansion-should-always-return-strin.patch --]
[-- Type: text/plain, Size: 26136 bytes --]
From d07087b072dd410040c7ddb1322d1a1f3cd547df Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Mon, 28 Feb 2022 17:38:39 -0800
Subject: [PATCH 1/3] Eshell variable expansion should always return strings
inside quotes
This is closer in behavior to regular shells, and gives Eshell users
greater flexibility in how variables are expanded.
* lisp/eshell/esh-util.el (eshell-convert): Add TO-STRING argument.
* lisp/eshell/esh-var.el (eshell-parse-variable-ref): Add MODIFIER-P
argument and adjust how 'eshell-convert' and 'eshell-apply-indices'
are called.
(eshell-get-variable, eshell-apply-indices): Add QUOTED argument.
* test/lisp/eshell/esh-var-tests.el (eshell-test-value): New defvar.
(esh-var-test/interp-convert-var-number)
(esh-var-test/interp-convert-var-split-indices)
(esh-var-test/interp-convert-quoted-var-number)
(esh-var-test/interp-convert-quoted-var-split-indices)
(esh-var-test/interp-convert-cmd-string-newline)
(esh-var-test/interp-convert-cmd-multiline)
(esh-var-test/interp-convert-cmd-number)
(esh-var-test/interp-convert-cmd-split-indices)
(esh-var-test/quoted-interp-convert-var-number)
(esh-var-test/quoted-interp-convert-var-split-indices)
(esh-var-test/quoted-interp-convert-quoted-var-number)
(esh-var-test/quoted-interp-convert-quoted-var-split-indices)
(esh-var-test/quoted-interp-convert-cmd-string-newline)
(esh-var-test/quoted-interp-convert-cmd-multiline)
(esh-var-test/quoted-interp-convert-cmd-number)
(esh-var-test/quoted-interp-convert-cmd-split-indices): New tests.
* doc/misc/eshell.texi (Arguments): Expand this section, and document
the new behavior.
(Dollars Expansion): Provide more detail about '$(lisp)' and
'${command}' forms.
* etc/NEWS (Eshell): Announce this change (bug#55236).
---
doc/misc/eshell.texi | 57 +++++++---
etc/NEWS | 6 ++
lisp/eshell/esh-util.el | 46 +++++---
lisp/eshell/esh-var.el | 63 +++++++----
test/lisp/eshell/esh-var-tests.el | 171 ++++++++++++++++++++++++++----
5 files changed, 272 insertions(+), 71 deletions(-)
diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index a3ed922cf2..164a71f309 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -228,15 +228,39 @@ Invocation
@node Arguments
@section Arguments
-Command arguments are passed to the functions as either strings or
-numbers, depending on what the parser thinks they look like. If you
-need to use a function that takes some other data type, you will need to
-call it in an Elisp expression (which can also be used with
-@ref{Expansion, expansions}). As with other shells, you can
-escape special characters and spaces with the backslash (@code{\}) and
-apostrophes (@code{''}) and double quotes (@code{""}). This is needed
-especially for file names with special characters like pipe
-(@code{|}), which could be part of remote file names.
+Ordinarily, command arguments are parsed by Eshell as either strings
+or numbers, depending on what the parser thinks they look like. To
+specify an argument of some other data type, you can use an
+@ref{Dollars Expansion, Elisp expression}:
+
+@example
+~ $ echo (list 1 2 3)
+(1 2 3)
+@end example
+
+Additionally, many @ref{Built-ins, Eshell commands} will flatten the
+arguments they receive, so passing a list as an argument will
+``spread'' the elements into multiple arguments:
+
+@example
+~ $ printnl (list 1 2) 3
+1
+2
+3
+@end example
+
+@subsection Quoting and escaping
+
+As with other shells, you can escape special characters and spaces
+with by prefixing the character with a backslash (@code{\}), or by
+surrounding the string with apostrophes (@code{''}) or double quotes
+(@code{""}). This is needed especially for file names with special
+characters like pipe (@code{|}), which could be part of remote file
+names.
+
+When using @ref{Expansion, expansions} in an Eshell command, the
+result may potentially be of any data type. To ensure that the result
+is always a string, the expansion can be surrounded by double quotes.
@node Built-ins
@section Built-in commands
@@ -1026,11 +1050,20 @@ Dollars Expansion
@item $(@var{lisp})
Expands to the result of evaluating the S-expression @code{(@var{lisp})}. On
its own, this is identical to just @code{(@var{lisp})}, but with the @code{$},
-it can be used in a string, such as @samp{/some/path/$(@var{lisp}).txt}.
+it can be used inside double quotes or within a longer string, such as
+@samp{/some/path/$(@var{lisp}).txt}.
@item $@{@var{command}@}
-Returns the output of @command{@var{command}}, which can be any valid Eshell
-command invocation, and may even contain expansions.
+Returns the output of @command{@var{command}}, which can be any valid
+Eshell command invocation, and may even contain expansions. Similar
+to @code{$(@var{lisp})}, this is identical to @code{@{@var{command}@}}
+when on its own, but the @code{$} allows it to be used inside double
+quotes or as part of a string.
+
+Normally, the output is split line-by-line, returning a list (or the
+first element if there's only one line of output). However, when this
+expansion is surrounded by double quotes, it returns the output as a
+single string instead.
@item $<@var{command}>
As with @samp{$@{@var{command}@}}, evaluates the Eshell command invocation
diff --git a/etc/NEWS b/etc/NEWS
index 882748d8c7..ae815bb785 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1370,6 +1370,12 @@ Lisp function. This frees you from having to keep track of whether
commands are Lisp function or external when supplying absolute file
name arguments. See "Electric forward slash" in the Eshell manual.
++++
+*** Double-quoting an Eshell expansion now treats the result as a single string.
+If an Eshell expansion like '$FOO' is surrounded by double quotes, the
+result will always be a single string, no matter the type that would
+otherwise be returned.
+
---
*** Built-in Eshell commands now follow POSIX/GNU argument syntax conventions.
Built-in commands in Eshell now accept command-line options with
diff --git a/lisp/eshell/esh-util.el b/lisp/eshell/esh-util.el
index 3da712c719..6c130974e9 100644
--- a/lisp/eshell/esh-util.el
+++ b/lisp/eshell/esh-util.el
@@ -198,23 +198,37 @@ eshell-find-delimiter
(when (= depth 0)
(if reverse-p (point) (1- (point)))))))
-(defun eshell-convert (string)
- "Convert STRING into a more native looking Lisp object."
- (if (not (stringp string))
- string
- (let ((len (length string)))
- (if (= len 0)
- string
- (if (eq (aref string (1- len)) ?\n)
+(defun eshell-convert (string &optional to-string)
+ "Convert STRING into a more-native Lisp object.
+If TO-STRING is non-nil, always return a single string with
+trailing newlines removed. Otherwise, this behaves as follows:
+
+* Return non-strings as-is.
+
+* Split multiline strings by line.
+
+* If `eshell-convert-numeric-aguments' is non-nil, convert
+ numeric strings to numbers."
+ (cond
+ ((not (stringp string))
+ (if to-string
+ (eshell-stringify string)
+ string))
+ (to-string (string-trim-right string "\n+"))
+ (t (let ((len (length string)))
+ (if (= len 0)
+ string
+ (when (eq (aref string (1- len)) ?\n)
(setq string (substring string 0 (1- len))))
- (if (string-search "\n" string)
- (split-string string "\n")
- (if (and eshell-convert-numeric-arguments
- (string-match
- (concat "\\`\\s-*" eshell-number-regexp
- "\\s-*\\'") string))
- (string-to-number string)
- string))))))
+ (cond
+ ((string-search "\n" string)
+ (split-string string "\n"))
+ ((and eshell-convert-numeric-arguments
+ (string-match
+ (concat "\\`\\s-*" eshell-number-regexp "\\s-*\\'")
+ string))
+ (string-to-number string))
+ (t string)))))))
(defvar-local eshell-path-env (getenv "PATH")
"Content of $PATH.
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 3c6bcc753c..1c28d24af1 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -402,23 +402,30 @@ eshell-parse-variable
(let* ((get-len (when (eq (char-after) ?#)
(forward-char) t))
value indices)
- (setq value (eshell-parse-variable-ref)
+ (setq value (eshell-parse-variable-ref get-len)
indices (and (not (eobp))
(eq (char-after) ?\[)
(eshell-parse-indices))
;; This is an expression that will be evaluated by `eshell-do-eval',
;; which only support let-binding of dynamically-scoped vars
value `(let ((indices (eshell-eval-indices ',indices))) ,value))
- (if get-len
- `(length ,value)
- value)))
+ (when get-len
+ (setq value `(length ,value)))
+ (when eshell-current-quoted
+ (setq value `(eshell-stringify ,value)))
+ value))
-(defun eshell-parse-variable-ref ()
+(defun eshell-parse-variable-ref (&optional modifier-p)
"Eval a variable reference.
Returns a Lisp form which, if evaluated, will return the value of the
variable.
-Possible options are:
+If MODIFIER-P is non-nil, the value of the variable will be
+modified by some function. If MODIFIER-P is nil, the value will be
+used as-is; this allows optimization of some kinds of variable
+references.
+
+Possible variable references are:
NAME an environment or Lisp variable value
\"LONG-NAME\" disambiguates the length of the name
@@ -441,8 +448,16 @@ eshell-parse-variable-ref
,(let ((subcmd (or (eshell-unescape-inner-double-quote end)
(cons (point) end)))
(eshell-current-quoted nil))
- (eshell-parse-command subcmd)))))
- indices)
+ (eshell-parse-command subcmd))))
+ ;; If this is a simple double-quoted form like
+ ;; "${COMMAND}" (i.e. no indices after the subcommand
+ ;; and no `#' modifier before), ensure we convert to a
+ ;; single string. This avoids unnecessary work
+ ;; (e.g. splitting the output by lines) when it would
+ ;; just be joined back together afterwards.
+ ,(when (and (not modifier-p) eshell-current-quoted)
+ '(not indices)))
+ indices ,eshell-current-quoted)
(goto-char (1+ end))))))
((eq (char-after) ?\<)
(let ((end (eshell-find-delimiter ?\< ?\>)))
@@ -466,7 +481,7 @@ eshell-parse-variable-ref
;; properly. See bug#54190.
(list (function (lambda ()
(delete-file ,temp))))))
- (eshell-apply-indices ,temp indices)))
+ (eshell-apply-indices ,temp indices ,eshell-current-quoted)))
(goto-char (1+ end)))))))
((eq (char-after) ?\()
(condition-case nil
@@ -475,7 +490,7 @@ eshell-parse-variable-ref
(eshell-lisp-command
',(read (or (eshell-unescape-inner-double-quote (point-max))
(current-buffer)))))
- indices)
+ indices ,eshell-current-quoted)
(end-of-file
(throw 'eshell-incomplete ?\())))
((looking-at (rx-to-string
@@ -487,14 +502,15 @@ eshell-parse-variable-ref
(eshell-parse-literal-quote)
(eshell-parse-double-quote))))
(when name
- `(eshell-get-variable ,(eval name) indices)))))
+ `(eshell-get-variable ,(eval name) indices ,eshell-current-quoted)))))
((assoc (char-to-string (char-after))
eshell-variable-aliases-list)
(forward-char)
- `(eshell-get-variable ,(char-to-string (char-before)) indices))
+ `(eshell-get-variable ,(char-to-string (char-before)) indices
+ ,eshell-current-quoted))
((looking-at eshell-variable-name-regexp)
(prog1
- `(eshell-get-variable ,(match-string 0) indices)
+ `(eshell-get-variable ,(match-string 0) indices ,eshell-current-quoted)
(goto-char (match-end 0))))
(t
(error "Invalid variable reference"))))
@@ -525,8 +541,10 @@ eshell-eval-indices
"Evaluate INDICES, a list of index-lists generated by `eshell-parse-indices'."
(mapcar (lambda (i) (mapcar #'eval i)) indices))
-(defun eshell-get-variable (name &optional indices)
- "Get the value for the variable NAME."
+(defun eshell-get-variable (name &optional indices quoted)
+ "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)
@@ -547,9 +565,9 @@ eshell-get-variable
(symbol-value var))
(t
(error "Unknown variable `%s'" (eshell-stringify var))))
- indices))))
+ indices quoted))))
-(defun eshell-apply-indices (value indices)
+(defun eshell-apply-indices (value indices &optional quoted)
"Apply to VALUE all of the given INDICES, returning the sub-result.
The format of INDICES is:
@@ -558,12 +576,17 @@ eshell-apply-indices
Each member of INDICES represents a level of nesting. If the first
member of a sublist is not an integer or name, and the value it's
-reference is a string, that will be used as the regexp with which is
-to divide the string into sub-parts. The default is whitespace.
+referencing is a string, that will be used as the regexp with which
+is to divide the string into sub-parts. The default is whitespace.
Otherwise, each INT-OR-NAME refers to an element of the list value.
Integers imply a direct index, and names, an associate lookup using
`assoc'.
+If QUOTED is non-nil, this was invoked inside double-quotes. This
+affects the behavior of splitting strings: without quoting, the
+split values are converted to Lisp forms via `eshell-convert'; with
+quoting, they're left as strings.
+
For example, to retrieve the second element of a user's record in
'/etc/passwd', the variable reference would look like:
@@ -577,7 +600,7 @@ eshell-apply-indices
(setq separator index
refs (cdr refs)))
(setq value
- (mapcar #'eshell-convert
+ (mapcar (lambda (i) (eshell-convert i quoted))
(split-string value separator)))))
(cond
((< (length refs) 0)
diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el
index 1d051d681a..5363a86e71 100644
--- a/test/lisp/eshell/esh-var-tests.el
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -210,12 +210,17 @@ esh-var-test/quoted-interp-var-indices
(should (equal (eshell-test-command-result
"echo \"$eshell-test-value[0]\"")
"zero"))
+ ;; FIXME: These tests would use the 0th index like the other tests
+ ;; here, but evaluating the command just above adds an `escaped'
+ ;; property to the string "zero". This results in the output
+ ;; printing the string properties, which is probably the wrong
+ ;; behavior. See bug#54486.
(should (equal (eshell-test-command-result
- "echo \"$eshell-test-value[0 2]\"")
- '("zero" "two")))
+ "echo \"$eshell-test-value[1 2]\"")
+ "(\"one\" \"two\")"))
(should (equal (eshell-test-command-result
- "echo \"$eshell-test-value[0 2 4]\"")
- '("zero" "two" "four")))))
+ "echo \"$eshell-test-value[1 2 4]\"")
+ "(\"one\" \"two\" \"four\")"))))
(ert-deftest esh-var-test/quoted-interp-var-split-indices ()
"Interpolate string variable with indices inside double-quotes"
@@ -225,7 +230,7 @@ esh-var-test/quoted-interp-var-split-indices
"zero"))
(should (equal (eshell-test-command-result
"echo \"$eshell-test-value[0 2]\"")
- '("zero" "two")))))
+ "(\"zero\" \"two\")"))))
(ert-deftest esh-var-test/quoted-interp-var-string-split-indices ()
"Interpolate string variable with string splitter and indices
@@ -236,14 +241,14 @@ esh-var-test/quoted-interp-var-string-split-indices
"zero"))
(should (equal (eshell-test-command-result
"echo \"$eshell-test-value[: 0 2]\"")
- '("zero" "two"))))
+ "(\"zero\" \"two\")")))
(let ((eshell-test-value "zeroXoneXtwoXthreeXfour"))
(should (equal (eshell-test-command-result
"echo \"$eshell-test-value[X 0]\"")
"zero"))
(should (equal (eshell-test-command-result
"echo \"$eshell-test-value[X 0 2]\"")
- '("zero" "two")))))
+ "(\"zero\" \"two\")"))))
(ert-deftest esh-var-test/quoted-interp-var-regexp-split-indices ()
"Interpolate string variable with regexp splitter and indices"
@@ -253,43 +258,47 @@ esh-var-test/quoted-interp-var-regexp-split-indices
"zero"))
(should (equal (eshell-test-command-result
"echo \"$eshell-test-value['[:!]' 0 2]\"")
- '("zero" "two")))
+ "(\"zero\" \"two\")"))
(should (equal (eshell-test-command-result
"echo \"$eshell-test-value[\\\"[:!]\\\" 0]\"")
"zero"))
(should (equal (eshell-test-command-result
"echo \"$eshell-test-value[\\\"[:!]\\\" 0 2]\"")
- '("zero" "two")))))
+ "(\"zero\" \"two\")"))))
(ert-deftest esh-var-test/quoted-interp-var-assoc ()
"Interpolate alist variable with index inside double-quotes"
(let ((eshell-test-value '(("foo" . 1))))
(should (equal (eshell-test-command-result
"echo \"$eshell-test-value[foo]\"")
- 1))))
+ "1"))))
(ert-deftest esh-var-test/quoted-interp-var-length-list ()
"Interpolate length of list variable inside double-quotes"
(let ((eshell-test-value '((1 2) (3) (5 (6 7 8 9)))))
- (should (eq (eshell-test-command-result "echo \"$#eshell-test-value\"") 3))
- (should (eq (eshell-test-command-result "echo \"$#eshell-test-value[1]\"")
- 1))
- (should (eq (eshell-test-command-result
- "echo \"$#eshell-test-value[2][1]\"")
- 4))))
+ (should (equal (eshell-test-command-result "echo \"$#eshell-test-value\"")
+ "3"))
+ (should (equal (eshell-test-command-result
+ "echo \"$#eshell-test-value[1]\"")
+ "1"))
+ (should (equal (eshell-test-command-result
+ "echo \"$#eshell-test-value[2][1]\"")
+ "4"))))
(ert-deftest esh-var-test/quoted-interp-var-length-string ()
"Interpolate length of string variable inside double-quotes"
(let ((eshell-test-value "foobar"))
- (should (eq (eshell-test-command-result "echo \"$#eshell-test-value\"")
- 6))))
+ (should (equal (eshell-test-command-result "echo \"$#eshell-test-value\"")
+ "6"))))
(ert-deftest esh-var-test/quoted-interp-var-length-alist ()
"Interpolate length of alist variable inside double-quotes"
(let ((eshell-test-value '(("foo" . (1 2 3)))))
- (should (eq (eshell-test-command-result "echo \"$#eshell-test-value\"") 1))
- (should (eq (eshell-test-command-result "echo \"$#eshell-test-value[foo]\"")
- 3))))
+ (should (equal (eshell-test-command-result "echo \"$#eshell-test-value\"")
+ "1"))
+ (should (equal (eshell-test-command-result
+ "echo \"$#eshell-test-value[foo]\"")
+ "3"))))
(ert-deftest esh-var-test/quoted-interp-lisp ()
"Interpolate Lisp form evaluation inside double-quotes"
@@ -299,7 +308,8 @@ esh-var-test/quoted-interp-lisp
(ert-deftest esh-var-test/quoted-interp-lisp-indices ()
"Interpolate Lisp form evaluation with index"
- (should (equal (eshell-test-command-result "+ \"$(list 1 2)[1]\" 3") 5)))
+ (should (equal (eshell-test-command-result "concat \"$(list 1 2)[1]\" cool")
+ "2cool")))
(ert-deftest esh-var-test/quoted-interp-cmd ()
"Interpolate command result inside double-quotes"
@@ -309,12 +319,127 @@ esh-var-test/quoted-interp-cmd
(ert-deftest esh-var-test/quoted-interp-cmd-indices ()
"Interpolate command result with index inside double-quotes"
- (should (equal (eshell-test-command-result "+ \"${list 1 2}[1]\" 3") 5)))
+ (should (equal (eshell-test-command-result "concat \"${list 1 2}[1]\" cool")
+ "2cool")))
(ert-deftest esh-var-test/quoted-interp-temp-cmd ()
"Interpolate command result redirected to temp file inside double-quotes"
(should (equal (eshell-test-command-result "cat \"$<echo hi>\"") "hi")))
+\f
+;; Interpolated variable conversion
+
+(ert-deftest esh-var-test/interp-convert-var-number ()
+ "Interpolate numeric variable"
+ (let ((eshell-test-value 123))
+ (should (equal (eshell-test-command-result "type-of $eshell-test-value")
+ 'integer))))
+
+(ert-deftest esh-var-test/interp-convert-var-split-indices ()
+ "Interpolate and convert string variable with indices"
+ (let ((eshell-test-value "000 010 020 030 040"))
+ (should (equal (eshell-test-command-result "echo $eshell-test-value[0]")
+ 0))
+ (should (equal (eshell-test-command-result "echo $eshell-test-value[0 2]")
+ '(0 20)))))
+
+(ert-deftest esh-var-test/interp-convert-quoted-var-number ()
+ "Interpolate numeric quoted numeric variable"
+ (let ((eshell-test-value 123))
+ (should (equal (eshell-test-command-result "type-of $'eshell-test-value'")
+ 'integer))
+ (should (equal (eshell-test-command-result "type-of $\"eshell-test-value\"")
+ 'integer))))
+
+(ert-deftest esh-var-test/interp-convert-quoted-var-split-indices ()
+ "Interpolate and convert quoted string variable with indices"
+ (let ((eshell-test-value "000 010 020 030 040"))
+ (should (equal (eshell-test-command-result "echo $'eshell-test-value'[0]")
+ 0))
+ (should (equal (eshell-test-command-result "echo $'eshell-test-value'[0 2]")
+ '(0 20)))))
+
+(ert-deftest esh-var-test/interp-convert-cmd-string-newline ()
+ "Interpolate trailing-newline command result"
+ (should (equal (eshell-test-command-result "echo ${echo \"foo\n\"}") "foo")))
+
+(ert-deftest esh-var-test/interp-convert-cmd-multiline ()
+ "Interpolate multi-line command result"
+ (should (equal (eshell-test-command-result "echo ${echo \"foo\nbar\"}")
+ '("foo" "bar"))))
+
+(ert-deftest esh-var-test/interp-convert-cmd-number ()
+ "Interpolate numeric command result"
+ (should (equal (eshell-test-command-result "echo ${echo \"1\"}") 1)))
+
+(ert-deftest esh-var-test/interp-convert-cmd-split-indices ()
+ "Interpolate command result with indices"
+ (should (equal (eshell-test-command-result "echo ${echo \"000 010 020\"}[0]")
+ 0))
+ (should (equal (eshell-test-command-result
+ "echo ${echo \"000 010 020\"}[0 2]")
+ '(0 20))))
+
+(ert-deftest esh-var-test/quoted-interp-convert-var-number ()
+ "Interpolate numeric variable inside double-quotes"
+ (let ((eshell-test-value 123))
+ (should (equal (eshell-test-command-result "type-of \"$eshell-test-value\"")
+ 'string))))
+
+(ert-deftest esh-var-test/quoted-interp-convert-var-split-indices ()
+ "Interpolate string variable with indices inside double-quotes"
+ (let ((eshell-test-value "000 010 020 030 040"))
+ (should (equal (eshell-test-command-result
+ "echo \"$eshell-test-value[0]\"")
+ "000"))
+ (should (equal (eshell-test-command-result
+ "echo \"$eshell-test-value[0 2]\"")
+ "(\"000\" \"020\")"))))
+
+(ert-deftest esh-var-test/quoted-interp-convert-quoted-var-number ()
+ "Interpolate numeric quoted variable inside double-quotes"
+ (let ((eshell-test-value 123))
+ (should (equal (eshell-test-command-result
+ "type-of \"$'eshell-test-value'\"")
+ 'string))
+ (should (equal (eshell-test-command-result
+ "type-of \"$\\\"eshell-test-value\\\"\"")
+ 'string))))
+
+(ert-deftest esh-var-test/quoted-interp-convert-quoted-var-split-indices ()
+ "Interpolate quoted string variable with indices inside double-quotes"
+ (let ((eshell-test-value "000 010 020 030 040"))
+ (should (equal (eshell-test-command-result
+ "echo \"$eshell-test-value[0]\"")
+ "000"))
+ (should (equal (eshell-test-command-result
+ "echo \"$eshell-test-value[0 2]\"")
+ "(\"000\" \"020\")"))))
+
+(ert-deftest esh-var-test/quoted-interp-convert-cmd-string-newline ()
+ "Interpolate trailing-newline command result inside double-quotes"
+ (should (equal (eshell-test-command-result "echo \"${echo \\\"foo\n\\\"}\"")
+ "foo"))
+ (should (equal (eshell-test-command-result "echo \"${echo \\\"foo\n\n\\\"}\"")
+ "foo")))
+
+(ert-deftest esh-var-test/quoted-interp-convert-cmd-multiline ()
+ "Interpolate multi-line command result inside double-quotes"
+ (should (equal (eshell-test-command-result
+ "echo \"${echo \\\"foo\nbar\\\"}\"")
+ "foo\nbar")))
+
+(ert-deftest esh-var-test/quoted-interp-convert-cmd-number ()
+ "Interpolate numeric command result inside double-quotes"
+ (should (equal (eshell-test-command-result "echo \"${echo \\\"1\\\"}\"")
+ "1")))
+
+(ert-deftest esh-var-test/quoted-interp-convert-cmd-split-indices ()
+ "Interpolate command result with indices inside double-quotes"
+ (should (equal (eshell-test-command-result
+ "echo \"${echo \\\"000 010 020\\\"}[0]\"")
+ "000")))
+
\f
;; Built-in variables
--
2.25.1
[-- Attachment #3: 0002-Return-a-list-of-numbers-if-all-lines-of-an-Eshell-s.patch --]
[-- Type: text/plain, Size: 5742 bytes --]
From d675817d056100bfa909d7ea88cb89f952474992 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 1 May 2022 22:09:17 -0700
Subject: [PATCH 2/3] Return a list of numbers if all lines of an Eshell
subcommand are numeric
* lisp/eshell/esh-util.el (eshell-convertible-to-number-p)
(eshell-convert-to-number): New functions...
(eshell-convert): ... use them.
* test/lisp/eshell/esh-var-tests.el
(esh-var-test/interp-convert-cmd-string-newline): Add checks for
numeric output.
* doc/misc/eshell.texi (Dollars Expansion): Document the new behavior.
* etc/NEWS: Announce the change (bug#55236).
---
doc/misc/eshell.texi | 8 ++++---
etc/NEWS | 7 ++++++
lisp/eshell/esh-util.el | 36 +++++++++++++++++++++----------
test/lisp/eshell/esh-var-tests.el | 8 ++++++-
4 files changed, 44 insertions(+), 15 deletions(-)
diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 164a71f309..53f7fdd9ea 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1061,9 +1061,11 @@ Dollars Expansion
quotes or as part of a string.
Normally, the output is split line-by-line, returning a list (or the
-first element if there's only one line of output). However, when this
-expansion is surrounded by double quotes, it returns the output as a
-single string instead.
+first element if there's only one line of output); if
+@code{eshell-convert-numeric-agument} is non-@code{nil} and every line
+of output looks like a number, convert each line to a number.
+However, when this expansion is surrounded by double quotes, it
+returns the output as a single string instead.
@item $<@var{command}>
As with @samp{$@{@var{command}@}}, evaluates the Eshell command invocation
diff --git a/etc/NEWS b/etc/NEWS
index ae815bb785..0c77b0bf58 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1376,6 +1376,13 @@ If an Eshell expansion like '$FOO' is surrounded by double quotes, the
result will always be a single string, no matter the type that would
otherwise be returned.
++++
+*** Eshell subcommands with multiline numeric output return lists of numbers.
+If every line of the output of an Eshell subcommand like '${COMMAND}'
+is numeric, the result will be a list of numbers (or a single number
+if only one line of output). Previously, this only converted numbers
+when there was a single line of output.
+
---
*** Built-in Eshell commands now follow POSIX/GNU argument syntax conventions.
Built-in commands in Eshell now accept command-line options with
diff --git a/lisp/eshell/esh-util.el b/lisp/eshell/esh-util.el
index 6c130974e9..9960912bce 100644
--- a/lisp/eshell/esh-util.el
+++ b/lisp/eshell/esh-util.el
@@ -198,6 +198,23 @@ eshell-find-delimiter
(when (= depth 0)
(if reverse-p (point) (1- (point)))))))
+(defun eshell-convertible-to-number-p (string)
+ "Return non-nil if STRING can be converted to a number.
+If `eshell-convert-numeric-aguments', always return nil."
+ (and eshell-convert-numeric-arguments
+ (string-match
+ (concat "\\`\\s-*" eshell-number-regexp "\\s-*\\'")
+ string)))
+
+(defun eshell-convert-to-number (string)
+ "Try to convert STRING to a number.
+If STRING doesn't look like a number (or
+`eshell-convert-numeric-aguments' is nil), just return STRING
+unchanged."
+ (if (eshell-convertible-to-number-p string)
+ (string-to-number string)
+ string))
+
(defun eshell-convert (string &optional to-string)
"Convert STRING into a more-native Lisp object.
If TO-STRING is non-nil, always return a single string with
@@ -207,8 +224,8 @@ eshell-convert
* Split multiline strings by line.
-* If `eshell-convert-numeric-aguments' is non-nil, convert
- numeric strings to numbers."
+* If `eshell-convert-numeric-aguments' is non-nil and every line
+ of output looks like a number, convert them to numbers."
(cond
((not (stringp string))
(if to-string
@@ -220,15 +237,12 @@ eshell-convert
string
(when (eq (aref string (1- len)) ?\n)
(setq string (substring string 0 (1- len))))
- (cond
- ((string-search "\n" string)
- (split-string string "\n"))
- ((and eshell-convert-numeric-arguments
- (string-match
- (concat "\\`\\s-*" eshell-number-regexp "\\s-*\\'")
- string))
- (string-to-number string))
- (t string)))))))
+ (if (string-search "\n" string)
+ (let ((lines (split-string string "\n")))
+ (if (seq-every-p #'eshell-convertible-to-number-p lines)
+ (mapcar #'string-to-number lines)
+ lines))
+ (eshell-convert-to-number string)))))))
(defvar-local eshell-path-env (getenv "PATH")
"Content of $PATH.
diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el
index 5363a86e71..2ce6bb4f1b 100644
--- a/test/lisp/eshell/esh-var-tests.el
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -366,7 +366,13 @@ esh-var-test/interp-convert-cmd-string-newline
(ert-deftest esh-var-test/interp-convert-cmd-multiline ()
"Interpolate multi-line command result"
(should (equal (eshell-test-command-result "echo ${echo \"foo\nbar\"}")
- '("foo" "bar"))))
+ '("foo" "bar")))
+ ;; Numeric output should be converted to numbers...
+ (should (equal (eshell-test-command-result "echo ${echo \"01\n02\n03\"}")
+ '(1 2 3)))
+ ;; ... but only if every line is numeric.
+ (should (equal (eshell-test-command-result "echo ${echo \"01\n02\nhi\"}")
+ '("01" "02" "hi"))))
(ert-deftest esh-var-test/interp-convert-cmd-number ()
"Interpolate numeric command result"
--
2.25.1
[-- Attachment #4: 0003-Improve-the-behavior-of-concatenating-parts-of-Eshel.patch --]
[-- Type: text/plain, Size: 9764 bytes --]
From d8b211c81df0d89b072049d79671b4c0584c67ab Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Mon, 2 May 2022 16:56:49 -0700
Subject: [PATCH 3/3] Improve the behavior of concatenating parts of Eshell
arguments
Previously, concatenating a list to a string would first convert the
list to a string. Now, the string is concatenated with the last
element of the list.
* lisp/eshell/esh-util.el (eshell-to-flat-string): Make obsolete.
* lisp/eshell/esh-arg.el (eshell-concat, eshell-concat-1): New
functions.
(eshell-resolve-current-argument): Use 'eshell-concat'.
* test/lisp/eshell/esh-var-tests.el (esh-var-test/interp-concat-cmd):
Add check for concatenation of multiline output of subcommands.
(esh-var-test/quoted-interp-concat-cmd): New test.
* test/lisp/eshell/em-extpipe-tests.el (em-extpipe-test-13): Use
'eshell-concat'.
* doc/misc/eshell.texi (Expansion): Document this behavior.
* etc/NEWS: Announce the change (bug#55236).
---
doc/misc/eshell.texi | 34 +++++++++++++--
etc/NEWS | 7 ++++
lisp/eshell/esh-arg.el | 62 ++++++++++++++++++++++++----
lisp/eshell/esh-util.el | 1 +
test/lisp/eshell/em-extpipe-tests.el | 2 +-
test/lisp/eshell/esh-var-tests.el | 19 ++++++++-
6 files changed, 109 insertions(+), 16 deletions(-)
diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 53f7fdd9ea..684d1553f1 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1017,11 +1017,37 @@ Expansion
shell, they are less often used for constants, and usually for using
variables and string manipulation.@footnote{Eshell has no
string-manipulation expansions because the Elisp library already
-provides many functions for this.} For example, @code{$var} on a line
-expands to the value of the variable @code{var} when the line is
+provides many functions for this.} For example, @code{$@var{var}} on
+a line expands to the value of the variable @var{var} when the line is
executed. Expansions are usually passed as arguments, but may also be
-used as commands.@footnote{E.g., entering just @samp{$var} at the prompt
-is equivalent to entering the value of @code{var} at the prompt.}
+used as commands.@footnote{E.g., entering just @samp{$@var{var}} at
+the prompt is equivalent to entering the value of @var{var} at the
+prompt.}
+
+You can concatenate expansions with regular string arguments or even
+other expansions. In the simplest case, when the expansion returns a
+string value, this is equivalent to ordinary string concatenation; for
+example, @samp{$@{echo "foo"@}bar} returns @samp{foobar}. The exact
+behavior depends on the types of each value being concatenated:
+
+@table @asis
+
+@item both strings
+Concatenate both values together.
+
+@item one or both numbers
+Concatenate the string representation of each value, converting back to
+a number if possible.
+
+@item one or both (non-@code{nil}) lists
+Concatenate ``adjacent'' elements of each value (possibly converting
+back to a number as above). For example, @samp{$list("a" "b")c}
+returns @samp{("a" "bc")}.
+
+@item anything else
+Concatenate the string represenation of each value.
+
+@end table
@menu
* Dollars Expansion::
diff --git a/etc/NEWS b/etc/NEWS
index 0c77b0bf58..360540b02a 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1376,6 +1376,13 @@ If an Eshell expansion like '$FOO' is surrounded by double quotes, the
result will always be a single string, no matter the type that would
otherwise be returned.
++++
+*** Concatenating Eshell expansions now works more similarly to other shells.
+When concatenating an Eshell expansion that returns a list, "adjacent"
+elements of each operand are now concatenated together,
+e.g. '$list("a" "b")c' returns '("a" "bc")'. See the "(eshell)
+Expansion" node in the Eshell manual for more details.
+
+++
*** Eshell subcommands with multiline numeric output return lists of numbers.
If every line of the output of an Eshell subcommand like '${COMMAND}'
diff --git a/lisp/eshell/esh-arg.el b/lisp/eshell/esh-arg.el
index 395aa87ff0..459487f435 100644
--- a/lisp/eshell/esh-arg.el
+++ b/lisp/eshell/esh-arg.el
@@ -180,19 +180,63 @@ eshell-escape-arg
(add-text-properties 0 (length string) '(escaped t) string))
string)
+(defun eshell-concat (quoted &rest rest)
+ "Concatenate all the arguments in REST and return the result.
+If QUOTED is nil, the resulting value(s) may be converted to
+numbers (see `eshell-concat-1').
+
+If each argument in REST is a non-list value, the result will be
+a single value, as if (mapconcat #'eshell-stringify REST) had been
+called, possibly converted to a number.
+
+If there is at least one (non-nil) list argument, the result will
+be a list, with \"adjacent\" elements of consecutive arguments
+concatenated as strings (again, possibly converted to numbers).
+For example, concatenating \"a\", (\"b\"), and (\"c\" \"d\")
+would produce (\"abc\" \"d\")."
+ (let (result)
+ (dolist (i rest result)
+ (when i
+ (cond
+ ((null result)
+ (setq result i))
+ ((listp result)
+ (let (curr-head curr-tail)
+ (if (listp i)
+ (setq curr-head (car i)
+ curr-tail (cdr i))
+ (setq curr-head i
+ curr-tail nil))
+ (setq result
+ (append
+ (butlast result 1)
+ (list (eshell-concat-1 quoted (car (last result))
+ curr-head))
+ curr-tail))))
+ ((listp i)
+ (setq result
+ (cons (eshell-concat-1 quoted result (car i))
+ (cdr i))))
+ (t
+ (setq result (eshell-concat-1 quoted result i))))))))
+
+(defun eshell-concat-1 (quoted first second)
+ "Concatenate FIRST and SECOND.
+If QUOTED is nil and either FIRST or SECOND are numbers, try to
+convert the result to a number as well."
+ (let ((result (concat (eshell-stringify first) (eshell-stringify second))))
+ (if (and (not quoted)
+ (or (numberp first) (numberp second)))
+ (eshell-convert-to-number result)
+ result)))
+
(defun eshell-resolve-current-argument ()
"If there are pending modifications to be made, make them now."
(when eshell-current-argument
(when eshell-arg-listified
- (let ((parts eshell-current-argument))
- (while parts
- (unless (stringp (car parts))
- (setcar parts
- (list 'eshell-to-flat-string (car parts))))
- (setq parts (cdr parts)))
- (setq eshell-current-argument
- (list 'eshell-convert
- (append (list 'concat) eshell-current-argument))))
+ (setq eshell-current-argument
+ (append (list 'eshell-concat eshell-current-quoted)
+ eshell-current-argument))
(setq eshell-arg-listified nil))
(while eshell-current-modifiers
(setq eshell-current-argument
diff --git a/lisp/eshell/esh-util.el b/lisp/eshell/esh-util.el
index 9960912bce..b5a423f023 100644
--- a/lisp/eshell/esh-util.el
+++ b/lisp/eshell/esh-util.el
@@ -293,6 +293,7 @@ eshell-split-path
(defun eshell-to-flat-string (value)
"Make value a string. If separated by newlines change them to spaces."
+ (declare (obsolete nil "29.1"))
(let ((text (eshell-stringify value)))
(if (string-match "\n+\\'" text)
(setq text (replace-match "" t t text)))
diff --git a/test/lisp/eshell/em-extpipe-tests.el b/test/lisp/eshell/em-extpipe-tests.el
index 91c2fba479..3b84d763ac 100644
--- a/test/lisp/eshell/em-extpipe-tests.el
+++ b/test/lisp/eshell/em-extpipe-tests.el
@@ -170,7 +170,7 @@ em-extpipe-test-12
(em-extpipe-tests--deftest em-extpipe-test-13 "foo*|bar"
(should-parse '(eshell-execute-pipeline
- '((eshell-named-command (concat "foo" "*"))
+ '((eshell-named-command (eshell-concat nil "foo" "*"))
(eshell-named-command "bar")))))
(em-extpipe-tests--deftest em-extpipe-test-14 "tac *<temp"
diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el
index 2ce6bb4f1b..3f3b591c5a 100644
--- a/test/lisp/eshell/esh-var-tests.el
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -176,8 +176,17 @@ esh-var-test/interp-concat-lisp2
(should (equal (eshell-test-command-result "+ $(+ 1 2)$(+ 1 2) 3") 36)))
(ert-deftest esh-var-test/interp-concat-cmd ()
- "Interpolate and concat command"
- (should (equal (eshell-test-command-result "+ ${+ 1 2}3 3") 36)))
+ "Interpolate and concat command with literal"
+ (should (equal (eshell-test-command-result "+ ${+ 1 2}3 3") 36))
+ (should (equal (eshell-test-command-result "echo ${*echo \"foo\nbar\"}-baz")
+ '("foo" "bar-baz")))
+ ;; Concatenating to a number in a list should produce a number...
+ (should (equal (eshell-test-command-result "echo ${*echo \"1\n2\"}3")
+ '(1 23)))
+ ;; ... but concatenating to a string that looks like a number in a list
+ ;; should produce a string.
+ (should (equal (eshell-test-command-result "echo ${*echo \"hi\n2\"}3")
+ '("hi" "23"))))
(ert-deftest esh-var-test/interp-concat-cmd2 ()
"Interpolate and concat two commands"
@@ -326,6 +335,12 @@ esh-var-test/quoted-interp-temp-cmd
"Interpolate command result redirected to temp file inside double-quotes"
(should (equal (eshell-test-command-result "cat \"$<echo hi>\"") "hi")))
+(ert-deftest esh-var-test/quoted-interp-concat-cmd ()
+ "Interpolate and concat command with literal"
+ (should (equal (eshell-test-command-result
+ "echo \"${echo \\\"foo\nbar\\\"} baz\"")
+ "foo\nbar baz")))
+
\f
;; Interpolated variable conversion
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* bug#55236: [PATCH v2] 29.0.50; Surprising behaviors with Eshell expansions
2022-05-03 3:45 ` bug#55236: [PATCH] " Jim Porter
@ 2022-05-03 4:20 ` Jim Porter
2022-05-03 16:24 ` bug#55236: " Lars Ingebrigtsen
0 siblings, 1 reply; 5+ messages in thread
From: Jim Porter @ 2022-05-03 4:20 UTC (permalink / raw)
To: 55236
[-- Attachment #1: Type: text/plain, Size: 970 bytes --]
On 5/2/2022 8:45 PM, Jim Porter wrote:
> Here are the patches. Each patch corresponds to one of the headings in
> the original email. Hopefully they're fairly self-explanatory since I've
> added documentation to the manual as well as NEWS entries for each. I
> debated whether these should count as incompatible changes, but for most
> cases, I think there shouldn't be any incompatibility.
>
> The highest-risk patch is the second one, since it converts multi-line
> numeric output from subcommands into a list of numbers. However, this
> helps a lot in fixing some of the problems in the third issue
> (concatenating expansions), and I think it's more consistent overall
> than before. However, it could make sense to provide a defcustom to opt
> out of the behavior if people think that would be helpful.
Spelling is hard. I fixed a typo in the documentation for the second
patch (`eshell-convert-numeric-agument' ->
`eshell-convert-numeric-arguments').
[-- Attachment #2: 0001-Eshell-variable-expansion-should-always-return-strin.patch --]
[-- Type: text/plain, Size: 26136 bytes --]
From d07087b072dd410040c7ddb1322d1a1f3cd547df Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Mon, 28 Feb 2022 17:38:39 -0800
Subject: [PATCH 1/3] Eshell variable expansion should always return strings
inside quotes
This is closer in behavior to regular shells, and gives Eshell users
greater flexibility in how variables are expanded.
* lisp/eshell/esh-util.el (eshell-convert): Add TO-STRING argument.
* lisp/eshell/esh-var.el (eshell-parse-variable-ref): Add MODIFIER-P
argument and adjust how 'eshell-convert' and 'eshell-apply-indices'
are called.
(eshell-get-variable, eshell-apply-indices): Add QUOTED argument.
* test/lisp/eshell/esh-var-tests.el (eshell-test-value): New defvar.
(esh-var-test/interp-convert-var-number)
(esh-var-test/interp-convert-var-split-indices)
(esh-var-test/interp-convert-quoted-var-number)
(esh-var-test/interp-convert-quoted-var-split-indices)
(esh-var-test/interp-convert-cmd-string-newline)
(esh-var-test/interp-convert-cmd-multiline)
(esh-var-test/interp-convert-cmd-number)
(esh-var-test/interp-convert-cmd-split-indices)
(esh-var-test/quoted-interp-convert-var-number)
(esh-var-test/quoted-interp-convert-var-split-indices)
(esh-var-test/quoted-interp-convert-quoted-var-number)
(esh-var-test/quoted-interp-convert-quoted-var-split-indices)
(esh-var-test/quoted-interp-convert-cmd-string-newline)
(esh-var-test/quoted-interp-convert-cmd-multiline)
(esh-var-test/quoted-interp-convert-cmd-number)
(esh-var-test/quoted-interp-convert-cmd-split-indices): New tests.
* doc/misc/eshell.texi (Arguments): Expand this section, and document
the new behavior.
(Dollars Expansion): Provide more detail about '$(lisp)' and
'${command}' forms.
* etc/NEWS (Eshell): Announce this change (bug#55236).
---
doc/misc/eshell.texi | 57 +++++++---
etc/NEWS | 6 ++
lisp/eshell/esh-util.el | 46 +++++---
lisp/eshell/esh-var.el | 63 +++++++----
test/lisp/eshell/esh-var-tests.el | 171 ++++++++++++++++++++++++++----
5 files changed, 272 insertions(+), 71 deletions(-)
diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index a3ed922cf2..164a71f309 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -228,15 +228,39 @@ Invocation
@node Arguments
@section Arguments
-Command arguments are passed to the functions as either strings or
-numbers, depending on what the parser thinks they look like. If you
-need to use a function that takes some other data type, you will need to
-call it in an Elisp expression (which can also be used with
-@ref{Expansion, expansions}). As with other shells, you can
-escape special characters and spaces with the backslash (@code{\}) and
-apostrophes (@code{''}) and double quotes (@code{""}). This is needed
-especially for file names with special characters like pipe
-(@code{|}), which could be part of remote file names.
+Ordinarily, command arguments are parsed by Eshell as either strings
+or numbers, depending on what the parser thinks they look like. To
+specify an argument of some other data type, you can use an
+@ref{Dollars Expansion, Elisp expression}:
+
+@example
+~ $ echo (list 1 2 3)
+(1 2 3)
+@end example
+
+Additionally, many @ref{Built-ins, Eshell commands} will flatten the
+arguments they receive, so passing a list as an argument will
+``spread'' the elements into multiple arguments:
+
+@example
+~ $ printnl (list 1 2) 3
+1
+2
+3
+@end example
+
+@subsection Quoting and escaping
+
+As with other shells, you can escape special characters and spaces
+with by prefixing the character with a backslash (@code{\}), or by
+surrounding the string with apostrophes (@code{''}) or double quotes
+(@code{""}). This is needed especially for file names with special
+characters like pipe (@code{|}), which could be part of remote file
+names.
+
+When using @ref{Expansion, expansions} in an Eshell command, the
+result may potentially be of any data type. To ensure that the result
+is always a string, the expansion can be surrounded by double quotes.
@node Built-ins
@section Built-in commands
@@ -1026,11 +1050,20 @@ Dollars Expansion
@item $(@var{lisp})
Expands to the result of evaluating the S-expression @code{(@var{lisp})}. On
its own, this is identical to just @code{(@var{lisp})}, but with the @code{$},
-it can be used in a string, such as @samp{/some/path/$(@var{lisp}).txt}.
+it can be used inside double quotes or within a longer string, such as
+@samp{/some/path/$(@var{lisp}).txt}.
@item $@{@var{command}@}
-Returns the output of @command{@var{command}}, which can be any valid Eshell
-command invocation, and may even contain expansions.
+Returns the output of @command{@var{command}}, which can be any valid
+Eshell command invocation, and may even contain expansions. Similar
+to @code{$(@var{lisp})}, this is identical to @code{@{@var{command}@}}
+when on its own, but the @code{$} allows it to be used inside double
+quotes or as part of a string.
+
+Normally, the output is split line-by-line, returning a list (or the
+first element if there's only one line of output). However, when this
+expansion is surrounded by double quotes, it returns the output as a
+single string instead.
@item $<@var{command}>
As with @samp{$@{@var{command}@}}, evaluates the Eshell command invocation
diff --git a/etc/NEWS b/etc/NEWS
index 882748d8c7..ae815bb785 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1370,6 +1370,12 @@ Lisp function. This frees you from having to keep track of whether
commands are Lisp function or external when supplying absolute file
name arguments. See "Electric forward slash" in the Eshell manual.
++++
+*** Double-quoting an Eshell expansion now treats the result as a single string.
+If an Eshell expansion like '$FOO' is surrounded by double quotes, the
+result will always be a single string, no matter the type that would
+otherwise be returned.
+
---
*** Built-in Eshell commands now follow POSIX/GNU argument syntax conventions.
Built-in commands in Eshell now accept command-line options with
diff --git a/lisp/eshell/esh-util.el b/lisp/eshell/esh-util.el
index 3da712c719..6c130974e9 100644
--- a/lisp/eshell/esh-util.el
+++ b/lisp/eshell/esh-util.el
@@ -198,23 +198,37 @@ eshell-find-delimiter
(when (= depth 0)
(if reverse-p (point) (1- (point)))))))
-(defun eshell-convert (string)
- "Convert STRING into a more native looking Lisp object."
- (if (not (stringp string))
- string
- (let ((len (length string)))
- (if (= len 0)
- string
- (if (eq (aref string (1- len)) ?\n)
+(defun eshell-convert (string &optional to-string)
+ "Convert STRING into a more-native Lisp object.
+If TO-STRING is non-nil, always return a single string with
+trailing newlines removed. Otherwise, this behaves as follows:
+
+* Return non-strings as-is.
+
+* Split multiline strings by line.
+
+* If `eshell-convert-numeric-aguments' is non-nil, convert
+ numeric strings to numbers."
+ (cond
+ ((not (stringp string))
+ (if to-string
+ (eshell-stringify string)
+ string))
+ (to-string (string-trim-right string "\n+"))
+ (t (let ((len (length string)))
+ (if (= len 0)
+ string
+ (when (eq (aref string (1- len)) ?\n)
(setq string (substring string 0 (1- len))))
- (if (string-search "\n" string)
- (split-string string "\n")
- (if (and eshell-convert-numeric-arguments
- (string-match
- (concat "\\`\\s-*" eshell-number-regexp
- "\\s-*\\'") string))
- (string-to-number string)
- string))))))
+ (cond
+ ((string-search "\n" string)
+ (split-string string "\n"))
+ ((and eshell-convert-numeric-arguments
+ (string-match
+ (concat "\\`\\s-*" eshell-number-regexp "\\s-*\\'")
+ string))
+ (string-to-number string))
+ (t string)))))))
(defvar-local eshell-path-env (getenv "PATH")
"Content of $PATH.
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 3c6bcc753c..1c28d24af1 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -402,23 +402,30 @@ eshell-parse-variable
(let* ((get-len (when (eq (char-after) ?#)
(forward-char) t))
value indices)
- (setq value (eshell-parse-variable-ref)
+ (setq value (eshell-parse-variable-ref get-len)
indices (and (not (eobp))
(eq (char-after) ?\[)
(eshell-parse-indices))
;; This is an expression that will be evaluated by `eshell-do-eval',
;; which only support let-binding of dynamically-scoped vars
value `(let ((indices (eshell-eval-indices ',indices))) ,value))
- (if get-len
- `(length ,value)
- value)))
+ (when get-len
+ (setq value `(length ,value)))
+ (when eshell-current-quoted
+ (setq value `(eshell-stringify ,value)))
+ value))
-(defun eshell-parse-variable-ref ()
+(defun eshell-parse-variable-ref (&optional modifier-p)
"Eval a variable reference.
Returns a Lisp form which, if evaluated, will return the value of the
variable.
-Possible options are:
+If MODIFIER-P is non-nil, the value of the variable will be
+modified by some function. If MODIFIER-P is nil, the value will be
+used as-is; this allows optimization of some kinds of variable
+references.
+
+Possible variable references are:
NAME an environment or Lisp variable value
\"LONG-NAME\" disambiguates the length of the name
@@ -441,8 +448,16 @@ eshell-parse-variable-ref
,(let ((subcmd (or (eshell-unescape-inner-double-quote end)
(cons (point) end)))
(eshell-current-quoted nil))
- (eshell-parse-command subcmd)))))
- indices)
+ (eshell-parse-command subcmd))))
+ ;; If this is a simple double-quoted form like
+ ;; "${COMMAND}" (i.e. no indices after the subcommand
+ ;; and no `#' modifier before), ensure we convert to a
+ ;; single string. This avoids unnecessary work
+ ;; (e.g. splitting the output by lines) when it would
+ ;; just be joined back together afterwards.
+ ,(when (and (not modifier-p) eshell-current-quoted)
+ '(not indices)))
+ indices ,eshell-current-quoted)
(goto-char (1+ end))))))
((eq (char-after) ?\<)
(let ((end (eshell-find-delimiter ?\< ?\>)))
@@ -466,7 +481,7 @@ eshell-parse-variable-ref
;; properly. See bug#54190.
(list (function (lambda ()
(delete-file ,temp))))))
- (eshell-apply-indices ,temp indices)))
+ (eshell-apply-indices ,temp indices ,eshell-current-quoted)))
(goto-char (1+ end)))))))
((eq (char-after) ?\()
(condition-case nil
@@ -475,7 +490,7 @@ eshell-parse-variable-ref
(eshell-lisp-command
',(read (or (eshell-unescape-inner-double-quote (point-max))
(current-buffer)))))
- indices)
+ indices ,eshell-current-quoted)
(end-of-file
(throw 'eshell-incomplete ?\())))
((looking-at (rx-to-string
@@ -487,14 +502,15 @@ eshell-parse-variable-ref
(eshell-parse-literal-quote)
(eshell-parse-double-quote))))
(when name
- `(eshell-get-variable ,(eval name) indices)))))
+ `(eshell-get-variable ,(eval name) indices ,eshell-current-quoted)))))
((assoc (char-to-string (char-after))
eshell-variable-aliases-list)
(forward-char)
- `(eshell-get-variable ,(char-to-string (char-before)) indices))
+ `(eshell-get-variable ,(char-to-string (char-before)) indices
+ ,eshell-current-quoted))
((looking-at eshell-variable-name-regexp)
(prog1
- `(eshell-get-variable ,(match-string 0) indices)
+ `(eshell-get-variable ,(match-string 0) indices ,eshell-current-quoted)
(goto-char (match-end 0))))
(t
(error "Invalid variable reference"))))
@@ -525,8 +541,10 @@ eshell-eval-indices
"Evaluate INDICES, a list of index-lists generated by `eshell-parse-indices'."
(mapcar (lambda (i) (mapcar #'eval i)) indices))
-(defun eshell-get-variable (name &optional indices)
- "Get the value for the variable NAME."
+(defun eshell-get-variable (name &optional indices quoted)
+ "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)
@@ -547,9 +565,9 @@ eshell-get-variable
(symbol-value var))
(t
(error "Unknown variable `%s'" (eshell-stringify var))))
- indices))))
+ indices quoted))))
-(defun eshell-apply-indices (value indices)
+(defun eshell-apply-indices (value indices &optional quoted)
"Apply to VALUE all of the given INDICES, returning the sub-result.
The format of INDICES is:
@@ -558,12 +576,17 @@ eshell-apply-indices
Each member of INDICES represents a level of nesting. If the first
member of a sublist is not an integer or name, and the value it's
-reference is a string, that will be used as the regexp with which is
-to divide the string into sub-parts. The default is whitespace.
+referencing is a string, that will be used as the regexp with which
+is to divide the string into sub-parts. The default is whitespace.
Otherwise, each INT-OR-NAME refers to an element of the list value.
Integers imply a direct index, and names, an associate lookup using
`assoc'.
+If QUOTED is non-nil, this was invoked inside double-quotes. This
+affects the behavior of splitting strings: without quoting, the
+split values are converted to Lisp forms via `eshell-convert'; with
+quoting, they're left as strings.
+
For example, to retrieve the second element of a user's record in
'/etc/passwd', the variable reference would look like:
@@ -577,7 +600,7 @@ eshell-apply-indices
(setq separator index
refs (cdr refs)))
(setq value
- (mapcar #'eshell-convert
+ (mapcar (lambda (i) (eshell-convert i quoted))
(split-string value separator)))))
(cond
((< (length refs) 0)
diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el
index 1d051d681a..5363a86e71 100644
--- a/test/lisp/eshell/esh-var-tests.el
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -210,12 +210,17 @@ esh-var-test/quoted-interp-var-indices
(should (equal (eshell-test-command-result
"echo \"$eshell-test-value[0]\"")
"zero"))
+ ;; FIXME: These tests would use the 0th index like the other tests
+ ;; here, but evaluating the command just above adds an `escaped'
+ ;; property to the string "zero". This results in the output
+ ;; printing the string properties, which is probably the wrong
+ ;; behavior. See bug#54486.
(should (equal (eshell-test-command-result
- "echo \"$eshell-test-value[0 2]\"")
- '("zero" "two")))
+ "echo \"$eshell-test-value[1 2]\"")
+ "(\"one\" \"two\")"))
(should (equal (eshell-test-command-result
- "echo \"$eshell-test-value[0 2 4]\"")
- '("zero" "two" "four")))))
+ "echo \"$eshell-test-value[1 2 4]\"")
+ "(\"one\" \"two\" \"four\")"))))
(ert-deftest esh-var-test/quoted-interp-var-split-indices ()
"Interpolate string variable with indices inside double-quotes"
@@ -225,7 +230,7 @@ esh-var-test/quoted-interp-var-split-indices
"zero"))
(should (equal (eshell-test-command-result
"echo \"$eshell-test-value[0 2]\"")
- '("zero" "two")))))
+ "(\"zero\" \"two\")"))))
(ert-deftest esh-var-test/quoted-interp-var-string-split-indices ()
"Interpolate string variable with string splitter and indices
@@ -236,14 +241,14 @@ esh-var-test/quoted-interp-var-string-split-indices
"zero"))
(should (equal (eshell-test-command-result
"echo \"$eshell-test-value[: 0 2]\"")
- '("zero" "two"))))
+ "(\"zero\" \"two\")")))
(let ((eshell-test-value "zeroXoneXtwoXthreeXfour"))
(should (equal (eshell-test-command-result
"echo \"$eshell-test-value[X 0]\"")
"zero"))
(should (equal (eshell-test-command-result
"echo \"$eshell-test-value[X 0 2]\"")
- '("zero" "two")))))
+ "(\"zero\" \"two\")"))))
(ert-deftest esh-var-test/quoted-interp-var-regexp-split-indices ()
"Interpolate string variable with regexp splitter and indices"
@@ -253,43 +258,47 @@ esh-var-test/quoted-interp-var-regexp-split-indices
"zero"))
(should (equal (eshell-test-command-result
"echo \"$eshell-test-value['[:!]' 0 2]\"")
- '("zero" "two")))
+ "(\"zero\" \"two\")"))
(should (equal (eshell-test-command-result
"echo \"$eshell-test-value[\\\"[:!]\\\" 0]\"")
"zero"))
(should (equal (eshell-test-command-result
"echo \"$eshell-test-value[\\\"[:!]\\\" 0 2]\"")
- '("zero" "two")))))
+ "(\"zero\" \"two\")"))))
(ert-deftest esh-var-test/quoted-interp-var-assoc ()
"Interpolate alist variable with index inside double-quotes"
(let ((eshell-test-value '(("foo" . 1))))
(should (equal (eshell-test-command-result
"echo \"$eshell-test-value[foo]\"")
- 1))))
+ "1"))))
(ert-deftest esh-var-test/quoted-interp-var-length-list ()
"Interpolate length of list variable inside double-quotes"
(let ((eshell-test-value '((1 2) (3) (5 (6 7 8 9)))))
- (should (eq (eshell-test-command-result "echo \"$#eshell-test-value\"") 3))
- (should (eq (eshell-test-command-result "echo \"$#eshell-test-value[1]\"")
- 1))
- (should (eq (eshell-test-command-result
- "echo \"$#eshell-test-value[2][1]\"")
- 4))))
+ (should (equal (eshell-test-command-result "echo \"$#eshell-test-value\"")
+ "3"))
+ (should (equal (eshell-test-command-result
+ "echo \"$#eshell-test-value[1]\"")
+ "1"))
+ (should (equal (eshell-test-command-result
+ "echo \"$#eshell-test-value[2][1]\"")
+ "4"))))
(ert-deftest esh-var-test/quoted-interp-var-length-string ()
"Interpolate length of string variable inside double-quotes"
(let ((eshell-test-value "foobar"))
- (should (eq (eshell-test-command-result "echo \"$#eshell-test-value\"")
- 6))))
+ (should (equal (eshell-test-command-result "echo \"$#eshell-test-value\"")
+ "6"))))
(ert-deftest esh-var-test/quoted-interp-var-length-alist ()
"Interpolate length of alist variable inside double-quotes"
(let ((eshell-test-value '(("foo" . (1 2 3)))))
- (should (eq (eshell-test-command-result "echo \"$#eshell-test-value\"") 1))
- (should (eq (eshell-test-command-result "echo \"$#eshell-test-value[foo]\"")
- 3))))
+ (should (equal (eshell-test-command-result "echo \"$#eshell-test-value\"")
+ "1"))
+ (should (equal (eshell-test-command-result
+ "echo \"$#eshell-test-value[foo]\"")
+ "3"))))
(ert-deftest esh-var-test/quoted-interp-lisp ()
"Interpolate Lisp form evaluation inside double-quotes"
@@ -299,7 +308,8 @@ esh-var-test/quoted-interp-lisp
(ert-deftest esh-var-test/quoted-interp-lisp-indices ()
"Interpolate Lisp form evaluation with index"
- (should (equal (eshell-test-command-result "+ \"$(list 1 2)[1]\" 3") 5)))
+ (should (equal (eshell-test-command-result "concat \"$(list 1 2)[1]\" cool")
+ "2cool")))
(ert-deftest esh-var-test/quoted-interp-cmd ()
"Interpolate command result inside double-quotes"
@@ -309,12 +319,127 @@ esh-var-test/quoted-interp-cmd
(ert-deftest esh-var-test/quoted-interp-cmd-indices ()
"Interpolate command result with index inside double-quotes"
- (should (equal (eshell-test-command-result "+ \"${list 1 2}[1]\" 3") 5)))
+ (should (equal (eshell-test-command-result "concat \"${list 1 2}[1]\" cool")
+ "2cool")))
(ert-deftest esh-var-test/quoted-interp-temp-cmd ()
"Interpolate command result redirected to temp file inside double-quotes"
(should (equal (eshell-test-command-result "cat \"$<echo hi>\"") "hi")))
+\f
+;; Interpolated variable conversion
+
+(ert-deftest esh-var-test/interp-convert-var-number ()
+ "Interpolate numeric variable"
+ (let ((eshell-test-value 123))
+ (should (equal (eshell-test-command-result "type-of $eshell-test-value")
+ 'integer))))
+
+(ert-deftest esh-var-test/interp-convert-var-split-indices ()
+ "Interpolate and convert string variable with indices"
+ (let ((eshell-test-value "000 010 020 030 040"))
+ (should (equal (eshell-test-command-result "echo $eshell-test-value[0]")
+ 0))
+ (should (equal (eshell-test-command-result "echo $eshell-test-value[0 2]")
+ '(0 20)))))
+
+(ert-deftest esh-var-test/interp-convert-quoted-var-number ()
+ "Interpolate numeric quoted numeric variable"
+ (let ((eshell-test-value 123))
+ (should (equal (eshell-test-command-result "type-of $'eshell-test-value'")
+ 'integer))
+ (should (equal (eshell-test-command-result "type-of $\"eshell-test-value\"")
+ 'integer))))
+
+(ert-deftest esh-var-test/interp-convert-quoted-var-split-indices ()
+ "Interpolate and convert quoted string variable with indices"
+ (let ((eshell-test-value "000 010 020 030 040"))
+ (should (equal (eshell-test-command-result "echo $'eshell-test-value'[0]")
+ 0))
+ (should (equal (eshell-test-command-result "echo $'eshell-test-value'[0 2]")
+ '(0 20)))))
+
+(ert-deftest esh-var-test/interp-convert-cmd-string-newline ()
+ "Interpolate trailing-newline command result"
+ (should (equal (eshell-test-command-result "echo ${echo \"foo\n\"}") "foo")))
+
+(ert-deftest esh-var-test/interp-convert-cmd-multiline ()
+ "Interpolate multi-line command result"
+ (should (equal (eshell-test-command-result "echo ${echo \"foo\nbar\"}")
+ '("foo" "bar"))))
+
+(ert-deftest esh-var-test/interp-convert-cmd-number ()
+ "Interpolate numeric command result"
+ (should (equal (eshell-test-command-result "echo ${echo \"1\"}") 1)))
+
+(ert-deftest esh-var-test/interp-convert-cmd-split-indices ()
+ "Interpolate command result with indices"
+ (should (equal (eshell-test-command-result "echo ${echo \"000 010 020\"}[0]")
+ 0))
+ (should (equal (eshell-test-command-result
+ "echo ${echo \"000 010 020\"}[0 2]")
+ '(0 20))))
+
+(ert-deftest esh-var-test/quoted-interp-convert-var-number ()
+ "Interpolate numeric variable inside double-quotes"
+ (let ((eshell-test-value 123))
+ (should (equal (eshell-test-command-result "type-of \"$eshell-test-value\"")
+ 'string))))
+
+(ert-deftest esh-var-test/quoted-interp-convert-var-split-indices ()
+ "Interpolate string variable with indices inside double-quotes"
+ (let ((eshell-test-value "000 010 020 030 040"))
+ (should (equal (eshell-test-command-result
+ "echo \"$eshell-test-value[0]\"")
+ "000"))
+ (should (equal (eshell-test-command-result
+ "echo \"$eshell-test-value[0 2]\"")
+ "(\"000\" \"020\")"))))
+
+(ert-deftest esh-var-test/quoted-interp-convert-quoted-var-number ()
+ "Interpolate numeric quoted variable inside double-quotes"
+ (let ((eshell-test-value 123))
+ (should (equal (eshell-test-command-result
+ "type-of \"$'eshell-test-value'\"")
+ 'string))
+ (should (equal (eshell-test-command-result
+ "type-of \"$\\\"eshell-test-value\\\"\"")
+ 'string))))
+
+(ert-deftest esh-var-test/quoted-interp-convert-quoted-var-split-indices ()
+ "Interpolate quoted string variable with indices inside double-quotes"
+ (let ((eshell-test-value "000 010 020 030 040"))
+ (should (equal (eshell-test-command-result
+ "echo \"$eshell-test-value[0]\"")
+ "000"))
+ (should (equal (eshell-test-command-result
+ "echo \"$eshell-test-value[0 2]\"")
+ "(\"000\" \"020\")"))))
+
+(ert-deftest esh-var-test/quoted-interp-convert-cmd-string-newline ()
+ "Interpolate trailing-newline command result inside double-quotes"
+ (should (equal (eshell-test-command-result "echo \"${echo \\\"foo\n\\\"}\"")
+ "foo"))
+ (should (equal (eshell-test-command-result "echo \"${echo \\\"foo\n\n\\\"}\"")
+ "foo")))
+
+(ert-deftest esh-var-test/quoted-interp-convert-cmd-multiline ()
+ "Interpolate multi-line command result inside double-quotes"
+ (should (equal (eshell-test-command-result
+ "echo \"${echo \\\"foo\nbar\\\"}\"")
+ "foo\nbar")))
+
+(ert-deftest esh-var-test/quoted-interp-convert-cmd-number ()
+ "Interpolate numeric command result inside double-quotes"
+ (should (equal (eshell-test-command-result "echo \"${echo \\\"1\\\"}\"")
+ "1")))
+
+(ert-deftest esh-var-test/quoted-interp-convert-cmd-split-indices ()
+ "Interpolate command result with indices inside double-quotes"
+ (should (equal (eshell-test-command-result
+ "echo \"${echo \\\"000 010 020\\\"}[0]\"")
+ "000")))
+
\f
;; Built-in variables
--
2.25.1
[-- Attachment #3: 0002-Return-a-list-of-numbers-if-all-lines-of-an-Eshell-s.patch --]
[-- Type: text/plain, Size: 5744 bytes --]
From 907fa43255b2919e70f5297483367ddbc21d1b4e Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 1 May 2022 22:09:17 -0700
Subject: [PATCH 2/3] Return a list of numbers if all lines of an Eshell
subcommand are numeric
* lisp/eshell/esh-util.el (eshell-convertible-to-number-p)
(eshell-convert-to-number): New functions...
(eshell-convert): ... use them.
* test/lisp/eshell/esh-var-tests.el
(esh-var-test/interp-convert-cmd-string-newline): Add checks for
numeric output.
* doc/misc/eshell.texi (Dollars Expansion): Document the new behavior.
* etc/NEWS: Announce the change (bug#55236).
---
doc/misc/eshell.texi | 8 ++++---
etc/NEWS | 7 ++++++
lisp/eshell/esh-util.el | 36 +++++++++++++++++++++----------
test/lisp/eshell/esh-var-tests.el | 8 ++++++-
4 files changed, 44 insertions(+), 15 deletions(-)
diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 164a71f309..f13f1cacfc 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1061,9 +1061,11 @@ Dollars Expansion
quotes or as part of a string.
Normally, the output is split line-by-line, returning a list (or the
-first element if there's only one line of output). However, when this
-expansion is surrounded by double quotes, it returns the output as a
-single string instead.
+first element if there's only one line of output); if
+@code{eshell-convert-numeric-arguments} is non-@code{nil} and every
+line of output looks like a number, convert each line to a number.
+However, when this expansion is surrounded by double quotes, it
+returns the output as a single string instead.
@item $<@var{command}>
As with @samp{$@{@var{command}@}}, evaluates the Eshell command invocation
diff --git a/etc/NEWS b/etc/NEWS
index ae815bb785..0c77b0bf58 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1376,6 +1376,13 @@ If an Eshell expansion like '$FOO' is surrounded by double quotes, the
result will always be a single string, no matter the type that would
otherwise be returned.
++++
+*** Eshell subcommands with multiline numeric output return lists of numbers.
+If every line of the output of an Eshell subcommand like '${COMMAND}'
+is numeric, the result will be a list of numbers (or a single number
+if only one line of output). Previously, this only converted numbers
+when there was a single line of output.
+
---
*** Built-in Eshell commands now follow POSIX/GNU argument syntax conventions.
Built-in commands in Eshell now accept command-line options with
diff --git a/lisp/eshell/esh-util.el b/lisp/eshell/esh-util.el
index 6c130974e9..9960912bce 100644
--- a/lisp/eshell/esh-util.el
+++ b/lisp/eshell/esh-util.el
@@ -198,6 +198,23 @@ eshell-find-delimiter
(when (= depth 0)
(if reverse-p (point) (1- (point)))))))
+(defun eshell-convertible-to-number-p (string)
+ "Return non-nil if STRING can be converted to a number.
+If `eshell-convert-numeric-aguments', always return nil."
+ (and eshell-convert-numeric-arguments
+ (string-match
+ (concat "\\`\\s-*" eshell-number-regexp "\\s-*\\'")
+ string)))
+
+(defun eshell-convert-to-number (string)
+ "Try to convert STRING to a number.
+If STRING doesn't look like a number (or
+`eshell-convert-numeric-aguments' is nil), just return STRING
+unchanged."
+ (if (eshell-convertible-to-number-p string)
+ (string-to-number string)
+ string))
+
(defun eshell-convert (string &optional to-string)
"Convert STRING into a more-native Lisp object.
If TO-STRING is non-nil, always return a single string with
@@ -207,8 +224,8 @@ eshell-convert
* Split multiline strings by line.
-* If `eshell-convert-numeric-aguments' is non-nil, convert
- numeric strings to numbers."
+* If `eshell-convert-numeric-aguments' is non-nil and every line
+ of output looks like a number, convert them to numbers."
(cond
((not (stringp string))
(if to-string
@@ -220,15 +237,12 @@ eshell-convert
string
(when (eq (aref string (1- len)) ?\n)
(setq string (substring string 0 (1- len))))
- (cond
- ((string-search "\n" string)
- (split-string string "\n"))
- ((and eshell-convert-numeric-arguments
- (string-match
- (concat "\\`\\s-*" eshell-number-regexp "\\s-*\\'")
- string))
- (string-to-number string))
- (t string)))))))
+ (if (string-search "\n" string)
+ (let ((lines (split-string string "\n")))
+ (if (seq-every-p #'eshell-convertible-to-number-p lines)
+ (mapcar #'string-to-number lines)
+ lines))
+ (eshell-convert-to-number string)))))))
(defvar-local eshell-path-env (getenv "PATH")
"Content of $PATH.
diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el
index 5363a86e71..2ce6bb4f1b 100644
--- a/test/lisp/eshell/esh-var-tests.el
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -366,7 +366,13 @@ esh-var-test/interp-convert-cmd-string-newline
(ert-deftest esh-var-test/interp-convert-cmd-multiline ()
"Interpolate multi-line command result"
(should (equal (eshell-test-command-result "echo ${echo \"foo\nbar\"}")
- '("foo" "bar"))))
+ '("foo" "bar")))
+ ;; Numeric output should be converted to numbers...
+ (should (equal (eshell-test-command-result "echo ${echo \"01\n02\n03\"}")
+ '(1 2 3)))
+ ;; ... but only if every line is numeric.
+ (should (equal (eshell-test-command-result "echo ${echo \"01\n02\nhi\"}")
+ '("01" "02" "hi"))))
(ert-deftest esh-var-test/interp-convert-cmd-number ()
"Interpolate numeric command result"
--
2.25.1
[-- Attachment #4: 0003-Improve-the-behavior-of-concatenating-parts-of-Eshel.patch --]
[-- Type: text/plain, Size: 9764 bytes --]
From 55959a5874f65c2dfd9e3ac5549577daceaabe6e Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Mon, 2 May 2022 16:56:49 -0700
Subject: [PATCH 3/3] Improve the behavior of concatenating parts of Eshell
arguments
Previously, concatenating a list to a string would first convert the
list to a string. Now, the string is concatenated with the last
element of the list.
* lisp/eshell/esh-util.el (eshell-to-flat-string): Make obsolete.
* lisp/eshell/esh-arg.el (eshell-concat, eshell-concat-1): New
functions.
(eshell-resolve-current-argument): Use 'eshell-concat'.
* test/lisp/eshell/esh-var-tests.el (esh-var-test/interp-concat-cmd):
Add check for concatenation of multiline output of subcommands.
(esh-var-test/quoted-interp-concat-cmd): New test.
* test/lisp/eshell/em-extpipe-tests.el (em-extpipe-test-13): Use
'eshell-concat'.
* doc/misc/eshell.texi (Expansion): Document this behavior.
* etc/NEWS: Announce the change (bug#55236).
---
doc/misc/eshell.texi | 34 +++++++++++++--
etc/NEWS | 7 ++++
lisp/eshell/esh-arg.el | 62 ++++++++++++++++++++++++----
lisp/eshell/esh-util.el | 1 +
test/lisp/eshell/em-extpipe-tests.el | 2 +-
test/lisp/eshell/esh-var-tests.el | 19 ++++++++-
6 files changed, 109 insertions(+), 16 deletions(-)
diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index f13f1cacfc..960f5d9335 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1017,11 +1017,37 @@ Expansion
shell, they are less often used for constants, and usually for using
variables and string manipulation.@footnote{Eshell has no
string-manipulation expansions because the Elisp library already
-provides many functions for this.} For example, @code{$var} on a line
-expands to the value of the variable @code{var} when the line is
+provides many functions for this.} For example, @code{$@var{var}} on
+a line expands to the value of the variable @var{var} when the line is
executed. Expansions are usually passed as arguments, but may also be
-used as commands.@footnote{E.g., entering just @samp{$var} at the prompt
-is equivalent to entering the value of @code{var} at the prompt.}
+used as commands.@footnote{E.g., entering just @samp{$@var{var}} at
+the prompt is equivalent to entering the value of @var{var} at the
+prompt.}
+
+You can concatenate expansions with regular string arguments or even
+other expansions. In the simplest case, when the expansion returns a
+string value, this is equivalent to ordinary string concatenation; for
+example, @samp{$@{echo "foo"@}bar} returns @samp{foobar}. The exact
+behavior depends on the types of each value being concatenated:
+
+@table @asis
+
+@item both strings
+Concatenate both values together.
+
+@item one or both numbers
+Concatenate the string representation of each value, converting back to
+a number if possible.
+
+@item one or both (non-@code{nil}) lists
+Concatenate ``adjacent'' elements of each value (possibly converting
+back to a number as above). For example, @samp{$list("a" "b")c}
+returns @samp{("a" "bc")}.
+
+@item anything else
+Concatenate the string represenation of each value.
+
+@end table
@menu
* Dollars Expansion::
diff --git a/etc/NEWS b/etc/NEWS
index 0c77b0bf58..360540b02a 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1376,6 +1376,13 @@ If an Eshell expansion like '$FOO' is surrounded by double quotes, the
result will always be a single string, no matter the type that would
otherwise be returned.
++++
+*** Concatenating Eshell expansions now works more similarly to other shells.
+When concatenating an Eshell expansion that returns a list, "adjacent"
+elements of each operand are now concatenated together,
+e.g. '$list("a" "b")c' returns '("a" "bc")'. See the "(eshell)
+Expansion" node in the Eshell manual for more details.
+
+++
*** Eshell subcommands with multiline numeric output return lists of numbers.
If every line of the output of an Eshell subcommand like '${COMMAND}'
diff --git a/lisp/eshell/esh-arg.el b/lisp/eshell/esh-arg.el
index 395aa87ff0..459487f435 100644
--- a/lisp/eshell/esh-arg.el
+++ b/lisp/eshell/esh-arg.el
@@ -180,19 +180,63 @@ eshell-escape-arg
(add-text-properties 0 (length string) '(escaped t) string))
string)
+(defun eshell-concat (quoted &rest rest)
+ "Concatenate all the arguments in REST and return the result.
+If QUOTED is nil, the resulting value(s) may be converted to
+numbers (see `eshell-concat-1').
+
+If each argument in REST is a non-list value, the result will be
+a single value, as if (mapconcat #'eshell-stringify REST) had been
+called, possibly converted to a number.
+
+If there is at least one (non-nil) list argument, the result will
+be a list, with \"adjacent\" elements of consecutive arguments
+concatenated as strings (again, possibly converted to numbers).
+For example, concatenating \"a\", (\"b\"), and (\"c\" \"d\")
+would produce (\"abc\" \"d\")."
+ (let (result)
+ (dolist (i rest result)
+ (when i
+ (cond
+ ((null result)
+ (setq result i))
+ ((listp result)
+ (let (curr-head curr-tail)
+ (if (listp i)
+ (setq curr-head (car i)
+ curr-tail (cdr i))
+ (setq curr-head i
+ curr-tail nil))
+ (setq result
+ (append
+ (butlast result 1)
+ (list (eshell-concat-1 quoted (car (last result))
+ curr-head))
+ curr-tail))))
+ ((listp i)
+ (setq result
+ (cons (eshell-concat-1 quoted result (car i))
+ (cdr i))))
+ (t
+ (setq result (eshell-concat-1 quoted result i))))))))
+
+(defun eshell-concat-1 (quoted first second)
+ "Concatenate FIRST and SECOND.
+If QUOTED is nil and either FIRST or SECOND are numbers, try to
+convert the result to a number as well."
+ (let ((result (concat (eshell-stringify first) (eshell-stringify second))))
+ (if (and (not quoted)
+ (or (numberp first) (numberp second)))
+ (eshell-convert-to-number result)
+ result)))
+
(defun eshell-resolve-current-argument ()
"If there are pending modifications to be made, make them now."
(when eshell-current-argument
(when eshell-arg-listified
- (let ((parts eshell-current-argument))
- (while parts
- (unless (stringp (car parts))
- (setcar parts
- (list 'eshell-to-flat-string (car parts))))
- (setq parts (cdr parts)))
- (setq eshell-current-argument
- (list 'eshell-convert
- (append (list 'concat) eshell-current-argument))))
+ (setq eshell-current-argument
+ (append (list 'eshell-concat eshell-current-quoted)
+ eshell-current-argument))
(setq eshell-arg-listified nil))
(while eshell-current-modifiers
(setq eshell-current-argument
diff --git a/lisp/eshell/esh-util.el b/lisp/eshell/esh-util.el
index 9960912bce..b5a423f023 100644
--- a/lisp/eshell/esh-util.el
+++ b/lisp/eshell/esh-util.el
@@ -293,6 +293,7 @@ eshell-split-path
(defun eshell-to-flat-string (value)
"Make value a string. If separated by newlines change them to spaces."
+ (declare (obsolete nil "29.1"))
(let ((text (eshell-stringify value)))
(if (string-match "\n+\\'" text)
(setq text (replace-match "" t t text)))
diff --git a/test/lisp/eshell/em-extpipe-tests.el b/test/lisp/eshell/em-extpipe-tests.el
index 91c2fba479..3b84d763ac 100644
--- a/test/lisp/eshell/em-extpipe-tests.el
+++ b/test/lisp/eshell/em-extpipe-tests.el
@@ -170,7 +170,7 @@ em-extpipe-test-12
(em-extpipe-tests--deftest em-extpipe-test-13 "foo*|bar"
(should-parse '(eshell-execute-pipeline
- '((eshell-named-command (concat "foo" "*"))
+ '((eshell-named-command (eshell-concat nil "foo" "*"))
(eshell-named-command "bar")))))
(em-extpipe-tests--deftest em-extpipe-test-14 "tac *<temp"
diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el
index 2ce6bb4f1b..3f3b591c5a 100644
--- a/test/lisp/eshell/esh-var-tests.el
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -176,8 +176,17 @@ esh-var-test/interp-concat-lisp2
(should (equal (eshell-test-command-result "+ $(+ 1 2)$(+ 1 2) 3") 36)))
(ert-deftest esh-var-test/interp-concat-cmd ()
- "Interpolate and concat command"
- (should (equal (eshell-test-command-result "+ ${+ 1 2}3 3") 36)))
+ "Interpolate and concat command with literal"
+ (should (equal (eshell-test-command-result "+ ${+ 1 2}3 3") 36))
+ (should (equal (eshell-test-command-result "echo ${*echo \"foo\nbar\"}-baz")
+ '("foo" "bar-baz")))
+ ;; Concatenating to a number in a list should produce a number...
+ (should (equal (eshell-test-command-result "echo ${*echo \"1\n2\"}3")
+ '(1 23)))
+ ;; ... but concatenating to a string that looks like a number in a list
+ ;; should produce a string.
+ (should (equal (eshell-test-command-result "echo ${*echo \"hi\n2\"}3")
+ '("hi" "23"))))
(ert-deftest esh-var-test/interp-concat-cmd2 ()
"Interpolate and concat two commands"
@@ -326,6 +335,12 @@ esh-var-test/quoted-interp-temp-cmd
"Interpolate command result redirected to temp file inside double-quotes"
(should (equal (eshell-test-command-result "cat \"$<echo hi>\"") "hi")))
+(ert-deftest esh-var-test/quoted-interp-concat-cmd ()
+ "Interpolate and concat command with literal"
+ (should (equal (eshell-test-command-result
+ "echo \"${echo \\\"foo\nbar\\\"} baz\"")
+ "foo\nbar baz")))
+
\f
;; Interpolated variable conversion
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* bug#55236: 29.0.50; Surprising behaviors with Eshell expansions
2022-05-03 3:41 bug#55236: 29.0.50; Surprising behaviors with Eshell expansions Jim Porter
2022-05-03 3:45 ` bug#55236: [PATCH] " Jim Porter
@ 2022-05-03 9:32 ` Michael Albinus
1 sibling, 0 replies; 5+ messages in thread
From: Michael Albinus @ 2022-05-03 9:32 UTC (permalink / raw)
To: Jim Porter; +Cc: 55236
Jim Porter <jporterbugs@gmail.com> writes:
Hi Jim,
> From "emacs -Q --eval '(eshell)'":
Unrelated to this bug, but convenient in general: The same can be done by
emacs -Q -f eshell
Best regards, Michael (always lazy).
^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#55236: 29.0.50; Surprising behaviors with Eshell expansions
2022-05-03 4:20 ` bug#55236: [PATCH v2] " Jim Porter
@ 2022-05-03 16:24 ` Lars Ingebrigtsen
0 siblings, 0 replies; 5+ messages in thread
From: Lars Ingebrigtsen @ 2022-05-03 16:24 UTC (permalink / raw)
To: Jim Porter; +Cc: 55236
Jim Porter <jporterbugs@gmail.com> writes:
>> The highest-risk patch is the second one, since it converts
>> multi-line numeric output from subcommands into a list of
>> numbers. However, this helps a lot in fixing some of the problems in
>> the third issue (concatenating expansions), and I think it's more
>> consistent overall than before. However, it could make sense to
>> provide a defcustom to opt out of the behavior if people think that
>> would be helpful.
I guess it makes sense to wait to see whether anybody requests a
defcustom before adding it here.
> Spelling is hard. I fixed a typo in the documentation for the second
> patch (`eshell-convert-numeric-agument' ->
> `eshell-convert-numeric-arguments').
Thanks; pushed to Emacs 29.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-05-03 16:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-03 3:41 bug#55236: 29.0.50; Surprising behaviors with Eshell expansions Jim Porter
2022-05-03 3:45 ` bug#55236: [PATCH] " Jim Porter
2022-05-03 4:20 ` bug#55236: [PATCH v2] " Jim Porter
2022-05-03 16:24 ` bug#55236: " Lars Ingebrigtsen
2022-05-03 9:32 ` Michael Albinus
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).