From 81d8575740b12bfac27c7db1c813fbfa5586dd9d Mon Sep 17 00:00:00 2001 From: Jim Porter Date: Sun, 22 Jan 2023 13:20:46 -0800 Subject: [PATCH 3/3] During completion, convert all Eshell arguments to strings Eshell was already converting some types (numbers, nil) to strings, as well as fixing up multiple-dot forms like ".../", so this way is more consistent and should produce fewer problems for Pcomplete functions. * lisp/eshell/em-cmpl.el (eshell-complete-parse-arguments): Always convert parsed arguments to strings. * test/lisp/eshell/em-cmpl-tests.el (eshell-arguments-equal, eshell-arguments-equal--equal-explainer): New functions. (em-cmpl-test/parse-arguments/pipeline) (em-cmpl-test/parse-arguments/multiple-dots) (em-cmpl-test/parse-arguments/variable/numeric) (em-cmpl-test/parse-arguments/variable/nil) (em-cmpl-test/parse-arguments/variable/list) (em-cmpl-test/parse-arguments/variable/splice): Use 'eshell-arguments-equal'. --- lisp/eshell/em-cmpl.el | 24 +++++++------- test/lisp/eshell/em-cmpl-tests.el | 54 ++++++++++++++++++++++++------- 2 files changed, 55 insertions(+), 23 deletions(-) diff --git a/lisp/eshell/em-cmpl.el b/lisp/eshell/em-cmpl.el index a67fe53ed46..f3a6064a483 100644 --- a/lisp/eshell/em-cmpl.el +++ b/lisp/eshell/em-cmpl.el @@ -386,17 +386,19 @@ eshell-complete-parse-arguments ;; Convert arguments to forms that Pcomplete can understand. (cons (mapcar (lambda (arg) - (cond - ((numberp arg) - (number-to-string arg)) - ;; Expand ".../" etc that only Eshell understands to the - ;; standard "../../". - ((and (stringp arg) (string-match "\\.\\.\\.+/" arg)) - (eshell-expand-multiple-dots arg)) - ((null arg) - "") - (t - arg))) + (pcase arg + ;; Expand ".../" etc that only Eshell understands to + ;; the standard "../../". + ((rx ".." (+ ".") "/") + (propertize (eshell-expand-multiple-dots arg) + 'pcomplete-arg-value arg)) + ((pred stringp) + arg) + ('nil + (propertize "" 'pcomplete-arg-value arg)) + (_ + (propertize (eshell-stringify arg) + 'pcomplete-arg-value arg)))) args) posns))) diff --git a/test/lisp/eshell/em-cmpl-tests.el b/test/lisp/eshell/em-cmpl-tests.el index 3f8f890f6e5..12a156fbb38 100644 --- a/test/lisp/eshell/em-cmpl-tests.el +++ b/test/lisp/eshell/em-cmpl-tests.el @@ -44,6 +44,26 @@ eshell-insert-and-complete (completion-at-point) (eshell-get-old-input)) +(defun eshell-arguments-equal (actual expected) + "Return t if ACTUAL and EXPECTED are equal, including properties of strings. +ACTUAL and EXPECTED should both be lists of strings." + (when (length= actual (length expected)) + (catch 'not-equal + (cl-mapc (lambda (i j) + (unless (equal-including-properties i j) + (throw 'not-equal nil))) + actual expected) + t))) + +(defun eshell-arguments-equal--equal-explainer (actual expected) + "Explain the result of `eshell-arguments-equal'." + `(nonequal-result + (actual ,actual) + (expected ,expected))) + +(put 'eshell-arguments-equal 'ert-explainer + #'eshell-arguments-equal--equal-explainer) + ;;; Tests: (ert-deftest em-cmpl-test/parse-arguments/pipeline () @@ -51,47 +71,57 @@ em-cmpl-test/parse-arguments/pipeline (with-temp-eshell (let ((eshell-test-value '("foo" "bar"))) (insert "echo hi | cat") - (should (equal (car (eshell-complete-parse-arguments)) - '("cat")))))) + (should (eshell-arguments-equal + (car (eshell-complete-parse-arguments)) + '("cat")))))) (ert-deftest em-cmpl-test/parse-arguments/multiple-dots () "Test parsing arguments with multiple dots like \".../\"." (with-temp-eshell (insert "echo .../file.txt") - (should (equal (car (eshell-complete-parse-arguments)) - '("echo" "../../file.txt"))))) + (should (eshell-arguments-equal + (car (eshell-complete-parse-arguments)) + `("echo" ,(propertize "../../file.txt" + 'pcomplete-arg-value + ".../file.txt")))))) (ert-deftest em-cmpl-test/parse-arguments/variable/numeric () "Test parsing arguments with a numeric variable interpolation." (with-temp-eshell (let ((eshell-test-value 42)) (insert "echo $eshell-test-value") - (should (equal (car (eshell-complete-parse-arguments)) - '("echo" "42")))))) + (should (eshell-arguments-equal + (car (eshell-complete-parse-arguments)) + `("echo" ,(propertize "42" 'pcomplete-arg-value 42))))))) (ert-deftest em-cmpl-test/parse-arguments/variable/nil () "Test parsing arguments with a nil variable interpolation." (with-temp-eshell (let ((eshell-test-value nil)) (insert "echo $eshell-test-value") - (should (equal (car (eshell-complete-parse-arguments)) - '("echo" "")))))) + (should (eshell-arguments-equal + (car (eshell-complete-parse-arguments)) + `("echo" ,(propertize "" 'pcomplete-arg-value nil))))))) (ert-deftest em-cmpl-test/parse-arguments/variable/list () "Test parsing arguments with a list variable interpolation." (with-temp-eshell (let ((eshell-test-value '("foo" "bar"))) (insert "echo $eshell-test-value") - (should (equal (car (eshell-complete-parse-arguments)) - '("echo" ("foo" "bar"))))))) + (should (eshell-arguments-equal + (car (eshell-complete-parse-arguments)) + `("echo" ,(propertize "(\"foo\" \"bar\")" + 'pcomplete-arg-value + eshell-test-value))))))) (ert-deftest em-cmpl-test/parse-arguments/variable/splice () "Test parsing arguments with a spliced variable interpolation." (with-temp-eshell (let ((eshell-test-value '("foo" "bar"))) (insert "echo $@eshell-test-value") - (should (equal (car (eshell-complete-parse-arguments)) - '("echo" "foo" "bar")))))) + (should (eshell-arguments-equal + (car (eshell-complete-parse-arguments)) + '("echo" "foo" "bar")))))) (ert-deftest em-cmpl-test/file-completion/unique () "Test completion of file names when there's a unique result." -- 2.25.1