From: Jim Porter <jporterbugs@gmail.com>
To: 60845@debbugs.gnu.org
Subject: bug#60845: 30.0.50; [PATCH] Add tests for Eshell interactive completion (and fix a bug in it)
Date: Sun, 22 Jan 2023 13:35:33 -0800 [thread overview]
Message-ID: <40d31b3d-d429-5a48-02bc-a3c1e4a1d903@gmail.com> (raw)
In-Reply-To: <bbea0f18-f5d7-163d-a5d0-5d9a75a5894a@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 235 bytes --]
On 1/22/2023 1:34 PM, Jim Porter wrote:
> The third patch in this series fixes this for the Eshell side, so we
> could probably remove the workaround in pcomplete.el for Emacs 30.
... It would help if I actually attached the patches.
[-- Attachment #2: 0001-Add-regression-tests-for-Eshell-completions.patch --]
[-- Type: text/plain, Size: 7927 bytes --]
From aaefe4aed6276b2af219b54a9d16106a986ec9eb 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/3] 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 | 170 ++++++++++++++++++++++++++++++
2 files changed, 171 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..32b0781dd75
--- /dev/null
+++ b/test/lisp/eshell/em-cmpl-tests.el
@@ -0,0 +1,170 @@
+;;; 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/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")))))
+
+(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"))))))
+
+(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" ""))))))
+
+(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")))))))
+
+(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: 5026 bytes --]
From 1e2bb0dec06eb0218496f87795ab2b7507492cfc 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/3] 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 32b0781dd75..3f8f890f6e5 100644
--- a/test/lisp/eshell/em-cmpl-tests.el
+++ b/test/lisp/eshell/em-cmpl-tests.el
@@ -85,6 +85,14 @@ em-cmpl-test/parse-arguments/variable/list
(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
[-- Attachment #4: 0003-During-completion-convert-all-Eshell-arguments-to-st.patch --]
[-- Type: text/plain, Size: 6681 bytes --]
From 81d8575740b12bfac27c7db1c813fbfa5586dd9d Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
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
next prev parent reply other threads:[~2023-01-22 21:35 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=40d31b3d-d429-5a48-02bc-a3c1e4a1d903@gmail.com \
--to=jporterbugs@gmail.com \
--cc=60845@debbugs.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.