* bug#59956: 29.0.60: Failure when completing arguments in Eshell after variable interpolation
@ 2022-12-11 1:25 Jim Porter
2022-12-11 7:44 ` Eli Zaretskii
0 siblings, 1 reply; 24+ messages in thread
From: Jim Porter @ 2022-12-11 1:25 UTC (permalink / raw)
To: 59956; +Cc: arstoffel
X-Debbugs-CC: arstoffel@gmail.com
Starting from "emacs -Q -f -eshell", type "echo $exec-path " (note the
trailing space), and then hit TAB. The result is this error:
pcomplete-match: Wrong type argument: stringp, ("/usr/bin" ...)
This is a regression from Emacs 28, and it looks like it's due to
'pcomplete-here-using-help' assuming that all the pcomplete args are
strings. However, 'exec-path' is a list (and Eshell reports it this way
to pcomplete), so the completion fails. I think all that's necessary is
checking that the pcomplete args are strings in
'pcomplete-here-using-help', but I know next to nothing about pcomplete...
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#59956: 29.0.60: Failure when completing arguments in Eshell after variable interpolation
2022-12-11 1:25 bug#59956: 29.0.60: Failure when completing arguments in Eshell after variable interpolation Jim Porter
@ 2022-12-11 7:44 ` Eli Zaretskii
2022-12-11 8:56 ` Augusto Stoffel
2022-12-12 22:21 ` Gregory Heytings
0 siblings, 2 replies; 24+ messages in thread
From: Eli Zaretskii @ 2022-12-11 7:44 UTC (permalink / raw)
To: Jim Porter, Stefan Monnier; +Cc: arstoffel, 59956
> Cc: arstoffel@gmail.com
> Date: Sat, 10 Dec 2022 17:25:53 -0800
> From: Jim Porter <jporterbugs@gmail.com>
>
> X-Debbugs-CC: arstoffel@gmail.com
>
> Starting from "emacs -Q -f -eshell", type "echo $exec-path " (note the
> trailing space), and then hit TAB. The result is this error:
>
> pcomplete-match: Wrong type argument: stringp, ("/usr/bin" ...)
>
> This is a regression from Emacs 28, and it looks like it's due to
> 'pcomplete-here-using-help' assuming that all the pcomplete args are
> strings. However, 'exec-path' is a list (and Eshell reports it this way
> to pcomplete), so the completion fails. I think all that's necessary is
> checking that the pcomplete args are strings in
> 'pcomplete-here-using-help', but I know next to nothing about pcomplete...
Adding Stefan, who should know more about pcomplete.
Stefan, any suggestions?
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#59956: 29.0.60: Failure when completing arguments in Eshell after variable interpolation
2022-12-11 7:44 ` Eli Zaretskii
@ 2022-12-11 8:56 ` Augusto Stoffel
2022-12-11 15:27 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-12 22:21 ` Gregory Heytings
1 sibling, 1 reply; 24+ messages in thread
From: Augusto Stoffel @ 2022-12-11 8:56 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Jim Porter, Stefan Monnier, 59956
On Sun, 11 Dec 2022 at 09:44, Eli Zaretskii wrote:
>> Cc: arstoffel@gmail.com
>> Date: Sat, 10 Dec 2022 17:25:53 -0800
>> From: Jim Porter <jporterbugs@gmail.com>
>>
>> X-Debbugs-CC: arstoffel@gmail.com
>>
>> Starting from "emacs -Q -f -eshell", type "echo $exec-path " (note the
>> trailing space), and then hit TAB. The result is this error:
>>
>> pcomplete-match: Wrong type argument: stringp, ("/usr/bin" ...)
>>
>> This is a regression from Emacs 28, and it looks like it's due to
>> 'pcomplete-here-using-help' assuming that all the pcomplete args are
>> strings. However, 'exec-path' is a list (and Eshell reports it this way
>> to pcomplete), so the completion fails. I think all that's necessary is
>> checking that the pcomplete args are strings in
>> 'pcomplete-here-using-help', but I know next to nothing about pcomplete...
>
> Adding Stefan, who should know more about pcomplete.
>
> Stefan, any suggestions?
For the record, this problem afflicts just about every pcomplete rule
(except probably those meant specifically for eshell). I just tried
`cvs $echo-path TAB` and I get the same error.
The issue here is what eshell returns via (pcomplete-arg ...) etc.
Currently it can be a list. How about changing it to the string
"$echo-path", with the list sneaked in as a text property?
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#59956: 29.0.60: Failure when completing arguments in Eshell after variable interpolation
2022-12-11 8:56 ` Augusto Stoffel
@ 2022-12-11 15:27 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 0 replies; 24+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-12-11 15:27 UTC (permalink / raw)
To: John Wiegley; +Cc: Jim Porter, Eli Zaretskii, Augusto Stoffel, 59956
Hi John,
As the original designer of both Eshell and Pcomplete, what's your
opinion on the below:
>>> Starting from "emacs -Q -f -eshell", type "echo $exec-path " (note the
>>> trailing space), and then hit TAB. The result is this error:
>>>
>>> pcomplete-match: Wrong type argument: stringp, ("/usr/bin" ...)
>>>
>>> This is a regression from Emacs 28, and it looks like it's due to
>>> 'pcomplete-here-using-help' assuming that all the pcomplete args are
>>> strings. However, 'exec-path' is a list (and Eshell reports it this way
>>> to pcomplete), so the completion fails. I think all that's necessary is
>>> checking that the pcomplete args are strings in
>>> 'pcomplete-here-using-help', but I know next to nothing about pcomplete...
>>
>> Adding Stefan, who should know more about pcomplete.
>> Stefan, any suggestions?
>
> For the record, this problem afflicts just about every pcomplete rule
> (except probably those meant specifically for eshell). I just tried
> `cvs $echo-path TAB` and I get the same error.
Indeed, it seems to be a pervasive problem.
> The issue here is what eshell returns via (pcomplete-arg ...) etc.
> Currently it can be a list. How about changing it to the string
> "$echo-path", with the list sneaked in as a text property?
From the point of view of Pcomplete, `pcomplete-arg` should indeed
always return a string, I think. But should it be the string as it
stands in the buffer or the string *represented* by what's in
the buffer (i.e. after $ expansion and such)?
Should `pcomplete-arg` return nil in a case like `echo $nil`?
For "normal" shells, this is not an issue since they don't really
manipulate anything else than strings (or so we can pretend), but
obviously things aren't that simple for Eshell.
Stefan
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#59956: 29.0.60: Failure when completing arguments in Eshell after variable interpolation
2022-12-11 7:44 ` Eli Zaretskii
2022-12-11 8:56 ` Augusto Stoffel
@ 2022-12-12 22:21 ` Gregory Heytings
2022-12-12 22:37 ` Augusto Stoffel
2022-12-16 6:09 ` Jim Porter
1 sibling, 2 replies; 24+ messages in thread
From: Gregory Heytings @ 2022-12-12 22:21 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Jim Porter, arstoffel, Stefan Monnier, 59956
[-- Attachment #1: Type: text/plain, Size: 801 bytes --]
>> Starting from "emacs -Q -f -eshell", type "echo $exec-path " (note the
>> trailing space), and then hit TAB. The result is this error:
>>
>> pcomplete-match: Wrong type argument: stringp, ("/usr/bin" ...)
>>
>> This is a regression from Emacs 28, and it looks like it's due to
>> 'pcomplete-here-using-help' assuming that all the pcomplete args are
>> strings. However, 'exec-path' is a list (and Eshell reports it this way
>> to pcomplete), so the completion fails. I think all that's necessary is
>> checking that the pcomplete args are strings in
>> 'pcomplete-here-using-help', but I know next to nothing about
>> pcomplete...
>
> Adding Stefan, who should know more about pcomplete.
>
> Stefan, any suggestions?
>
I'm still not Stefan, but this bug is fixed by the attached patch.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Check-that-arguments-are-strings-in-pcomplete-here-u.patch --]
[-- Type: text/x-diff; name=Check-that-arguments-are-strings-in-pcomplete-here-u.patch, Size: 1661 bytes --]
From 60416d39cda3ebf336bdb77487d3b7d8d429f1b6 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Mon, 12 Dec 2022 22:16:18 +0000
Subject: [PATCH] Check that arguments are strings in pcomplete-here-using-help
* lisp/pcomplete.el (pcomplete-here-using-help): Check that
pcomplete-args are strings before applying string operations on
them. Fixes bug#59956 and bug#60021.
---
lisp/pcomplete.el | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/lisp/pcomplete.el b/lisp/pcomplete.el
index 4e3a88bbda8..c63c490bcaa 100644
--- a/lisp/pcomplete.el
+++ b/lisp/pcomplete.el
@@ -1449,12 +1449,18 @@ pcomplete-here-using-help
The switches are obtained by calling `pcomplete-from-help' with
COMMAND and ARGS as arguments."
(while (cond
- ((string= "--" (pcomplete-arg 1))
+ ((let ((arg (pcomplete-arg 1)))
+ (if (stringp arg)
+ (string= "--" arg)))
(while (pcomplete-here (pcomplete-entries))))
- ((pcomplete-match "\\`--[^=]+=\\(.*\\)" 0)
+ ((let ((arg (pcomplete-arg 0)))
+ (if (stringp arg)
+ (pcomplete-match "\\`--[^=]+=\\(.*\\)" 0)))
(pcomplete-here (pcomplete-entries)
(pcomplete-match-string 1 0)))
- ((string-prefix-p "-" (pcomplete-arg 0))
+ ((let ((arg (pcomplete-arg 0)))
+ (if (stringp arg)
+ (string-prefix-p "-" arg)))
(pcomplete-here (apply #'pcomplete-from-help command args)))
(t (pcomplete-here* (pcomplete-entries))))))
--
2.35.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* bug#59956: 29.0.60: Failure when completing arguments in Eshell after variable interpolation
2022-12-12 22:21 ` Gregory Heytings
@ 2022-12-12 22:37 ` Augusto Stoffel
2022-12-12 23:27 ` Gregory Heytings
2022-12-16 6:09 ` Jim Porter
1 sibling, 1 reply; 24+ messages in thread
From: Augusto Stoffel @ 2022-12-12 22:37 UTC (permalink / raw)
To: Gregory Heytings; +Cc: Jim Porter, Eli Zaretskii, Stefan Monnier, 59956
On Mon, 12 Dec 2022 at 22:21, Gregory Heytings wrote:
>>> Starting from "emacs -Q -f -eshell", type "echo $exec-path " (note
>>> the trailing space), and then hit TAB. The result is this error:
>>>
>>> pcomplete-match: Wrong type argument: stringp, ("/usr/bin" ...)
>>>
>>> This is a regression from Emacs 28, and it looks like it's due to
>>> 'pcomplete-here-using-help' assuming that all the pcomplete args
>>> are strings. However, 'exec-path' is a list (and Eshell reports it
>>> this way to pcomplete), so the completion fails. I think all that's
>>> necessary is checking that the pcomplete args are strings in
>>> 'pcomplete-here-using-help', but I know next to nothing about
>>> pcomplete...
>>
>> Adding Stefan, who should know more about pcomplete.
>>
>> Stefan, any suggestions?
>>
>
> I'm still not Stefan, but this bug is fixed by the attached patch.
This is not a fix to the problem, in my opinion. Just take any other
command with a pcomplete rule, say "cvs $exec-path SPC TAB" and you will
see the same problem.
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#59956: 29.0.60: Failure when completing arguments in Eshell after variable interpolation
2022-12-12 22:37 ` Augusto Stoffel
@ 2022-12-12 23:27 ` Gregory Heytings
2022-12-16 11:20 ` Augusto Stoffel
0 siblings, 1 reply; 24+ messages in thread
From: Gregory Heytings @ 2022-12-12 23:27 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: Jim Porter, Eli Zaretskii, Stefan Monnier, 59956
>>>> Starting from "emacs -Q -f -eshell", type "echo $exec-path " (note
>>>> the trailing space), and then hit TAB. The result is this error:
>>>>
>>>> pcomplete-match: Wrong type argument: stringp, ("/usr/bin" ...)
>>>>
>>>> This is a regression from Emacs 28, and it looks like it's due to
>>>> 'pcomplete-here-using-help' assuming that all the pcomplete args are
>>>> strings. However, 'exec-path' is a list (and Eshell reports it this
>>>> way to pcomplete), so the completion fails. I think all that's
>>>> necessary is checking that the pcomplete args are strings in
>>>> 'pcomplete-here-using-help', but I know next to nothing about
>>>> pcomplete...
>>>
>>> Adding Stefan, who should know more about pcomplete.
>>>
>>> Stefan, any suggestions?
>>
>> I'm still not Stefan, but this bug is fixed by the attached patch.
>
> This is not a fix to the problem, in my opinion. Just take any other
> command with a pcomplete rule, say "cvs $exec-path SPC TAB" and you will
> see the same problem.
>
Agreed, it's not a proper fix for that bug.
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#59956: 29.0.60: Failure when completing arguments in Eshell after variable interpolation
2022-12-12 23:27 ` Gregory Heytings
@ 2022-12-16 11:20 ` Augusto Stoffel
0 siblings, 0 replies; 24+ messages in thread
From: Augusto Stoffel @ 2022-12-16 11:20 UTC (permalink / raw)
To: Gregory Heytings
Cc: Jim Porter, Eli Zaretskii, Stefan Monnier, 59956, Daniel Mendler
On Mon, 12 Dec 2022 at 23:27, Gregory Heytings wrote:
> Agreed, it's not a proper fix for that bug.
There are only 6 pcomplete/eshell/* functions in my Emacs. It doesn't
look like the proper fix, which is to adjust eshell so that the
22-year-old docstring of `pcomplete-match' is not lying anymore, is a
lot of work.
But I'm not an eshell user and don't know its internals at all.
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#59956: 29.0.60: Failure when completing arguments in Eshell after variable interpolation
2022-12-12 22:21 ` Gregory Heytings
2022-12-12 22:37 ` Augusto Stoffel
@ 2022-12-16 6:09 ` Jim Porter
2022-12-16 14:13 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
1 sibling, 1 reply; 24+ messages in thread
From: Jim Porter @ 2022-12-16 6:09 UTC (permalink / raw)
To: Gregory Heytings, Eli Zaretskii; +Cc: arstoffel, Stefan Monnier, 59956
On 12/12/2022 2:21 PM, Gregory Heytings wrote:
> I'm still not Stefan, but this bug is fixed by the attached patch.
Thanks. This is pretty close to what I was thinking (I tried almost the
exact same patch locally, but wasn't sure if there was a better way).
Personally, I think something like your patch would probably be the
safest bet for the 29 branch.
Still, like Stefan and Augusto mentioned, there's probably a larger
issue here: should Eshell be allowed to feed Pcomplete non-strings?
Since Pcomplete was written for Eshell initially, there's some basis for
why it *might* support non-string values, but actually requiring that is
an awful lot to ask of every programmer who ever wants to write a
pcomplete function.
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#59956: 29.0.60: Failure when completing arguments in Eshell after variable interpolation
2022-12-16 6:09 ` Jim Porter
@ 2022-12-16 14:13 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-16 14:28 ` Gregory Heytings
0 siblings, 1 reply; 24+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-12-16 14:13 UTC (permalink / raw)
To: Jim Porter
Cc: John Wiegley, Gregory Heytings, arstoffel, Eli Zaretskii, 59956
> Still, like Stefan and Augusto mentioned, there's probably a larger issue
> here: should Eshell be allowed to feed Pcomplete non-strings? Since
> Pcomplete was written for Eshell initially, there's some basis for why it
> *might* support non-string values, but actually requiring that is an awful
> lot to ask of every programmer who ever wants to write a pcomplete function.
A "general" solution might be the one below, tho it looks more like
a general workaround, I think.
John?
Stefan
diff --git a/lisp/pcomplete.el b/lisp/pcomplete.el
index 4e3a88bbda8..6a4d754b8c0 100644
--- a/lisp/pcomplete.el
+++ b/lisp/pcomplete.el
@@ -646,12 +646,14 @@ 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."
- (nth (+ (pcase index
- ('first 0)
- ('last pcomplete-last)
- (_ (- pcomplete-index (or index 0))))
- (or offset 0))
- pcomplete-args))
+ (let ((arg (nth (+ (pcase index
+ ('first 0)
+ ('last pcomplete-last)
+ (_ (- pcomplete-index (or index 0))))
+ (or offset 0))
+ pcomplete-args)))
+ (when arg
+ (if (stringp arg) arg (format "%S" arg)))))
(defun pcomplete-begin (&optional index offset)
"Return the beginning position of the INDEXth argument.
^ permalink raw reply related [flat|nested] 24+ messages in thread
* bug#59956: 29.0.60: Failure when completing arguments in Eshell after variable interpolation
2022-12-16 14:13 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2022-12-16 14:28 ` Gregory Heytings
2022-12-19 0:54 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 1 reply; 24+ messages in thread
From: Gregory Heytings @ 2022-12-16 14:28 UTC (permalink / raw)
To: Stefan Monnier; +Cc: Jim Porter, Eli Zaretskii, arstoffel, 59956, John Wiegley
>
> A "general" solution might be the one below
>
Why not (format "%s" (nth (+ (pcase ...))))?
>
> tho it looks more like a general workaround, I think.
>
Indeed, I think it would be better if pcomplete could see the unexpanded
argument instead of its expansion.
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#59956: 29.0.60: Failure when completing arguments in Eshell after variable interpolation
2022-12-16 14:28 ` Gregory Heytings
@ 2022-12-19 0:54 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-19 1:19 ` Gregory Heytings
2022-12-19 10:31 ` Augusto Stoffel
0 siblings, 2 replies; 24+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-12-19 0:54 UTC (permalink / raw)
To: Gregory Heytings
Cc: Jim Porter, Eli Zaretskii, arstoffel, 59956, John Wiegley
>> A "general" solution might be the one below
> Why not (format "%s" (nth (+ (pcase ...))))?
Because I prefer to confine the workaround to those rare cases where we
actually need it.
>> tho it looks more like a general workaround, I think.
> Indeed, I think it would be better if pcomplete could see the
> unexpanded argument instead of its expansion.
There's a tension here: we want the completion to operate on the actual
buffer text obviously, so in some places we definitely want to see the
"unexpanded argument" [1], but when it comes to looking at other arguments
to decide which completion table to use at point, it's often more useful
to see the expanded arguments (i.e. the thing that the command will
actually see). E.g. if the previous arg is `$foo` which expands to `-u`
we'd probably prefer to see `-u` in order to know we should complete
against user names.
Stefan
[1] And we have an "API bug" in this area where Pcomplete can't reliably
figure out what the to-be-completed text actually is, BTW.
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#59956: 29.0.60: Failure when completing arguments in Eshell after variable interpolation
2022-12-19 0:54 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2022-12-19 1:19 ` Gregory Heytings
2022-12-19 2:15 ` Jim Porter
2022-12-19 3:07 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-19 10:31 ` Augusto Stoffel
1 sibling, 2 replies; 24+ messages in thread
From: Gregory Heytings @ 2022-12-19 1:19 UTC (permalink / raw)
To: Stefan Monnier; +Cc: Jim Porter, Eli Zaretskii, arstoffel, 59956, John Wiegley
>>> A "general" solution might be the one below
>>
>> Why not (format "%s" (nth (+ (pcase ...))))?
>
> Because I prefer to confine the workaround to those rare cases where we
> actually need it.
>
I see, thanks! And I agree that it's better.
>>> tho it looks more like a general workaround, I think.
>>
>> Indeed, I think it would be better if pcomplete could see the
>> unexpanded argument instead of its expansion.
>
> There's a tension here: we want the completion to operate on the actual
> buffer text obviously, so in some places we definitely want to see the
> "unexpanded argument" [1], but when it comes to looking at other
> arguments to decide which completion table to use at point, it's often
> more useful to see the expanded arguments (i.e. the thing that the
> command will actually see). E.g. if the previous arg is `$foo` which
> expands to `-u` we'd probably prefer to see `-u` in order to know we
> should complete against user names.
>
Hmmm... So I guess it would be better, instead of replacing lists (and in
general non-strings) by their string representation, and instead of always
looking at an unexpanded argument, to look at the unexpanded argument only
when the expanded argument is not a string? E.g. if $foo is "-u" we would
use "-u", but if $foo is (b a r) we would use "$foo".
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#59956: 29.0.60: Failure when completing arguments in Eshell after variable interpolation
2022-12-19 1:19 ` Gregory Heytings
@ 2022-12-19 2:15 ` Jim Porter
2022-12-19 3:07 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
1 sibling, 0 replies; 24+ messages in thread
From: Jim Porter @ 2022-12-19 2:15 UTC (permalink / raw)
To: Gregory Heytings, Stefan Monnier
Cc: John Wiegley, Eli Zaretskii, arstoffel, 59956
On 12/18/2022 5:19 PM, Gregory Heytings wrote:
> Hmmm... So I guess it would be better, instead of replacing lists (and
> in general non-strings) by their string representation, and instead of
> always looking at an unexpanded argument, to look at the unexpanded
> argument only when the expanded argument is not a string? E.g. if $foo
> is "-u" we would use "-u", but if $foo is (b a r) we would use "$foo".
For external commands called from Eshell, the most-correct thing to do
would be to take the list of all arguments, flatten it, and convert
every element in the flattened list to a string. That's how Eshell will
ultimately call the program.
The tricky bit in my mind is how to do that automatically for external
commands, but to still allow a Pcomplete function for an Eshell command
(i.e. a function named 'eshell/FOO') to have access to the "raw"[1]
expanded form, even if it contains sublists, numbers, other objects,
etc. Maybe we could just do the first thing above, and then think about
adding a new API to Pcomplete that gets the "raw" expanded form for later...
[1] Is it really "raw" if variables have been expanded? Probably not. I
can't think of a better name at the moment though.
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#59956: 29.0.60: Failure when completing arguments in Eshell after variable interpolation
2022-12-19 1:19 ` Gregory Heytings
2022-12-19 2:15 ` Jim Porter
@ 2022-12-19 3:07 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-19 15:00 ` Gregory Heytings
1 sibling, 1 reply; 24+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-12-19 3:07 UTC (permalink / raw)
To: Gregory Heytings
Cc: Jim Porter, Eli Zaretskii, arstoffel, 59956, John Wiegley
> Hmmm... So I guess it would be better, instead of replacing lists (and in
> general non-strings) by their string representation, and instead of always
> looking at an unexpanded argument, to look at the unexpanded argument only
> when the expanded argument is not a string? E.g. if $foo is "-u" we would
> use "-u", but if $foo is (b a r) we would use "$foo".
Sounds like just another way to work around the problem.
I think the only "real" fix is to change the API so the callers can say
what kind of data they're looking for.
The more I think about it, the more my workaround sounds appealing.
My main remaining question is whether there exist code out there that
relies on the current behavior.
Stefan
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#59956: 29.0.60: Failure when completing arguments in Eshell after variable interpolation
2022-12-19 3:07 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2022-12-19 15:00 ` Gregory Heytings
2022-12-19 16:22 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 1 reply; 24+ messages in thread
From: Gregory Heytings @ 2022-12-19 15:00 UTC (permalink / raw)
To: Stefan Monnier; +Cc: Jim Porter, Eli Zaretskii, arstoffel, 59956, John Wiegley
>
> The more I think about it, the more my workaround sounds appealing.
>
What do you think of this (based on Augusto's suggestion)? It seems to me
that it's a more elegant workaround, with which further improvements
become possible, which isn't the case if we squeeze the value into a
string.
diff --git a/lisp/pcomplete.el b/lisp/pcomplete.el
index 4e3a88bbda8..2a2bbf114e0 100644
--- a/lisp/pcomplete.el
+++ b/lisp/pcomplete.el
@@ -645,13 +645,25 @@ 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."
- (nth (+ (pcase index
- ('first 0)
- ('last pcomplete-last)
- (_ (- pcomplete-index (or index 0))))
- (or offset 0))
- pcomplete-args))
+accessing absolute argument positions.
+
+When the INDEXth argument has been transformed into something
+that is not a string by `pcomplete-parse-arguments-function', the
+text representation of the argument is returned, and its value is
+stored in its pcomplete-arg-value text property."
+ (let ((arg
+ (nth (+ (pcase index
+ ('first 0)
+ ('last pcomplete-last)
+ (_ (- pcomplete-index (or index 0))))
+ (or offset 0))
+ pcomplete-args)))
+ (if (stringp arg)
+ arg
+ (propertize
+ (buffer-substring (pcomplete-begin index offset)
+ (pcomplete-begin (1- (or index 0)) offset))
+ 'pcomplete-arg-value arg))))
(defun pcomplete-begin (&optional index offset)
"Return the beginning position of the INDEXth argument.
^ permalink raw reply related [flat|nested] 24+ messages in thread
* bug#59956: 29.0.60: Failure when completing arguments in Eshell after variable interpolation
2022-12-19 15:00 ` Gregory Heytings
@ 2022-12-19 16:22 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-19 22:22 ` Gregory Heytings
0 siblings, 1 reply; 24+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-12-19 16:22 UTC (permalink / raw)
To: Gregory Heytings
Cc: Jim Porter, Eli Zaretskii, arstoffel, 59956, John Wiegley
> What do you think of this (based on Augusto's suggestion)? It seems to me
> that it's a more elegant workaround, with which further improvements become
> possible, which isn't the case if we squeeze the value into a string.
Sounds good to me,
Stefan
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#59956: 29.0.60: Failure when completing arguments in Eshell after variable interpolation
2022-12-19 16:22 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2022-12-19 22:22 ` Gregory Heytings
2022-12-21 6:32 ` Jim Porter
0 siblings, 1 reply; 24+ messages in thread
From: Gregory Heytings @ 2022-12-19 22:22 UTC (permalink / raw)
To: Stefan Monnier; +Cc: Jim Porter, Eli Zaretskii, arstoffel, 59956, John Wiegley
[-- Attachment #1: Type: text/plain, Size: 159 bytes --]
>
> Sounds good to me,
>
Thanks.
Augusto and Jim, can you please try the attached patch and tell us if it
fixes the issue without introducing regressions?
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Handle-non-string-values-in-pcomplete.patch --]
[-- Type: text/x-diff; name=Handle-non-string-values-in-pcomplete.patch, Size: 2032 bytes --]
From 5bee9d8bec42ddf38bb7e8d8f2a1b36adb96a506 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Mon, 19 Dec 2022 22:18:22 +0000
Subject: [PATCH] Handle non-string values in pcomplete
* lisp/pcomplete.el (pcomplete-arg): When
pcomplete-parse-arguments-function returns a non-string value,
return the string the user typed in, and attach the value as a
text property to that string. Fixes bug#59956 and bug#60021.
---
lisp/pcomplete.el | 27 ++++++++++++++++++++-------
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/lisp/pcomplete.el b/lisp/pcomplete.el
index 4e3a88bbda8..b1fbd88a377 100644
--- a/lisp/pcomplete.el
+++ b/lisp/pcomplete.el
@@ -645,13 +645,26 @@ 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."
- (nth (+ (pcase index
- ('first 0)
- ('last pcomplete-last)
- (_ (- pcomplete-index (or index 0))))
- (or offset 0))
- pcomplete-args))
+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 (stringp arg)
+ arg
+ (propertize
+ (buffer-substring (pcomplete-begin index offset)
+ (pcomplete-begin (1- (or index 0)) offset))
+ 'pcomplete-arg-value arg))))
(defun pcomplete-begin (&optional index offset)
"Return the beginning position of the INDEXth argument.
--
2.35.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* bug#59956: 29.0.60: Failure when completing arguments in Eshell after variable interpolation
2022-12-19 22:22 ` Gregory Heytings
@ 2022-12-21 6:32 ` Jim Porter
2022-12-21 9:28 ` Gregory Heytings
0 siblings, 1 reply; 24+ messages in thread
From: Jim Porter @ 2022-12-21 6:32 UTC (permalink / raw)
To: Gregory Heytings, Stefan Monnier
Cc: John Wiegley, Eli Zaretskii, arstoffel, 59956
On 12/19/2022 2:22 PM, Gregory Heytings wrote:
> Augusto and Jim, can you please try the attached patch and tell us if it
> fixes the issue without introducing regressions?
It works for me after a bit of testing, thanks. I don't see any
regressions on the Eshell side of things, at least.
While it would be nice to have a way for Eshell to provide multiple
forms of its arguments for completion (one for external commands that
expect a flat list of strings, and another for Eshell commands), that
seems like a lot more work that would probably be better spent
elsewhere. (And a change like that would be too big for the release
branch anyway.)
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#59956: 29.0.60: Failure when completing arguments in Eshell after variable interpolation
2022-12-21 6:32 ` Jim Porter
@ 2022-12-21 9:28 ` Gregory Heytings
2022-12-29 22:02 ` Gregory Heytings
0 siblings, 1 reply; 24+ messages in thread
From: Gregory Heytings @ 2022-12-21 9:28 UTC (permalink / raw)
To: Jim Porter; +Cc: John Wiegley, Eli Zaretskii, arstoffel, Stefan Monnier, 59956
>> Augusto and Jim, can you please try the attached patch and tell us if
>> it fixes the issue without introducing regressions?
>
> It works for me after a bit of testing, thanks. I don't see any
> regressions on the Eshell side of things, at least.
>
Thanks for your feedback!
>
> While it would be nice to have a way for Eshell to provide multiple
> forms of its arguments for completion (one for external commands that
> expect a flat list of strings, and another for Eshell commands), that
> seems like a lot more work that would probably be better spent
> elsewhere. (And a change like that would be too big for the release
> branch anyway.)
>
Indeed. There are two levels involved here: the pcomplete one and the
eshell one. The patch makes the pcomplete level more robust by ensuring
that, even if pcomplete-parse-arguments-function does not return strings
(which was an implicit assumption), pcomplete will continue to work. On
the eshell level, it would probably be better add a companion function to
eshell-complete-parse-arguments (which is used for
pcomplete-parse-arguments-function) that would return a proper string
representation of the expanded arguments, and to use it for
pcomplete-parse-arguments-function, instead of
eshell-complete-parse-arguments.
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#59956: 29.0.60: Failure when completing arguments in Eshell after variable interpolation
2022-12-19 0:54 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-19 1:19 ` Gregory Heytings
@ 2022-12-19 10:31 ` Augusto Stoffel
2022-12-19 16:21 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-19 18:04 ` Jim Porter
1 sibling, 2 replies; 24+ messages in thread
From: Augusto Stoffel @ 2022-12-19 10:31 UTC (permalink / raw)
To: Stefan Monnier
Cc: Jim Porter, Gregory Heytings, Eli Zaretskii, 59956, John Wiegley
On Sun, 18 Dec 2022 at 19:54, Stefan Monnier wrote:
> There's a tension here: we want the completion to operate on the actual
> buffer text obviously, so in some places we definitely want to see the
> "unexpanded argument" [1], but when it comes to looking at other arguments
> to decide which completion table to use at point, it's often more useful
> to see the expanded arguments (i.e. the thing that the command will
> actually see). E.g. if the previous arg is `$foo` which expands to `-u`
> we'd probably prefer to see `-u` in order to know we should complete
> against user names.
If $foo is "-u -v"`, your patch will make pcomplete see this as a single
argument, no?
I think it makes sense to send the string "$foo" with the list '("-u"
"-v") embedded as text property, and add some convenience functions to
fetch the list for the pcomplete functions that can handle the more
refined information.
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#59956: 29.0.60: Failure when completing arguments in Eshell after variable interpolation
2022-12-19 10:31 ` Augusto Stoffel
@ 2022-12-19 16:21 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-19 18:04 ` Jim Porter
1 sibling, 0 replies; 24+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-12-19 16:21 UTC (permalink / raw)
To: Augusto Stoffel
Cc: Jim Porter, Gregory Heytings, Eli Zaretskii, 59956, John Wiegley
>> There's a tension here: we want the completion to operate on the actual
>> buffer text obviously, so in some places we definitely want to see the
>> "unexpanded argument" [1], but when it comes to looking at other arguments
>> to decide which completion table to use at point, it's often more useful
>> to see the expanded arguments (i.e. the thing that the command will
>> actually see). E.g. if the previous arg is `$foo` which expands to `-u`
>> we'd probably prefer to see `-u` in order to know we should complete
>> against user names.
>
> If $foo is "-u -v"`, your patch will make pcomplete see this as a single
> argument, no?
I must admit that this gets into detailed semantics of Eshell with which
I'm not familiar, but AFAIK if $foo contains a string such as "-u -v",
then my patch won't have any effect at all.
More generally my patch doesn't change the number of arguments as seen
by `pcomplete-arg`. IOW if $foo is turned into several arguments, it's
presumably done before those are turned into what Pcomplete sees as "the
list of arguments".
> I think it makes sense to send the string "$foo" with the list '("-u"
> "-v") embedded as text property, and add some convenience functions to
> fetch the list for the pcomplete functions that can handle the more
> refined information.
Could make sense, indeed. I'd be interested to see what a patch for
that could look like, tho.
Stefan
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#59956: 29.0.60: Failure when completing arguments in Eshell after variable interpolation
2022-12-19 10:31 ` Augusto Stoffel
2022-12-19 16:21 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2022-12-19 18:04 ` Jim Porter
1 sibling, 0 replies; 24+ messages in thread
From: Jim Porter @ 2022-12-19 18:04 UTC (permalink / raw)
To: Augusto Stoffel, Stefan Monnier
Cc: John Wiegley, Gregory Heytings, Eli Zaretskii, 59956
On 12/19/2022 2:31 AM, Augusto Stoffel wrote:
> If $foo is "-u -v"`, your patch will make pcomplete see this as a single
> argument, no?
For Eshell at least, that's what it should do:
~ $ setq foo "-AlF ."
-AlF .
~/src/emacs/build $ ls $foo
/usr/bin/ls: invalid option -- ' '
Try '/usr/bin/ls --help' for more information.
Variable expansions in Eshell work more like they do in Zsh, where they
*aren't* split into multiple arguments on word boundaries.
> I think it makes sense to send the string "$foo" with the list '("-u"
> "-v") embedded as text property, and add some convenience functions to
> fetch the list for the pcomplete functions that can handle the more
> refined information.
I think the best would be if code that invokes Pcomplete would tell
Pcomplete the expanded forms of its arguments like it does now, but to
additionally make sure that the Pcomplete function sees those arguments
the way the actual command would. If Eshell provided a flattened list of
strings to Pcomplete, then everything should Just Work, right?
However, I'm not sure how to do that *and* be able to provide the
un-stringified version so that Pcomplete functions for built-in Eshell
commands can access those arguments in their original (but still
expanded) forms. Can Eshell know ahead of time what form to provide its
arguments in? Should it just provide both and let the Pcomplete function
pick its poison?
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2022-12-29 22:02 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-11 1:25 bug#59956: 29.0.60: Failure when completing arguments in Eshell after variable interpolation Jim Porter
2022-12-11 7:44 ` Eli Zaretskii
2022-12-11 8:56 ` Augusto Stoffel
2022-12-11 15:27 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-12 22:21 ` Gregory Heytings
2022-12-12 22:37 ` Augusto Stoffel
2022-12-12 23:27 ` Gregory Heytings
2022-12-16 11:20 ` Augusto Stoffel
2022-12-16 6:09 ` Jim Porter
2022-12-16 14:13 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-16 14:28 ` Gregory Heytings
2022-12-19 0:54 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-19 1:19 ` Gregory Heytings
2022-12-19 2:15 ` Jim Porter
2022-12-19 3:07 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-19 15:00 ` Gregory Heytings
2022-12-19 16:22 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-19 22:22 ` Gregory Heytings
2022-12-21 6:32 ` Jim Porter
2022-12-21 9:28 ` Gregory Heytings
2022-12-29 22:02 ` Gregory Heytings
2022-12-19 10:31 ` Augusto Stoffel
2022-12-19 16:21 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-19 18:04 ` Jim Porter
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).