Improve correctness of 'eshell-quote-argument'

* lisp/eshell/esh-arg.el (eshell-quote-argument): Mention that this
function is for use within Eshell buffers.
(eshell-quote-backslash): Properly quote newlines.

* lisp/eshell/em-unix.el (eshell/cat, eshell/du): Throw
'eshell-external' instead; that's what it's here for.

* test/lisp/eshell/esh-proc-tests.el (esh-proc-test-quote-argument):
Remove.
(esh-proc-test/emacs-command):
* test/lisp/eshell/esh-var-tests.el (esh-var-test/path-var/set)
(esh-var-test/path-var/set-locally): Use 'eshell-quote-argument'.

* test/lisp/eshell/em-unix-tests.el (em-unix-test/compile/interactive):
Use 'shell-quote-argument' (Note: *not* 'eshell-...').
This commit is contained in:
Jim Porter 2024-06-09 15:21:08 -07:00
parent 12d44fe642
commit 32a75ecc73
5 changed files with 33 additions and 35 deletions

View file

@ -654,8 +654,7 @@ symlink, then revert to the system's definition of cat."
(throw 'special t)))))
(let ((ext-cat (eshell-search-path "cat")))
(if ext-cat
(throw 'eshell-replace-command
(eshell-parse-command (eshell-quote-argument ext-cat) args))
(throw 'eshell-external (eshell-external-command ext-cat args))
(if eshell-in-pipeline-p
(error "Eshell's `cat' does not work in pipelines")
(error "Eshell's `cat' cannot display one of the files given"))))
@ -921,8 +920,7 @@ external command."
(if (string-equal
(file-remote-p (expand-file-name arg) 'method) "ftp")
(throw 'have-ange-path t))))))
(throw 'eshell-replace-command
(eshell-parse-command (eshell-quote-argument ext-du) args))
(throw 'eshell-external (eshell-external-command ext-du args))
(eshell-eval-using-options
"du" args
'((?a "all" nil show-all

View file

@ -351,7 +351,8 @@ argument list in place of the value of the current argument."
(defun eshell-quote-argument (string)
"Return STRING with magic characters quoted.
Magic characters are those in `eshell-special-chars-outside-quoting'."
Magic characters are those in `eshell-special-chars-outside-quoting'.
For consistent results, only call this function within an Eshell buffer."
(let ((index 0))
(mapconcat (lambda (c)
(prog1
@ -448,12 +449,15 @@ Point is left at the end of the arguments."
(defun eshell-quote-backslash (string &optional index)
"Intelligently backslash the character occurring in STRING at INDEX.
If the character is itself a backslash, it needs no escaping."
If the character is itself a backslash, it needs no escaping. If the
character is a newline, quote it using single-quotes."
(let ((char (aref string index)))
(if (eq char ?\\)
(char-to-string char)
(if (memq char eshell-special-chars-outside-quoting)
(string ?\\ char)))))
(cond ((eq char ?\\)
(char-to-string char))
((eq char ?\n)
"'\n'")
((memq char eshell-special-chars-outside-quoting)
(string ?\\ char)))))
(defun eshell-parse-backslash ()
"Parse a single backslash (\\) character and the character after.

View file

@ -43,12 +43,9 @@
"#<buffer \\*compilation\\*>")
(with-current-buffer "*compilation*"
(forward-line 3)
(should (looking-at
;; MS-Windows/DOS quote by unconditionally enclosing in
;; double quotes.
(if (memq system-type '(windows-nt ms-dos))
"\"echo\" \"hello\""
"echo hello"))))))
(should (looking-at (regexp-quote
(mapconcat #'shell-quote-argument
'("echo" "hello") " ")))))))
(ert-deftest em-unix-test/compile/noninteractive ()
"Check that `eshell/compile' writes to stdout noninteractively."

View file

@ -196,15 +196,11 @@ pipeline."
;; against; that way, users don't need to have GNU coreutils (or
;; similar) installed.
;; This is needed because system shell quoting semantics is not relevant
;; when Eshell is the shell.
(defun esh-proc-test-quote-argument (argument)
"Quote ARGUMENT using Posix semantics."
(shell-quote-argument argument t))
(defsubst esh-proc-test/emacs-command (command)
"Evaluate COMMAND in a new Emacs batch instance."
(mapconcat #'esh-proc-test-quote-argument
;; Call `eshell-quote-argument' from within an Eshell buffer since its
;; behavior depends on activating various Eshell modules.
(mapconcat (lambda (arg) (with-temp-eshell (eshell-quote-argument arg)))
`(,(expand-file-name invocation-name invocation-directory)
"-Q" "--batch" "--eval" ,(prin1-to-string command))
" "))

View file

@ -855,24 +855,27 @@ the value of the $PAGER env var."
(let* ((path-to-set-list '("/some/path" "/other/path"))
(path-to-set (string-join path-to-set-list (path-separator))))
(with-temp-eshell
;; Quote PATH value, because on Windows path-separator is ';'.
(eshell-match-command-output (concat "set PATH \"" path-to-set "\"")
(concat path-to-set "\n"))
(eshell-match-command-output "echo $PATH" (concat path-to-set "\n"))
(should (equal (eshell-get-path t) path-to-set-list)))))
;; Quote PATH value, because on Windows path-separator is ';'.
(eshell-match-command-output
(concat "set PATH " (eshell-quote-argument path-to-set) "")
(concat path-to-set "\n"))
(eshell-match-command-output "echo $PATH" (concat path-to-set "\n"))
(should (equal (eshell-get-path t) path-to-set-list)))))
(ert-deftest esh-var-test/path-var/set-locally ()
"Test setting $PATH temporarily for a single command."
(let* ((path-to-set-list '("/some/path" "/other/path"))
(path-to-set (string-join path-to-set-list (path-separator))))
(with-temp-eshell
(eshell-match-command-output (concat "set PATH \"" path-to-set "\"")
(concat path-to-set "\n"))
(eshell-match-command-output "PATH=/local/path env"
"PATH=/local/path\n")
;; After the last command, the previous $PATH value should be restored.
(eshell-match-command-output "echo $PATH" (concat path-to-set "\n"))
(should (equal (eshell-get-path t) path-to-set-list)))))
;; As above, quote PATH value.
(eshell-match-command-output
(concat "set PATH " (eshell-quote-argument path-to-set) "")
(concat path-to-set "\n"))
(eshell-match-command-output "PATH=/local/path env"
"PATH=/local/path\n")
;; After the last command, the previous $PATH value should be restored.
(eshell-match-command-output "echo $PATH" (concat path-to-set "\n"))
(should (equal (eshell-get-path t) path-to-set-list)))))
(ert-deftest esh-var-test/path-var/preserve-across-hosts ()
"Test that $PATH can be set independently on multiple hosts."