master e7d0aa248e6 3/3: During completion, convert all Eshell arguments to strings

[Jim Porter]

branch: master
commit e7d0aa248e684a6de0d655d93bfcfee06cc8ff09
Author: Jim Porter <jporterbugs@gmail.com>
Commit: Jim Porter <jporterbugs@gmail.com>

    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 d1c7e81090a..acbf206a3c6 100644
--- a/lisp/eshell/em-cmpl.el
+++ b/lisp/eshell/em-cmpl.el
@@ -386,17 +386,19 @@ to writing a completion function."
     ;; 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 @@
   (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 @@
   (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."