bug#60845: 30.0.50; [PATCH] Add tests for Eshell interactive completion (and fix a bug in it)

[Jim Porter <jporterbugs@gmail.com>]

[-- 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