* 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
* bug#60845: 30.0.50; [PATCH] Add tests for Eshell interactive completion (and fix a bug in it)
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
0 siblings, 1 reply; 11+ messages in thread
From: Jim Porter @ 2023-01-22 21:34 UTC (permalink / raw)
To: 60845; +Cc: gregory
X-Debbugs-CC: gregory@heytings.org
On 1/15/2023 5:50 PM, Jim Porter wrote:
> 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.
After studying how Eshell interfaces with Pcomplete, I think I've
convinced myself that Eshell should be the one responsible for
converting all its arguments to strings. It already does this for some
cases (nil becomes "", numbers are stringified, and ".../" forms become
"../../"), so the fact that it doesn't do this universally is probably a
bug.
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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#60845: 30.0.50; [PATCH] Add tests for Eshell interactive completion (and fix a bug in it)
2023-01-22 21:34 ` Jim Porter
@ 2023-01-22 21:35 ` Jim Porter
2023-01-30 7:14 ` Jim Porter
0 siblings, 1 reply; 11+ messages in thread
From: Jim Porter @ 2023-01-22 21:35 UTC (permalink / raw)
To: 60845
[-- 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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* bug#60845: 30.0.50; [PATCH] Add tests for Eshell interactive completion (and fix a bug in it)
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
0 siblings, 1 reply; 11+ messages in thread
From: Jim Porter @ 2023-01-30 7:14 UTC (permalink / raw)
To: 60845; +Cc: monnier
X-Debbugs-Cc: monnier@iro.umontreal.ca
On 1/22/2023 1:35 PM, Jim Porter wrote:
> 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.
Stefan, do you have any thoughts on these changes, especially the third
patch? I did a bit of tinkering with pcomplete functions, and I think it
should be ok for the Eshell side of things. Eshell-specific pcomplete
functions can get the real value if they need by examining the
'pcomplete-arg-value' string property.
For pcomplete functions that aren't Eshell-specific, they'll always get
the string form of arguments by default, so things should Just Work there.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#60845: 30.0.50; [PATCH] Add tests for Eshell interactive completion (and fix a bug in it)
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
0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-01-30 14:54 UTC (permalink / raw)
To: Jim Porter; +Cc: 60845
>>> 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.
>
> Stefan, do you have any thoughts on these changes, especially the third
> patch? I did a bit of tinkering with pcomplete functions, and I think it
> should be ok for the Eshell side of things. Eshell-specific pcomplete
> functions can get the real value if they need by examining the
> 'pcomplete-arg-value' string property.
>
> For pcomplete functions that aren't Eshell-specific, they'll always get the
> string form of arguments by default, so things should Just Work there.
It sounds good to me, but I'm definitely not well versed in this aspect
of the interaction between Eshell and Pcomplete (more specifically,
this is a part of their interaction which I find quite tricky), so it's
good that you add corresponding regression tests.
Stefan
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#60845: 30.0.50; [PATCH] Add tests for Eshell interactive completion (and fix a bug in it)
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
0 siblings, 1 reply; 11+ messages in thread
From: Jim Porter @ 2023-01-31 2:00 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 60845
On 1/30/2023 6:54 AM, Stefan Monnier via Bug reports for GNU Emacs, the
Swiss army knife of text editors wrote:
> It sounds good to me, but I'm definitely not well versed in this aspect
> of the interaction between Eshell and Pcomplete (more specifically,
> this is a part of their interaction which I find quite tricky), so it's
> good that you add corresponding regression tests.
Thanks for taking a look. I've merged my patches as e7d0aa248e. We can
leave this open though to discuss what to do about the Pcomplete side of
things. I think we can remove the workaround for Emacs 29, but maybe we
want some additional changes.
(It would also be nice to add some more Pcomplete support on the Eshell
side. For example, when completing arguments to 'echo', Pcomplete shows
options for the real /bin/echo, instead of showing options for the
built-in function 'eshell/echo'.)
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#60845: 30.0.50; [PATCH] Add tests for Eshell interactive completion (and fix a bug in it)
2023-01-31 2:00 ` Jim Porter
@ 2023-09-05 23:36 ` Stefan Kangas
2023-09-06 0:47 ` Jim Porter
0 siblings, 1 reply; 11+ messages in thread
From: Stefan Kangas @ 2023-09-05 23:36 UTC (permalink / raw)
To: Jim Porter; +Cc: 60845, Stefan Monnier
Jim Porter <jporterbugs@gmail.com> writes:
> On 1/30/2023 6:54 AM, Stefan Monnier via Bug reports for GNU Emacs, the Swiss
> army knife of text editors wrote:
>> It sounds good to me, but I'm definitely not well versed in this aspect
>> of the interaction between Eshell and Pcomplete (more specifically,
>> this is a part of their interaction which I find quite tricky), so it's
>> good that you add corresponding regression tests.
>
> Thanks for taking a look. I've merged my patches as e7d0aa248e. We can leave
> this open though to discuss what to do about the Pcomplete side of things. I
> think we can remove the workaround for Emacs 29, but maybe we want some
> additional changes.
That was 9 months ago. Is it still relevant to keep this bug open?
> (It would also be nice to add some more Pcomplete support on the Eshell
> side. For example, when completing arguments to 'echo', Pcomplete shows options
> for the real /bin/echo, instead of showing options for the built-in function
> 'eshell/echo'.)
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#60845: 30.0.50; [PATCH] Add tests for Eshell interactive completion (and fix a bug in it)
2023-09-05 23:36 ` Stefan Kangas
@ 2023-09-06 0:47 ` Jim Porter
2023-09-06 1:37 ` Jim Porter
0 siblings, 1 reply; 11+ messages in thread
From: Jim Porter @ 2023-09-06 0:47 UTC (permalink / raw)
To: Stefan Kangas; +Cc: 60845, Stefan Monnier
On 9/5/2023 4:36 PM, Stefan Kangas wrote:
> Jim Porter <jporterbugs@gmail.com> writes:
>
>> On 1/30/2023 6:54 AM, Stefan Monnier via Bug reports for GNU Emacs, the Swiss
>> army knife of text editors wrote:
>>> It sounds good to me, but I'm definitely not well versed in this aspect
>>> of the interaction between Eshell and Pcomplete (more specifically,
>>> this is a part of their interaction which I find quite tricky), so it's
>>> good that you add corresponding regression tests.
>>
>> Thanks for taking a look. I've merged my patches as e7d0aa248e. We can leave
>> this open though to discuss what to do about the Pcomplete side of things. I
>> think we can remove the workaround for Emacs 29, but maybe we want some
>> additional changes.
>
> That was 9 months ago. Is it still relevant to keep this bug open?
Yes, I believe so. I was planning to wait until Emacs 29.1 was released
before pinging people on this, but then forgot all about it. We should
probably use this time to fix the FIXME in 'pcomplete-arg', since (I
think) the current behavior in Eshell no longer requires the FIXME bit:
;; FIXME: 'last' is handled specially in Emacs 29, because
;; 'pcomplete-parse-arguments' accepts a list of strings
;; (which are completion candidates) as return value for
;; (pcomplete-arg 'last). See below: "it means it's a
;; list of completions computed during parsing,
;; e.g. Eshell uses that to turn globs into lists of
;; completions". This special case will be dealt with
;; differently in Emacs 30: the pcomplete-arg-value
;; property will be used by 'pcomplete-parse-arguments'.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#60845: 30.0.50; [PATCH] Add tests for Eshell interactive completion (and fix a bug in it)
2023-09-06 0:47 ` Jim Porter
@ 2023-09-06 1:37 ` Jim Porter
2023-10-10 20:07 ` Jim Porter
0 siblings, 1 reply; 11+ messages in thread
From: Jim Porter @ 2023-09-06 1:37 UTC (permalink / raw)
To: Stefan Kangas
Cc: 60845, Daniel Mendler, Gregory Heytings, arstoffel,
Stefan Monnier
[-- Attachment #1: Type: text/plain, Size: 2253 bytes --]
On 9/5/2023 5:47 PM, Jim Porter wrote:
> On 9/5/2023 4:36 PM, Stefan Kangas wrote:
>> Jim Porter <jporterbugs@gmail.com> writes:
>>
>>> On 1/30/2023 6:54 AM, Stefan Monnier via Bug reports for GNU Emacs,
>>> the Swiss
>>> army knife of text editors wrote:
>>>> It sounds good to me, but I'm definitely not well versed in this aspect
>>>> of the interaction between Eshell and Pcomplete (more specifically,
>>>> this is a part of their interaction which I find quite tricky), so it's
>>>> good that you add corresponding regression tests.
>>>
>>> Thanks for taking a look. I've merged my patches as e7d0aa248e. We
>>> can leave
>>> this open though to discuss what to do about the Pcomplete side of
>>> things. I
>>> think we can remove the workaround for Emacs 29, but maybe we want some
>>> additional changes.
>>
>> That was 9 months ago. Is it still relevant to keep this bug open?
>
> Yes, I believe so. I was planning to wait until Emacs 29.1 was released
> before pinging people on this, but then forgot all about it. We should
> probably use this time to fix the FIXME in 'pcomplete-arg', since (I
> think) the current behavior in Eshell no longer requires the FIXME bit:
>
> ;; FIXME: 'last' is handled specially in Emacs 29, because
> ;; 'pcomplete-parse-arguments' accepts a list of strings
> ;; (which are completion candidates) as return value for
> ;; (pcomplete-arg 'last). See below: "it means it's a
> ;; list of completions computed during parsing,
> ;; e.g. Eshell uses that to turn globs into lists of
> ;; completions". This special case will be dealt with
> ;; differently in Emacs 30: the pcomplete-arg-value
> ;; property will be used by 'pcomplete-parse-arguments'.
Attached is a patch to revert the Emacs 29 workarounds. I *believe* I've
fixed this on the Eshell side by always providing Pcomplete with the
arguments in their string form. Could everyone try the patch out to make
sure things still work?
In particular, see the cases in the following bugs: bug#60464,
bug#60021, and bug#59956.
[-- Attachment #2: 0001-Revert-commits-dafa6d6badd6-and-72c45fa9109a.patch --]
[-- Type: text/plain, Size: 2771 bytes --]
From b07b5a37bc0f494a4a93c9a54ad7e302ac74c93d Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Tue, 5 Sep 2023 18:27:21 -0700
Subject: [PATCH] Revert commits dafa6d6badd6 and 72c45fa9109a
These were there to work around deficiencies in how Eshell produces
completions for 'pcomplete-argument' (Eshell passed various non-string
objects to Pcomplete, which broke things). Now, Eshell always returns
a stringified form of the argument, with the original value stored via
the text property 'pcomplete-arg-value'.
* lisp/pcomplete.el (pcomplete-arg): Revert changes back to a simpler
form.
---
lisp/pcomplete.el | 36 +++++++-----------------------------
1 file changed, 7 insertions(+), 29 deletions(-)
diff --git a/lisp/pcomplete.el b/lisp/pcomplete.el
index 151611f94b7..ac8edcff9f1 100644
--- a/lisp/pcomplete.el
+++ b/lisp/pcomplete.el
@@ -675,35 +675,13 @@ pcomplete-arg
The OFFSET argument is added to/taken away from the index that will be
used. This is really only useful with `first' and `last', for
-accessing absolute argument positions.
-
-When the argument has been transformed into something that is not
-a string by `pcomplete-parse-arguments-function', the text
-representation of the argument, namely what the user actually
-typed in, is returned, and the value of the argument is stored in
-the pcomplete-arg-value text property of that string."
- (let ((arg
- (nth (+ (pcase index
- ('first 0)
- ('last pcomplete-last)
- (_ (- pcomplete-index (or index 0))))
- (or offset 0))
- pcomplete-args)))
- (if (or (stringp arg)
- ;; FIXME: 'last' is handled specially in Emacs 29, because
- ;; 'pcomplete-parse-arguments' accepts a list of strings
- ;; (which are completion candidates) as return value for
- ;; (pcomplete-arg 'last). See below: "it means it's a
- ;; list of completions computed during parsing,
- ;; e.g. Eshell uses that to turn globs into lists of
- ;; completions". This special case will be dealt with
- ;; differently in Emacs 30: the pcomplete-arg-value
- ;; property will be used by 'pcomplete-parse-arguments'.
- (eq index 'last))
- arg
- (propertize
- (car (split-string (pcomplete-actual-arg index offset)))
- 'pcomplete-arg-value arg))))
+accessing absolute argument positions."
+ (nth (+ (pcase index
+ ('first 0)
+ ('last pcomplete-last)
+ (_ (- pcomplete-index (or index 0))))
+ (or offset 0))
+ pcomplete-args))
(defun pcomplete-begin (&optional index offset)
"Return the beginning position of the INDEXth argument.
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* bug#60845: 30.0.50; [PATCH] Add tests for Eshell interactive completion (and fix a bug in it)
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
0 siblings, 1 reply; 11+ messages in thread
From: Jim Porter @ 2023-10-10 20:07 UTC (permalink / raw)
To: Stefan Kangas
Cc: 60845-done, Daniel Mendler, Gregory Heytings, arstoffel,
Stefan Monnier
On 9/5/2023 6:37 PM, Jim Porter wrote:
> Attached is a patch to revert the Emacs 29 workarounds. I *believe* I've
> fixed this on the Eshell side by always providing Pcomplete with the
> arguments in their string form. Could everyone try the patch out to make
> sure things still work?
>
> In particular, see the cases in the following bugs: bug#60464,
> bug#60021, and bug#59956.
Since there have been no further comments in the last month, I've now
merged my patch to the master branch as 239db5d5162. If anyone sees any
issues resulting from this, feel free to back it out, file a bug, and/or
let me know directly.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#60845: 30.0.50; [PATCH] Add tests for Eshell interactive completion (and fix a bug in it)
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
0 siblings, 0 replies; 11+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-10-10 21:23 UTC (permalink / raw)
To: Jim Porter
Cc: 60845-done, Daniel Mendler, Gregory Heytings, arstoffel,
Stefan Kangas
>> Attached is a patch to revert the Emacs 29 workarounds. I *believe* I've
>> fixed this on the Eshell side by always providing Pcomplete with the
>> arguments in their string form. Could everyone try the patch out to make
>> sure things still work?
>> In particular, see the cases in the following bugs: bug#60464, bug#60021,
>> and bug#59956.
>
> Since there have been no further comments in the last month, I've now merged
> my patch to the master branch as 239db5d5162. If anyone sees any issues
> resulting from this, feel free to back it out, file a bug, and/or let me
> know directly.
Thanks Jim,
Stefan
^ permalink raw reply [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 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.