unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#60845: 30.0.50; [PATCH] Add tests for Eshell interactive completion (and fix a bug in it)
@ 2023-01-16  1:50 Jim Porter
  2023-01-22 21:34 ` Jim Porter
  0 siblings, 1 reply; 11+ messages in thread
From: Jim Porter @ 2023-01-16  1:50 UTC (permalink / raw)
  To: 60845

[-- Attachment #1: Type: text/plain, Size: 638 bytes --]

This is a followup from bug#60464 and friends. Over there, we discussed 
problems with Pcomplete when used from Eshell, namely that Eshell 
sometimes gives Pcomplete non-string arguments. I mentioned that we 
should probably have automated tests for the Eshell side so that we can 
make further improvements to Pcomplete without causing regressions, so 
here are some tests.

I also fixed an edge case in 'eshell-complete-parse-arguments' where it 
wasn't correctly handling the new variable-splicing syntax in Eshell. 
That's patch 0002.

Of course, these tests are just a start, and there are probably lots of 
others that we could add.

[-- Attachment #2: 0001-Add-regression-tests-for-Eshell-completions.patch --]
[-- Type: text/plain, Size: 6979 bytes --]

From bdef37f6cc101820f060c718f4016a29206c51d1 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 15 Jan 2023 16:44:17 -0800
Subject: [PATCH 1/2] Add regression tests for Eshell completions

* lisp/eshell/esh-cmd.el (eshell-complete-lisp-symbols): Fix
docstring.

* test/lisp/eshell/em-cmpl-tests.el: New file.
---
 lisp/eshell/esh-cmd.el            |   2 +-
 test/lisp/eshell/em-cmpl-tests.el | 147 ++++++++++++++++++++++++++++++
 2 files changed, 148 insertions(+), 1 deletion(-)
 create mode 100644 test/lisp/eshell/em-cmpl-tests.el

diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index 99c3d7f627d..b5f1d60ff18 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -343,7 +343,7 @@ eshell-cmd-initialize
 	      #'eshell-complete-lisp-symbols nil t)))
 
 (defun eshell-complete-lisp-symbols ()
-  "If there is a user reference, complete it."
+  "If there is a Lisp symbol, complete it."
   (let ((arg (pcomplete-actual-arg)))
     (when (string-match (concat "\\`" eshell-lisp-regexp) arg)
       (setq pcomplete-stub (substring arg (match-end 0))
diff --git a/test/lisp/eshell/em-cmpl-tests.el b/test/lisp/eshell/em-cmpl-tests.el
new file mode 100644
index 00000000000..837af9d1061
--- /dev/null
+++ b/test/lisp/eshell/em-cmpl-tests.el
@@ -0,0 +1,147 @@
+;;; em-cmpl-tests.el --- em-cmpl test suite  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2023 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; Tests for Eshell's interactive completion.
+
+;;; Code:
+
+(require 'ert)
+(require 'eshell)
+(require 'em-cmpl)
+(require 'em-dirs)
+(require 'em-hist)
+(require 'em-tramp)
+(require 'em-unix)
+
+(require 'eshell-tests-helpers
+         (expand-file-name "eshell-tests-helpers"
+                           (file-name-directory (or load-file-name
+                                                    default-directory))))
+
+(defvar eshell-test-value nil)
+
+(defun eshell-insert-and-complete (input)
+  "Insert INPUT and invoke completion, returning the result."
+  (insert input)
+  (completion-at-point)
+  (eshell-get-old-input))
+
+;;; Tests:
+
+(ert-deftest em-cmpl-test/parse-arguments/pipeline ()
+  "Test that parsing arguments for completion discards earlier commands."
+  (with-temp-eshell
+   (let ((eshell-test-value '("foo" "bar")))
+     (insert "echo hi | cat")
+     (should (equal (car (eshell-complete-parse-arguments))
+                    `("cat"))))))
+
+(ert-deftest em-cmpl-test/parse-arguments/variable ()
+  "Test parsing arguments with a 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")))))))
+
+(ert-deftest em-cmpl-test/file-completion/unique ()
+  "Test completion of file names when there's a unique result."
+  (with-temp-eshell
+   (ert-with-temp-directory default-directory
+     (write-region nil nil (expand-file-name "file.txt"))
+     (should (equal (eshell-insert-and-complete "echo fi")
+                    "echo file.txt ")))))
+
+(ert-deftest em-cmpl-test/file-completion/non-unique ()
+  "Test completion of file names when there are multiple results."
+  (with-temp-eshell
+   (ert-with-temp-directory default-directory
+     (write-region nil nil (expand-file-name "file.txt"))
+     (write-region nil nil (expand-file-name "file.el"))
+     (should (equal (eshell-insert-and-complete "echo fi")
+                    "echo file."))
+     ;; Now try completing again.
+     (let ((minibuffer-message-timeout 0)
+           (inhibit-message t))
+       (completion-at-point))
+     ;; FIXME: We can't use `current-message' here.
+     (with-current-buffer (messages-buffer)
+       (save-excursion
+         (goto-char (point-max))
+         (forward-line -1)
+         (should (looking-at "Complete, but not unique")))))))
+
+(ert-deftest em-cmpl-test/file-completion/after-list ()
+  "Test completion of file names after previous list arguments.
+See bug#59956."
+  (with-temp-eshell
+   (ert-with-temp-directory default-directory
+     (write-region nil nil (expand-file-name "file.txt"))
+     (should (equal (eshell-insert-and-complete "echo (list 1 2) fi")
+                    "echo (list 1 2) file.txt ")))))
+
+(ert-deftest em-cmpl-test/lisp-symbol-completion ()
+  "Test completion of Lisp forms like \"#'symbol\" and \"`symbol\".
+See <lisp/eshell/esh-cmd.el>."
+  (with-temp-eshell
+   (should (equal (eshell-insert-and-complete "echo #'system-nam")
+                  "echo #'system-name ")))
+  (with-temp-eshell
+   (should (equal (eshell-insert-and-complete "echo `system-nam")
+                  "echo `system-name "))))
+
+(ert-deftest em-cmpl-test/lisp-function-completion ()
+  "Test completion of Lisp forms like \"(func)\".
+See <lisp/eshell/esh-cmd.el>."
+  (with-temp-eshell
+   (should (equal (eshell-insert-and-complete "echo (eshell/ech")
+                  "echo (eshell/echo"))))
+
+(ert-deftest em-cmpl-test/variable-ref-completion ()
+  "Test completion of variable references like \"$var\".
+See <lisp/eshell/esh-var.el>."
+  (with-temp-eshell
+   (should (equal (eshell-insert-and-complete "echo $system-nam")
+                  "echo $system-name "))))
+
+(ert-deftest em-cmpl-test/variable-assign-completion ()
+  "Test completion of variable assignments like \"var=value\".
+See <lisp/eshell/esh-var.el>."
+  (with-temp-eshell
+   (ert-with-temp-directory default-directory
+     (write-region nil nil (expand-file-name "file.txt"))
+     (should (equal (eshell-insert-and-complete "VAR=f")
+                    "VAR=file.txt ")))))
+
+(ert-deftest em-cmpl-test/user-ref-completion ()
+  "Test completeion of user references like \"~user\".
+See <lisp/eshell/em-dirs.el>."
+  (unwind-protect
+      (with-temp-eshell
+       (cl-letf (((symbol-function 'eshell-read-user-names)
+                  (lambda () (setq eshell-user-names '((1234 . "user"))))))
+         ;; FIXME: Should this really add a space at the end?
+         (should (equal (eshell-insert-and-complete "echo ~us")
+                        "echo ~user/ "))))
+    ;; Clear the cached user names we set above.
+    (setq eshell-user-names nil)))
+
+;;; em-cmpl-tests.el ends here
-- 
2.25.1


[-- Attachment #3: 0002-Properly-parse-Eshell-variable-splices-for-interacti.patch --]
[-- Type: text/plain, Size: 5021 bytes --]

From 39fe1f15f0804e608891f158afc49e806f969e98 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 15 Jan 2023 16:44:23 -0800
Subject: [PATCH 2/2] Properly parse Eshell variable splices for interactive
 completion

Previously, the code simply ignored the splice operator, which usually
worked, but isn't actually correct.

* lisp/eshell/em-cmpl.el (eshell-complete-eval-argument-form): New
function.
(eshell-complete-parse-arguments): Properly parse variable splices.

* test/lisp/eshell/em-cmpl-tests.el
(em-cmpl-test/parse-arguments/variable-splice): New test.
---
 lisp/eshell/em-cmpl.el            | 56 ++++++++++++++++++++-----------
 test/lisp/eshell/em-cmpl-tests.el |  8 +++++
 2 files changed, 44 insertions(+), 20 deletions(-)

diff --git a/lisp/eshell/em-cmpl.el b/lisp/eshell/em-cmpl.el
index 4206ad048fa..a67fe53ed46 100644
--- a/lisp/eshell/em-cmpl.el
+++ b/lisp/eshell/em-cmpl.el
@@ -306,6 +306,12 @@ eshell--pcomplete-insert-tab
     (insert-and-inherit "\t")
     (throw 'pcompleted t)))
 
+(defun eshell-complete-eval-argument-form (arg)
+  "Evaluate a single Eshell argument form ARG for the purposes of completion."
+  (let ((result (eshell-do-eval `(eshell-commands ,arg) t)))
+    (cl-assert (eq (car result) 'quote))
+    (cadr result)))
+
 (defun eshell-complete-parse-arguments ()
   "Parse the command line arguments for `pcomplete-argument'."
   (when (and eshell-no-completion-during-jobs
@@ -344,11 +350,6 @@ eshell-complete-parse-arguments
     (cl-assert (= (length args) (length posns)))
     (let ((a args) (i 0) new-start)
       (while a
-        ;; Remove any top-level `eshell-splice-args' sigils.  These
-        ;; are meant to be rewritten and can't actually be called.
-        (when (and (consp (car a))
-                   (eq (caar a) 'eshell-splice-args))
-          (setcar a (cadar a)))
         ;; If there's an unreplaced `eshell-operator' sigil, consider
         ;; the token after it the new start of our arguments.
         (when (and (consp (car a))
@@ -364,23 +365,38 @@ eshell-complete-parse-arguments
 	       (not (eq (char-before (1- end)) ?\\)))
       (nconc args (list ""))
       (nconc posns (list (point))))
+    ;; Evaluate and expand Eshell forms.
+    (let (evaled-args evaled-posns)
+      (cl-mapc
+       (lambda (arg posn)
+         (pcase arg
+           (`(eshell-splice-args ,val)
+            (dolist (subarg (eshell-complete-eval-argument-form val))
+              (push subarg evaled-args)
+              (push posn evaled-posns)))
+           ((pred listp)
+            (push (eshell-complete-eval-argument-form arg) evaled-args)
+            (push posn evaled-posns))
+           (_
+            (push arg evaled-args)
+            (push posn evaled-posns))))
+       args posns)
+      (setq args (nreverse evaled-args)
+            posns (nreverse evaled-posns)))
+    ;; Convert arguments to forms that Pcomplete can understand.
     (cons (mapcar
            (lambda (arg)
-             (let ((val
-                    (if (listp arg)
-                        (let ((result
-                               (eshell-do-eval
-                                (list 'eshell-commands arg) t)))
-                          (cl-assert (eq (car result) 'quote))
-                          (cadr result))
-                      arg)))
-               (cond ((numberp val)
-                      (setq val (number-to-string val)))
-                     ;; expand .../ etc that only eshell understands to
-                     ;; standard ../../
-                     ((and (stringp val)) (string-match "\\.\\.\\.+/" val)
-                      (setq val (eshell-expand-multiple-dots val))))
-               (or val "")))
+             (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)))
 	   args)
 	  posns)))
 
diff --git a/test/lisp/eshell/em-cmpl-tests.el b/test/lisp/eshell/em-cmpl-tests.el
index 837af9d1061..2460f58c25c 100644
--- a/test/lisp/eshell/em-cmpl-tests.el
+++ b/test/lisp/eshell/em-cmpl-tests.el
@@ -62,6 +62,14 @@ em-cmpl-test/parse-arguments/variable
      (should (equal (car (eshell-complete-parse-arguments))
                     `("echo" ("foo" "bar")))))))
 
+(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"))))))
+
 (ert-deftest em-cmpl-test/file-completion/unique ()
   "Test completion of file names when there's a unique result."
   (with-temp-eshell
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-10-10 21:23 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-16  1:50 bug#60845: 30.0.50; [PATCH] Add tests for Eshell interactive completion (and fix a bug in it) Jim Porter
2023-01-22 21:34 ` Jim Porter
2023-01-22 21:35   ` Jim Porter
2023-01-30  7:14     ` Jim Porter
2023-01-30 14:54       ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-01-31  2:00         ` Jim Porter
2023-09-05 23:36           ` Stefan Kangas
2023-09-06  0:47             ` Jim Porter
2023-09-06  1:37               ` Jim Porter
2023-10-10 20:07                 ` Jim Porter
2023-10-10 21:23                   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors

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).