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

[Jim Porter]

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'.