bug#54603: 29.0.50; [PATCH] Eshell's external pipe module interferes with other argument parsing hooks

[Jim Porter <jporterbugs@gmail.com>]

On 3/31/2022 2:55 PM, Sean Whitton wrote:
> On Thu 31 Mar 2022 at 01:58PM -07, Jim Porter wrote:
> 
>> I'm not so sure. That would mean "'foo" in "echo 'foo" is treated as a
>> Lisp form, but Eshell expects you to use "#'" (or "(" or "`") to
>> introduce a Lisp form. As I understand it, that's so that typos don't do
>> such surprising things. There's a good chance that a user typing "echo
>> 'foo" actually meant "echo 'foo'".
> 
> Right okay.  We can just skip over entire Lisp forms when we find them.
> I don't think there could be a non-highly esoteric shell command for
> standard POSIX shells -- which is what this feature is for -- which that
> would break.  Like this:
> 
>>> diff --git a/lisp/eshell/em-extpipe.el b/lisp/eshell/em-extpipe.el
>>> index eb5b3bfe1d..0787ab791b 100644
>>> --- a/lisp/eshell/em-extpipe.el
>>> +++ b/lisp/eshell/em-extpipe.el
>>> @@ -99,7 +99,7 @@ eshell-parse-external-pipeline
>>>                        (let* ((found
>>>                                (save-excursion
>>>                                  (re-search-forward
>>> -                                "\\(?:#?'\\|\"\\|\\\\\\)" bound t)))
>>> +                                "\\(?:(\\|#?'\\|\"\\|\\\\\\)" bound t)))
>>>                               (next (or (and found (match-beginning 0))
>>>                                         bound)))
>>>                          (if (re-search-forward pat next t)
>>>
> 
> Something in my init.el is breaking the extpipe tests atm, but I ad hoc
> tested one of your cases for this bug and it works.  Could you confirm?

The first test case -- "(eq 'foo nil)" -- works, but the second doesn't:

   ~ $ echo $(list "one" "two")(:s'o'x')
   Invalid read syntax: ")", 1, 33

Similarly:

   ~ $ echo *(:gs'o'x')
   Invalid read syntax: ")", 1, 16

I guess this is because it's now trying to read the argument modifier as 
a Lisp form. Normally, `eshell-parse-lisp-argument' would ignore that 
bit, since the `(' starting the modifier isn't at the beginning of an 
argument. This could probably be resolved with some additional logic 
(setting `eshell-current-argument' as appropriate?), but I hope there's 
a cleaner way that doesn't involve reimplementing 
`eshell-parse-argument' within `eshell-parse-external-pipeline'.