GNU bug report logs - #46351
28.0.50; Add convenient way to bypass Eshell's own pipelining

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 46351 in the body.
You can then email your comments to 46351 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: bug-gnu-emacs <at> gnu.org
Subject: 28.0.50; Add convenient way to bypass Eshell's own pipelining
Date: Sat, 06 Feb 2021 13:06:53 -0700

[Message part 1 (text/plain, inline)]

Most people who use Eshell a lot need to be aware of the limitations of
its support for shell pipelines: primarily, that all data in pipelines
has to go through Emacs buffers, and that can be slow.  If you're
running pipelines which will move a lot of data, you should use a system
shell which will be more performant.

Currently, if Eshell is your primary shell, it's not particularly
convenient to switch to using an operating system shell when you're
preparing a pipeline that you know is going to move a lot of data.  You
would need either to switch to a shell-mode buffer, or quote and escape
your Eshell input and put something like `bash -c' in front of it.

I think that it would be good to have a way to easily toggle Eshell's
own pipelining support on and off for particular commands.  I have come
up with the attached patches.  When the new functions I've defined are
enabled, you can type

    !! foo | bar 'arg' >baz >>#<buffer *scratch*>

and it will be executed by Eshell as if you had typed

    bash -c 'foo | bar '"'"'arg'"'"' >baz' >>#<buffer *scratch*>

The idea is that you can easily toggle Eshell's pipelining on and off as
appropriate to your needs just by adding and removing the "!!" prefix.

I think that the patches I've prepared are a clean implementation of
this feature that would be good to include in Emacs.

-- 
Sean Whitton

[0001-Add-eshell-restore-unexpanded-input.patch (text/x-diff, attachment)]

[0002-Add-eshell-shell-command-and-eshell-expand-to-eshell.patch (text/x-diff, attachment)]

Message #8 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 46351 <at> debbugs.gnu.org
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Sun, 07 Feb 2021 10:17:39 +0100

Sean Whitton <spwhitton <at> spwhitton.name> writes:

Hi Sean,

> I think that it would be good to have a way to easily toggle Eshell's
> own pipelining support on and off for particular commands.  I have come
> up with the attached patches.  When the new functions I've defined are
> enabled, you can type
>
>     !! foo | bar 'arg' >baz >>#<buffer *scratch*>
>
> and it will be executed by Eshell as if you had typed
>
>     bash -c 'foo | bar '"'"'arg'"'"' >baz' >>#<buffer *scratch*>
>
> The idea is that you can easily toggle Eshell's pipelining on and off as
> appropriate to your needs just by adding and removing the "!!" prefix.

Nice idea.

> I think that the patches I've prepared are a clean implementation of
> this feature that would be good to include in Emacs.

Applying your patch, I get the compiler warning

--8<---------------cut here---------------start------------->8---
  ELC      eshell/esh-mode.elc

In end of data:
eshell/esh-mode.el:1114:1: Warning: the function ‘(setf buffer-substring)’ is
    not known to be defined.
--8<---------------cut here---------------end--------------->8---

And using it in eshell, there is

--8<---------------cut here---------------start------------->8---
~/src/emacs $ !! cat ~/.emacs | grep albinus
!!: command not found
--8<---------------cut here---------------end--------------->8---

I wanted to see, whether this works also for remote directories. Have
you tested this?

> Sean Whitton

Best regards, Michael.

Message #11 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 46351 <at> debbugs.gnu.org
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Sun, 07 Feb 2021 12:01:58 -0700

On Sun 07 Feb 2021 at 10:17AM +01, Michael Albinus wrote:

> Applying your patch, I get the compiler warning
>
> --8<---------------cut here---------------start------------->8---
>   ELC      eshell/esh-mode.elc
>
> In end of data:
> eshell/esh-mode.el:1114:1: Warning: the function ‘(setf buffer-substring)’ is
>     not known to be defined.
> --8<---------------cut here---------------end--------------->8---

Hrm, I can't reproduce this, but looking at the docs, I think that the
problem is a missing (require 'cl-lib).  Would you mind seeing whether
that eliminates the warning at your end?

> And using it in eshell, there is
>
> --8<---------------cut here---------------start------------->8---
> ~/src/emacs $ !! cat ~/.emacs | grep albinus
> !!: command not found
> --8<---------------cut here---------------end--------------->8---

You need to add eshell-expand-to-eshell-shell-command to
eshell-expand-input-functions and (optionally)
eshell-restore-unexpanded-input to eshell-input-filter-functions.  Or
maybe (the first of) these should be added by default?  What do you think?

For testing you can type these forms into an Eshell buffer:

(add-hook 'eshell-expand-input-functions #'eshell-expand-to-eshell-shell-command nil t)
(add-hook 'eshell-input-filter-functions #'eshell-restore-unexpanded-input nil t)

> I wanted to see, whether this works also for remote directories. Have
> you tested this?

Yes, I have, and it works (I did cat largefile >file and confirmed it's
much faster when prefixed with !!, of course because the data doesn't
get copied to the local machine).

-- 
Sean Whitton

Message #14 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 46351 <at> debbugs.gnu.org
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Mon, 08 Feb 2021 11:28:54 +0100

Sean Whitton <spwhitton <at> spwhitton.name> writes:

Hi Sean,

>> Applying your patch, I get the compiler warning
>>
>> --8<---------------cut here---------------start------------->8---
>>   ELC      eshell/esh-mode.elc
>>
>> In end of data:
>> eshell/esh-mode.el:1114:1: Warning: the function ‘(setf buffer-substring)’ is
>>     not known to be defined.
>> --8<---------------cut here---------------end--------------->8---
>
> Hrm, I can't reproduce this, but looking at the docs, I think that the
> problem is a missing (require 'cl-lib).  Would you mind seeing whether
> that eliminates the warning at your end?

Yes, that helps.

>> And using it in eshell, there is
>>
>> --8<---------------cut here---------------start------------->8---
>> ~/src/emacs $ !! cat ~/.emacs | grep albinus
>> !!: command not found
>> --8<---------------cut here---------------end--------------->8---
>
> You need to add eshell-expand-to-eshell-shell-command to
> eshell-expand-input-functions and (optionally)
> eshell-restore-unexpanded-input to eshell-input-filter-functions.  Or
> maybe (the first of) these should be added by default?  What do you think?
>
> For testing you can type these forms into an Eshell buffer:
>
> (add-hook 'eshell-expand-input-functions #'eshell-expand-to-eshell-shell-command nil t)
> (add-hook 'eshell-input-filter-functions #'eshell-restore-unexpanded-input nil t)

That works. Maybe we could add it to the defaults. But at least I would
like to see it documented in the eshell manual, otherwise nobody will
know the effect of "!!".

And I have the impression, that the eshell history is not preserved any
longer, when I have applied these settings.

>> I wanted to see, whether this works also for remote directories. Have
>> you tested this?
>
> Yes, I have, and it works (I did cat largefile >file and confirmed it's
> much faster when prefixed with !!, of course because the data doesn't
> get copied to the local machine).

It doesn't work for me. The reason is a little bit subtle: you use
`shell-file-name' in order to call a remote shell. In my case, it is
"/usr/local/bin/tcsh", which doesn't exist on the remote machine I've
used for testing.

Maybe you can base your implementation on `shell-command'? This should
know, how to handle a connection-local `shell-file-name'.

Best regards, Michael.

Message #17 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 46351 <at> debbugs.gnu.org
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Mon, 08 Feb 2021 11:07:30 -0700

Hello,

On Mon 08 Feb 2021 at 11:28AM +01, Michael Albinus wrote:

> Sean Whitton <spwhitton <at> spwhitton.name> writes:
>>
>> Hrm, I can't reproduce this, but looking at the docs, I think that the
>> problem is a missing (require 'cl-lib).  Would you mind seeing whether
>> that eliminates the warning at your end?
>
> Yes, that helps.

Thanks, I will add it.

> That works. Maybe we could add it to the defaults. But at least I would
> like to see it documented in the eshell manual, otherwise nobody will
> know the effect of "!!".

I was waiting to document it in the manual based on what others thought
the defaults should be.  If neither hook is added by default then I
think the docstrings of the functions that I've already written are
adequate documentation.

What do you think about adding only the eshell-expand-input-functions
hook by default, versus adding both of them?  I think that adding both
is probably most useful to most people, but it has the potential to
break existing uses of eshell-expand-input-functions in people's configs.

I'll look into your other points, thank you.

-- 
Sean Whitton

Message #20 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 46351 <at> debbugs.gnu.org
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Wed, 10 Feb 2021 12:33:40 +0100

Sean Whitton <spwhitton <at> spwhitton.name> writes:

> Hello,

Hi Sean,

>> That works. Maybe we could add it to the defaults. But at least I would
>> like to see it documented in the eshell manual, otherwise nobody will
>> know the effect of "!!".
>
> I was waiting to document it in the manual based on what others thought
> the defaults should be.  If neither hook is added by default then I
> think the docstrings of the functions that I've already written are
> adequate documentation.

Sure. But the manual needs an explanation of "!!".

> What do you think about adding only the eshell-expand-input-functions
> hook by default, versus adding both of them?  I think that adding both
> is probably most useful to most people, but it has the potential to
> break existing uses of eshell-expand-input-functions in people's
> configs.

Honestly, I'm not a heavy eshell user, so I don't know whether it will
disturb people.

If you don't feel comfortable with these changes by default, you might
move them to an own eshell module, people would load if they like.

Best regards, Michael.

Message #23 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: 46351 <at> debbugs.gnu.org, Michael Albinus <michael.albinus <at> gmx.de>
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Fri, 24 Dec 2021 14:20:27 -0700

[Message part 1 (text/plain, inline)]

Hello,

On Mon 08 Feb 2021 at 11:28AM +01, Michael Albinus wrote:

> Maybe we could add it to the defaults. But at least I would
> like to see it documented in the eshell manual, otherwise nobody will
> know the effect of "!!".

I'm attaching new patches which

- switch !! to ||, because !! in shells typically has something to do
  with rerunning the last command

- activate the syntax by default -- I do use Eshell regularly myself,
  and I don't believe that activating it by default is likely to disrupt
  anyone's usage

- adds a section to the Eshell manual introducing the new syntax.

> And I have the impression, that the eshell history is not preserved any
> longer, when I have applied these settings.

I have not been able to reproduce this -- can you confirm?

> It doesn't work for me. The reason is a little bit subtle: you use
> `shell-file-name' in order to call a remote shell. In my case, it is
> "/usr/local/bin/tcsh", which doesn't exist on the remote machine I've
> used for testing.

I was able to fix this by using `with-connection-local-variables'.
Thank you for pointing out the problem.

-- 
Sean Whitton

[v2-0001-Add-eshell-restore-unexpanded-input.patch (text/x-patch, attachment)]

[v2-0002-Add-eshell-shell-command-and-eshell-expand-to-esh.patch (text/x-patch, attachment)]

Message #28 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 46351 <at> debbugs.gnu.org
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Sat, 25 Dec 2021 14:51:30 +0100

Sean Whitton <spwhitton <at> spwhitton.name> writes:

> Hello,

Hi Sean,

>> Maybe we could add it to the defaults. But at least I would
>> like to see it documented in the eshell manual, otherwise nobody will
>> know the effect of "!!".
>
> I'm attaching new patches which
>
> - switch !! to ||, because !! in shells typically has something to do
>   with rerunning the last command
>
> - activate the syntax by default -- I do use Eshell regularly myself,
>   and I don't believe that activating it by default is likely to disrupt
>   anyone's usage
>
> - adds a section to the Eshell manual introducing the new syntax.

Thanks! I've played a little bit with it.

>> And I have the impression, that the eshell history is not preserved any
>> longer, when I have applied these settings.
>
> I have not been able to reproduce this -- can you confirm?

Yep, the eshell history is there. However, it looks a little bit weird,
when I have applied '|| cat .emacs | grep a'. The history shows me
'eshell-shell-command "cat .emacs | grep a"' instead. I understand
what's meant, but is this convenient for an occasional eshell user?

>> It doesn't work for me. The reason is a little bit subtle: you use
>> `shell-file-name' in order to call a remote shell. In my case, it is
>> "/usr/local/bin/tcsh", which doesn't exist on the remote machine I've
>> used for testing.
>
> I was able to fix this by using `with-connection-local-variables'.
> Thank you for pointing out the problem.

This is fixed, indeed.

Thinking more about, perhaps we could solve it a little bit
differently. Eshell knows how to invoke external commands explicitly, by
prefixing the command with a star like '*ls'. What if we do it the same
with pipes? That means, '*|' would mean to call an external pipe. So my
command above would be 'cat .emacs *| grep a'. And an external pipe
shall convert all other commands in that line into external commands as
well, like '*cat .emacs *| *grep a'.

By this you wouldn't need your restore-unexpanded-input patch any more, yes?

WDYT?

Best regards, Michael.

Message #31 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Michael Albinus <michael.albinus <at> gmx.de>, 46351 <at> debbugs.gnu.org
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Sat, 25 Dec 2021 15:45:43 -0700

Hello Michael,

On Sat 25 Dec 2021 at 02:51PM +01, Michael Albinus wrote:

> Yep, the eshell history is there. However, it looks a little bit weird,
> when I have applied '|| cat .emacs | grep a'. The history shows me
> 'eshell-shell-command "cat .emacs | grep a"' instead. I understand
> what's meant, but is this convenient for an occasional eshell user?

Yeah.  Actually, when I prepared the first version of this series, the
unexpanded version got stored in the history, so I think something has
changed about Eshell history recording in the past year.

> Thinking more about, perhaps we could solve it a little bit
> differently. Eshell knows how to invoke external commands explicitly, by
> prefixing the command with a star like '*ls'. What if we do it the same
> with pipes? That means, '*|' would mean to call an external pipe. So my
> command above would be 'cat .emacs *| grep a'. And an external pipe
> shall convert all other commands in that line into external commands as
> well, like '*cat .emacs *| *grep a'.
>
> By this you wouldn't need your restore-unexpanded-input patch any more, yes?

This is an intriguing suggestion.  It would need to be implemented in
`eshell-parse-pipeline, rather than `eshell-expand-input-functions',
because parsing is required to determine which *| are unquoted, etc.  We
also require *> and *< redirections, also in `eshell-parse-pipeline'.

So, I guess when A *| B is parsed, it would be rewritten to a single
call to `shell-file-name' in the way that `eshell-shell-command' does.
I am not sure how feasible it will be to implement that within the
existing structure of `eshell-parse-pipeline', but I can give it a try.

I have one unresolved design question.  A disadvantage of your proposal
compared to mine is that it is much more laborious to convert an
existing pipeline to use the external shell: you have to replace each |
with *| and > with *>, etc., instead of just putting || at the
beginning.

I think it's important that it is easy to switch this feature on and
off, because it is often only after you start figuring out a pipeline
that you realise you ought to bypass Eshell's pipelining.

One option would be to add an entry to `eshell-expand-input-functions'
which converts all | to *| and > to *> when the input begins with ||.
But that is too simple, because you need to parse the input to know
which | to replace ... any ideas?  I don't think it can be done inside
`eshell-parse-command'.

-- 
Sean Whitton

Message #34 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 46351 <at> debbugs.gnu.org
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Mon, 27 Dec 2021 15:42:37 +0100

Sean Whitton <spwhitton <at> spwhitton.name> writes:

> Hello Michael,

Hi Sean,

>> Thinking more about, perhaps we could solve it a little bit
>> differently. Eshell knows how to invoke external commands explicitly, by
>> prefixing the command with a star like '*ls'. What if we do it the same
>> with pipes? That means, '*|' would mean to call an external pipe. So my
>> command above would be 'cat .emacs *| grep a'. And an external pipe
>> shall convert all other commands in that line into external commands as
>> well, like '*cat .emacs *| *grep a'.
>>
>> By this you wouldn't need your restore-unexpanded-input patch any more, yes?
>
> This is an intriguing suggestion.  It would need to be implemented in
> `eshell-parse-pipeline, rather than `eshell-expand-input-functions',
> because parsing is required to determine which *| are unquoted, etc.  We
> also require *> and *< redirections, also in `eshell-parse-pipeline'.
>
> So, I guess when A *| B is parsed, it would be rewritten to a single
> call to `shell-file-name' in the way that `eshell-shell-command' does.
> I am not sure how feasible it will be to implement that within the
> existing structure of `eshell-parse-pipeline', but I can give it a try.

Perhaps. I'm not so familar with the eshell code, so I cannot give you
much recommendation on implementation details.

> I have one unresolved design question.  A disadvantage of your proposal
> compared to mine is that it is much more laborious to convert an
> existing pipeline to use the external shell: you have to replace each |
> with *| and > with *>, etc., instead of just putting || at the
> beginning.

Yep, but this isn't relevant for the user. This replacement shall happen
by eshell itself, in the background. And it isn't necessary to show this
replacement by the commands in the history.

> I think it's important that it is easy to switch this feature on and
> off, because it is often only after you start figuring out a pipeline
> that you realise you ought to bypass Eshell's pipelining.

As I said: switching on this feature just needs the asterisk to *|.

> One option would be to add an entry to `eshell-expand-input-functions'
> which converts all | to *| and > to *> when the input begins with ||.
> But that is too simple, because you need to parse the input to know
> which | to replace ... any ideas?  I don't think it can be done inside
> `eshell-parse-command'.

Again, I don't see the need for the heading ||. When there is a *| in
the command line, DTRT.

Best regards, Michael.

Message #37 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 46351 <at> debbugs.gnu.org
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Mon, 27 Dec 2021 11:13:29 -0700

Hello Michael,

On Mon 27 Dec 2021 at 03:42PM +01, Michael Albinus wrote:

>> I have one unresolved design question.  A disadvantage of your proposal
>> compared to mine is that it is much more laborious to convert an
>> existing pipeline to use the external shell: you have to replace each |
>> with *| and > with *>, etc., instead of just putting || at the
>> beginning.
>
> Yep, but this isn't relevant for the user. This replacement shall happen
> by eshell itself, in the background. And it isn't necessary to show this
> replacement by the commands in the history.
>
>> I think it's important that it is easy to switch this feature on and
>> off, because it is often only after you start figuring out a pipeline
>> that you realise you ought to bypass Eshell's pipelining.
>
> As I said: switching on this feature just needs the asterisk to *|.
>
>> One option would be to add an entry to `eshell-expand-input-functions'
>> which converts all | to *| and > to *> when the input begins with ||.
>> But that is too simple, because you need to parse the input to know
>> which | to replace ... any ideas?  I don't think it can be done inside
>> `eshell-parse-command'.
>
> Again, I don't see the need for the heading ||. When there is a *| in
> the command line, DTRT.

What I had in mind was a pipeline involving more than two commands, e.g.

    foo | bar | baz >quux

Indeed, this is exactly the sort of pipeline you might want this feature
for.  But the user would have to manually insert three asterisks to turn
on this feature.  That seems pretty inconvenient -- any thoughts on how
to improve it?

-- 
Sean Whitton

Message #40 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 46351 <at> debbugs.gnu.org, michael.albinus <at> gmx.de
Subject: Re: bug#46351: 28.0.50;
 Add convenient way to bypass Eshell's own pipelining
Date: Mon, 27 Dec 2021 20:22:31 +0200

> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Date: Mon, 27 Dec 2021 11:13:29 -0700
> Cc: 46351 <at> debbugs.gnu.org
> 
> What I had in mind was a pipeline involving more than two commands, e.g.
> 
>     foo | bar | baz >quux
> 
> Indeed, this is exactly the sort of pipeline you might want this feature
> for.  But the user would have to manually insert three asterisks to turn
> on this feature.  That seems pretty inconvenient -- any thoughts on how
> to improve it?

The easiest way is to add a user option that would make every pipe use
the external shell.

Message #43 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 46351 <at> debbugs.gnu.org
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Mon, 27 Dec 2021 19:26:17 +0100

Sean Whitton <spwhitton <at> spwhitton.name> writes:

> Hello Michael,

Hi Sean,

>> Again, I don't see the need for the heading ||. When there is a *| in
>> the command line, DTRT.
>
> What I had in mind was a pipeline involving more than two commands, e.g.
>
>     foo | bar | baz >quux
>
> Indeed, this is exactly the sort of pipeline you might want this feature
> for.  But the user would have to manually insert three asterisks to turn
> on this feature.  That seems pretty inconvenient -- any thoughts on how
> to improve it?

One to rule them all. As soon there is one *|, *< or *> in the command
line, everything is regarded external. Also commands like ls or cat,
which behave then like *ls or *cat.

Best regards, Michael.

Message #46 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Michael Albinus <michael.albinus <at> gmx.de>, Eli Zaretskii <eliz <at> gnu.org>,
 46351 <at> debbugs.gnu.org
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Mon, 27 Dec 2021 12:21:53 -0700

Hello,

On Mon 27 Dec 2021 at 08:22PM +02, Eli Zaretskii wrote:

> The easiest way is to add a user option that would make every pipe use
> the external shell.

On Mon 27 Dec 2021 at 07:26PM +01, Michael Albinus wrote:

> One to rule them all. As soon there is one *|, *< or *> in the command
> line, everything is regarded external. Also commands like ls or cat,
> which behave then like *ls or *cat.

I see what you mean now, but I don't think either of these options would
be desirable.  It would mean that you can't combine Lisp functions with
external commands; for example

    my-lisp-function arg1 arg2 | my-cool-encoder *>output.ogg

In this case, the first | needs to use Eshell's own pipelining support.

Similarly something like

    buffer-string #<buffer ...> | my-cool-command *| other-cmd *>file

Does that make sense?  Would you agree that this new feature needs to
work only on individual pairs of commands?

-- 
Sean Whitton

Message #49 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 46351 <at> debbugs.gnu.org, michael.albinus <at> gmx.de
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Mon, 27 Dec 2021 21:35:40 +0200

> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Date: Mon, 27 Dec 2021 12:21:53 -0700
> 
> On Mon 27 Dec 2021 at 08:22PM +02, Eli Zaretskii wrote:
> 
> > The easiest way is to add a user option that would make every pipe use
> > the external shell.
> 
> On Mon 27 Dec 2021 at 07:26PM +01, Michael Albinus wrote:
> 
> > One to rule them all. As soon there is one *|, *< or *> in the command
> > line, everything is regarded external. Also commands like ls or cat,
> > which behave then like *ls or *cat.
> 
> I see what you mean now, but I don't think either of these options would
> be desirable.  It would mean that you can't combine Lisp functions with
> external commands; for example
> 
>     my-lisp-function arg1 arg2 | my-cool-encoder *>output.ogg
> 
> In this case, the first | needs to use Eshell's own pipelining support.
> 
> Similarly something like
> 
>     buffer-string #<buffer ...> | my-cool-command *| other-cmd *>file
> 
> Does that make sense?  Would you agree that this new feature needs to
> work only on individual pairs of commands?

The *| thingy could be available as well, for when you need a mix.  My
proposal was only for the situation when it's hard or inconvenient to
change too many pipes manually.

Message #52 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 46351 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Mon, 27 Dec 2021 20:37:07 +0100

Sean Whitton <spwhitton <at> spwhitton.name> writes:

> Hello,

Hi Sean,

>> One to rule them all. As soon there is one *|, *< or *> in the command
>> line, everything is regarded external. Also commands like ls or cat,
>> which behave then like *ls or *cat.
>
> I see what you mean now, but I don't think either of these options would
> be desirable.  It would mean that you can't combine Lisp functions with
> external commands; for example
>
>     my-lisp-function arg1 arg2 | my-cool-encoder *>output.ogg
>
> In this case, the first | needs to use Eshell's own pipelining support.
>
> Similarly something like
>
>     buffer-string #<buffer ...> | my-cool-command *| other-cmd *>file
>
> Does that make sense?  Would you agree that this new feature needs to
> work only on individual pairs of commands?

If you want this complexity (external and internal pipelins and
stdin/stout redirection) in one command line, you must mark every single
such operator with an asterisk, which might be inconvenient. But you
have the eshell history, which gives you the command line including the
external asterisks, which let you edit the command prior reexecution.

Personally, I could live with both approaches. Your proposal with a
leading "||" is similar to my "one rules them all", because it changes
the meaning of all pipelines etc in a command line to be external.

Best regards, Michael.

Message #55 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 46351 <at> debbugs.gnu.org, michael.albinus <at> gmx.de
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Mon, 27 Dec 2021 12:53:47 -0700

Hello,

On Mon 27 Dec 2021 at 09:35PM +02, Eli Zaretskii wrote:

> The *| thingy could be available as well, for when you need a mix.  My
> proposal was only for the situation when it's hard or inconvenient to
> change too many pipes manually.

Ah right, I see -- yes, nice idea.

-- 
Sean Whitton

Message #58 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 46351 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Mon, 27 Dec 2021 12:54:39 -0700

Hello Michael,

On Mon 27 Dec 2021 at 08:37PM +01, Michael Albinus wrote:

> If you want this complexity (external and internal pipelins and
> stdin/stout redirection) in one command line, you must mark every single
> such operator with an asterisk, which might be inconvenient. But you
> have the eshell history, which gives you the command line including the
> external asterisks, which let you edit the command prior reexecution.
>
> Personally, I could live with both approaches. Your proposal with a
> leading "||" is similar to my "one rules them all", because it changes
> the meaning of all pipelines etc in a command line to be external.

Right.  I was thinking that the piecemeal approach was one of the
advantages of your idea.

I'll see if I can implement this more complex thing and get back to you.

-- 
Sean Whitton

Message #61 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 46351 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Tue, 28 Dec 2021 09:58:38 +0100

Sean Whitton <spwhitton <at> spwhitton.name> writes:

> Hello Michael,

Hi Sean,

>> If you want this complexity (external and internal pipelins and
>> stdin/stout redirection) in one command line, you must mark every single
>> such operator with an asterisk, which might be inconvenient. But you
>> have the eshell history, which gives you the command line including the
>> external asterisks, which let you edit the command prior reexecution.
>>
>> Personally, I could live with both approaches. Your proposal with a
>> leading "||" is similar to my "one rules them all", because it changes
>> the meaning of all pipelines etc in a command line to be external.
>
> Right.  I was thinking that the piecemeal approach was one of the
> advantages of your idea.

Yes, it would be. But it is more complex then. If you have an external
pipe *|, a built-in command on the LHS of the pipe must be external as
well. cat behaves already like this in pipelines, perhaps we shall
request this for all built-in commands.

> I'll see if I can implement this more complex thing and get back to you.

Yes, please do.

Best regards, Michael.

Message #64 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Michael Albinus <michael.albinus <at> gmx.de>, 46351 <at> debbugs.gnu.org
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Mon, 17 Jan 2022 22:19:59 -0700

[Message part 1 (text/plain, inline)]

Hello,

On Tue 28 Dec 2021 at 09:58AM +01, Michael Albinus wrote:

> Sean Whitton <spwhitton <at> spwhitton.name> writes:
>
>> I'll see if I can implement this more complex thing and get back to you.
>
> Yes, please do.

I've got it working.  Please let me know what you think of the attached.

-- 
Sean Whitton

[v3-0001-Add-Eshell-syntax-to-more-easily-bypass-Eshell-s-.patch (text/x-patch, attachment)]

Message #67 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Robert Pluim <rpluim <at> gmail.com>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 46351 <at> debbugs.gnu.org, Michael Albinus <michael.albinus <at> gmx.de>
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Tue, 18 Jan 2022 10:49:12 +0100

>>>>> On Mon, 17 Jan 2022 22:19:59 -0700, Sean Whitton <spwhitton <at> spwhitton.name> said:

    Sean> Hello,
    Sean> On Tue 28 Dec 2021 at 09:58AM +01, Michael Albinus wrote:

    >> Sean Whitton <spwhitton <at> spwhitton.name> writes:
    >> 
    >>> I'll see if I can implement this more complex thing and get back to you.
    >> 
    >> Yes, please do.

    Sean> I've got it working.  Please let me know what you think of the attached.

How does this deal with something like:

ls foo*|awk '{print $1}'

Before your changes that expands 'foo*', but now the '*' gets
swallowed by the '|'. Perhaps you should require whitespace before the
'*'?

Robert
-- 

Message #70 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 46351 <at> debbugs.gnu.org, michael.albinus <at> gmx.de
Subject: Re: bug#46351: 28.0.50;
 Add convenient way to bypass Eshell's own pipelining
Date: Tue, 18 Jan 2022 16:45:07 +0200

> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Date: Mon, 17 Jan 2022 22:19:59 -0700
> 
> I've got it working.  Please let me know what you think of the attached.

Thanks.

I see you play some games with shell syntactic rules and such: IME,
these are often tricky enough to defeat simplistic handling.  For
example, does this support commands that have redirection operators
_before_ the first verb?  It's allowed in every shell I've seen,
albeit very rarely used.

I think Robert found another possible issue.

> +(cl-macrolet
> +    ((deftest (name input expected)
> +       (let ((result (gensym)))
> +         `(ert-deftest ,name ()
> +            (let* ((shell-file-name "sh") (shell-command-switch "-c")
> +                   (,result
> +                    (with-temp-eshell (eshell-parse-command ,input))))
> +              ;; Strip `eshell-trap-errors'.
> +              (should (equal (cadr ,result) ,expected)))))))
> +
> +  (deftest em-extpipe-test-1
> +           "foo *| bar >baz"
> +           '(eshell-named-command "sh"
> +                                  (list "-c" "foo | bar >baz")))

Why do these tests only look for 'sh' as the shell?  What is the
importance of the shell for this purpose?

Message #73 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Robert Pluim <rpluim <at> gmail.com>
Cc: 46351 <at> debbugs.gnu.org, Michael Albinus <michael.albinus <at> gmx.de>
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Tue, 18 Jan 2022 11:27:46 -0700

On Tue 18 Jan 2022 at 10:49AM +01, Robert Pluim wrote:

> How does this deal with something like:
>
> ls foo*|awk '{print $1}'
>
> Before your changes that expands 'foo*', but now the '*' gets
> swallowed by the '|'. Perhaps you should require whitespace before the
> '*'?

Good catch.  I will require whitespace in the next version.  It should
be hard to use this new functionality by accident.

-- 
Sean Whitton

Message #76 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 46351 <at> debbugs.gnu.org, michael.albinus <at> gmx.de
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Tue, 18 Jan 2022 11:40:50 -0700

Hello,

On Tue 18 Jan 2022 at 04:45PM +02, Eli Zaretskii wrote:

> I see you play some games with shell syntactic rules and such: IME,
> these are often tricky enough to defeat simplistic handling.  For
> example, does this support commands that have redirection operators
> _before_ the first verb?  It's allowed in every shell I've seen,
> albeit very rarely used.

These things are indeed tricky.  So long as it is not too easy to
activate the functionality by accident, however, I think that it will be
very helpful with regard to one of the most common reasons people cite
for not using Eshell very much.

> I think Robert found another possible issue.

I will incorporate his suggestion to require whitespace.

>> +(cl-macrolet
>> +    ((deftest (name input expected)
>> +       (let ((result (gensym)))
>> +         `(ert-deftest ,name ()
>> +            (let* ((shell-file-name "sh") (shell-command-switch "-c")
>> +                   (,result
>> +                    (with-temp-eshell (eshell-parse-command ,input))))
>> +              ;; Strip `eshell-trap-errors'.
>> +              (should (equal (cadr ,result) ,expected)))))))
>> +
>> +  (deftest em-extpipe-test-1
>> +           "foo *| bar >baz"
>> +           '(eshell-named-command "sh"
>> +                                  (list "-c" "foo | bar >baz")))
>
> Why do these tests only look for 'sh' as the shell?  What is the
> importance of the shell for this purpose?

The code just substitutes in the value of `shell-file-name' (taking
TRAMP into account), so there isn't much point in varying that value in
the tests, so I just picked something standard.  The user is expected to
know what syntax their external shell will accept and what it will do
with it when they invoke this functionality.  Thanks for looking --
please let me know if I have missed any of the points of your questions.

-- 
Sean Whitton

Message #79 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 46351 <at> debbugs.gnu.org, michael.albinus <at> gmx.de
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Tue, 18 Jan 2022 11:42:27 -0700

Hello,

On Tue 18 Jan 2022 at 04:45PM +02, Eli Zaretskii wrote:

> I see you play some games with shell syntactic rules and such: IME,
> these are often tricky enough to defeat simplistic handling.  For
> example, does this support commands that have redirection operators
> _before_ the first verb?  It's allowed in every shell I've seen,
> albeit very rarely used.

Sorry, missed something -- redirection operators before the first verb
work correctly.  I will add a test to ensure that remains the case.

-- 
Sean Whitton

Message #82 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 46351 <at> debbugs.gnu.org, michael.albinus <at> gmx.de
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Tue, 18 Jan 2022 21:38:27 +0200

> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Cc: michael.albinus <at> gmx.de, 46351 <at> debbugs.gnu.org
> Date: Tue, 18 Jan 2022 11:40:50 -0700
> 
> > I see you play some games with shell syntactic rules and such: IME,
> > these are often tricky enough to defeat simplistic handling.  For
> > example, does this support commands that have redirection operators
> > _before_ the first verb?  It's allowed in every shell I've seen,
> > albeit very rarely used.
> 
> These things are indeed tricky.  So long as it is not too easy to
> activate the functionality by accident, however, I think that it will be
> very helpful with regard to one of the most common reasons people cite
> for not using Eshell very much.

OK, but I don't think I see how this answers my question.  Is it
possible today to say

  >foo something or other

in Eshell, and have the output of 'something' redirected to 'foo'?  If
it is possible today, will it be possible after your changes, and what
will happen with that if 'something' includes the new "*|" pipe
symbol?

> > Why do these tests only look for 'sh' as the shell?  What is the
> > importance of the shell for this purpose?
> 
> The code just substitutes in the value of `shell-file-name' (taking
> TRAMP into account), so there isn't much point in varying that value in
> the tests, so I just picked something standard.  The user is expected to
> know what syntax their external shell will accept and what it will do
> with it when they invoke this functionality.

Does this mean that those tests will not run on systems where 'sh' is
not available?

Message #85 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 46351 <at> debbugs.gnu.org, michael.albinus <at> gmx.de
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Tue, 18 Jan 2022 16:16:01 -0700

Hello,

On Tue 18 Jan 2022 at 09:38PM +02, Eli Zaretskii wrote:

> OK, but I don't think I see how this answers my question.  Is it
> possible today to say
>
>   >foo something or other
>
> in Eshell, and have the output of 'something' redirected to 'foo'?  If
> it is possible today, will it be possible after your changes, and what
> will happen with that if 'something' includes the new "*|" pipe
> symbol?

Interestingly, it's not possible to do that with plain Eshell.  If
someone modifies Eshell's parsing to make it possible, I do not believe
it will interact with em-extpipe.el in any meaningful way.

Even though

    >blah echo "Hello"

doesn't work, with my patch, these three do, and all output to "blah":

    *>blah echo "Hello"
    echo "Hello" *| cat >blah
    echo "Hello" *| >blah cat

If you do

    >blah echo "Hello" *| rev

then on my system, "blah" contains "Hello", not "olleH", as it looks
like GNU bash prioritises the output redirection over the pipe.

To answer your question directly, then, if 'something' contains *| then
the external shell will be asked to redirect the output of the first
command to the file foo and also to pipe it to a second command, and I
guess some shells might duplicate the output and others will just choose
one destination; maybe POSIX has something to say about it.

>> > Why do these tests only look for 'sh' as the shell?  What is the
>> > importance of the shell for this purpose?
>>
>> The code just substitutes in the value of `shell-file-name' (taking
>> TRAMP into account), so there isn't much point in varying that value in
>> the tests, so I just picked something standard.  The user is expected to
>> know what syntax their external shell will accept and what it will do
>> with it when they invoke this functionality.
>
> Does this mean that those tests will not run on systems where 'sh' is
> not available?

No, they will run on any system that Emacs runs on.  They test the
parsing and rewriting done by em-extpipe.el.  Actually executing
commands is tested by test/lisp/eshell/eshell-tests.el.  Executing the
commands is not specific to em-extpipe.el.

-- 
Sean Whitton

Message #88 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 46351 <at> debbugs.gnu.org, michael.albinus <at> gmx.de
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Wed, 19 Jan 2022 09:34:25 +0200

> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Cc: michael.albinus <at> gmx.de, 46351 <at> debbugs.gnu.org
> Date: Tue, 18 Jan 2022 16:16:01 -0700
> 
> >   >foo something or other
> >
> > in Eshell, and have the output of 'something' redirected to 'foo'?  If
> > it is possible today, will it be possible after your changes, and what
> > will happen with that if 'something' includes the new "*|" pipe
> > symbol?
> 
> Interestingly, it's not possible to do that with plain Eshell.  If
> someone modifies Eshell's parsing to make it possible, I do not believe
> it will interact with em-extpipe.el in any meaningful way.

If it doesn't work now, then we cannot break it.

> Even though
> 
>     >blah echo "Hello"
> 
> doesn't work, with my patch, these three do, and all output to "blah":
> 
>     *>blah echo "Hello"
>     echo "Hello" *| cat >blah
>     echo "Hello" *| >blah cat

These work because they delegate to the shell, and the shell does
DTRT.

> If you do
> 
>     >blah echo "Hello" *| rev
> 
> then on my system, "blah" contains "Hello", not "olleH", as it looks
> like GNU bash prioritises the output redirection over the pipe.

Of course.  If you want "olleH" in "blah", you need to do this
instead:

   echo "Hello" *| >blah rev

Thanks.

Message #91 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 46351 <at> debbugs.gnu.org
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Wed, 19 Jan 2022 16:52:00 +0100

Sean Whitton <spwhitton <at> spwhitton.name> writes:

> Hello,

Hi Sean,

> I've got it working.  Please let me know what you think of the attached.

Some comments:

> --- /dev/null
> +++ b/test/lisp/eshell/em-extpipe-tests.el
> @@ -0,0 +1,122 @@
> +(load (expand-file-name "eshell-tests"
> +                        (file-name-directory (or load-file-name
> +                                                 default-directory))))

This is problematic. Loading eshell-tests.el declares also all ert tests
which are contained in that file. Running em-extpipe-tests.el in batch
would run also all tests from that file, which is not intended I believe.

A better approach would be to factor out the helper functions from
eshell-tests.el into an own file, and load it here and in eshell-tests.el.

> +(cl-macrolet
> +    ((deftest (name input expected)
> +       (let ((result (gensym)))
> +         `(ert-deftest ,name ()
> +            (let* ((shell-file-name "sh") (shell-command-switch "-c")

I'm not sure this is the right approach. Why do you change
shell-file-name  and shell-command-switch? You've spoken in another
message about Tramp, but I don't understand this. Tramp has its own
machinery to handle them, via connection-local variables.

> +  (deftest em-extpipe-test-7

Looks like em-extpipe-test-6 is missing.

> Sean Whitton

Best regards, Michael.

Message #94 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 46351 <at> debbugs.gnu.org, michael.albinus <at> gmx.de
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Wed, 19 Jan 2022 13:39:57 -0700

Hello,

On Wed 19 Jan 2022 at 09:34AM +02, Eli Zaretskii wrote:

>> From: Sean Whitton <spwhitton <at> spwhitton.name>
>> Cc: michael.albinus <at> gmx.de, 46351 <at> debbugs.gnu.org
>> Date: Tue, 18 Jan 2022 16:16:01 -0700
>>
>> >   >foo something or other
>> >
>> > in Eshell, and have the output of 'something' redirected to 'foo'?  If
>> > it is possible today, will it be possible after your changes, and what
>> > will happen with that if 'something' includes the new "*|" pipe
>> > symbol?
>>
>> Interestingly, it's not possible to do that with plain Eshell.  If
>> someone modifies Eshell's parsing to make it possible, I do not believe
>> it will interact with em-extpipe.el in any meaningful way.
>
> If it doesn't work now, then we cannot break it.

Okay, so I should add something to skip over my parsing code in the case
that the first thing in the input is a redirection?

-- 
Sean Whitton

Message #97 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 46351 <at> debbugs.gnu.org
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Wed, 19 Jan 2022 13:54:04 -0700

Hello Michael,

On Wed 19 Jan 2022 at 04:52PM +01, Michael Albinus wrote:

>> --- /dev/null
>> +++ b/test/lisp/eshell/em-extpipe-tests.el
>> @@ -0,0 +1,122 @@
>> +(load (expand-file-name "eshell-tests"
>> +                        (file-name-directory (or load-file-name
>> +                                                 default-directory))))
>
> This is problematic. Loading eshell-tests.el declares also all ert tests
> which are contained in that file. Running em-extpipe-tests.el in batch
> would run also all tests from that file, which is not intended I believe.
>
> A better approach would be to factor out the helper functions from
> eshell-tests.el into an own file, and load it here and in eshell-tests.el.

Good point, I'll factor that out.

>> +(cl-macrolet
>> +    ((deftest (name input expected)
>> +       (let ((result (gensym)))
>> +         `(ert-deftest ,name ()
>> +            (let* ((shell-file-name "sh") (shell-command-switch "-c")
>
> I'm not sure this is the right approach. Why do you change
> shell-file-name  and shell-command-switch? You've spoken in another
> message about Tramp, but I don't understand this. Tramp has its own
> machinery to handle them, via connection-local variables.

The unit tests are all about seeing whether em-extpipe sets up
`eshell-parse-command' to do the right thing.  When it comes to
shell-file-name and shell-command-switch, however, all em-extpipe does
is substitute them in verbatim, using `with-connection-local-variables'.
So there isn't much point in varying the values of the two variables in
the tests.

However, as the values of the two variables show up in the expected
return values of `eshell-parse-command' that are part of the test
definitions, in order to write the tests, I need to know what those two
values will be.  It seemed simplest just to bind them to constants.

I could instead substitute the actual values of those variables into the
expected return values.  It seems to me that would sacrifice readability
of the tests, though.  Am I perhaps missing some other benefit?

>> +  (deftest em-extpipe-test-7
>
> Looks like em-extpipe-test-6 is missing.

Oops, will fix.

Many thanks for the review.

-- 
Sean Whitton

Message #100 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 46351 <at> debbugs.gnu.org, michael.albinus <at> gmx.de
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Thu, 20 Jan 2022 08:53:51 +0200

> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Cc: michael.albinus <at> gmx.de, 46351 <at> debbugs.gnu.org
> Date: Wed, 19 Jan 2022 13:39:57 -0700
> 
> >> >   >foo something or other
> >> >
> >> > in Eshell, and have the output of 'something' redirected to 'foo'?  If
> >> > it is possible today, will it be possible after your changes, and what
> >> > will happen with that if 'something' includes the new "*|" pipe
> >> > symbol?
> >>
> >> Interestingly, it's not possible to do that with plain Eshell.  If
> >> someone modifies Eshell's parsing to make it possible, I do not believe
> >> it will interact with em-extpipe.el in any meaningful way.
> >
> > If it doesn't work now, then we cannot break it.
> 
> Okay, so I should add something to skip over my parsing code in the case
> that the first thing in the input is a redirection?

I'm not sure I understand the intent: what do you want to accomplish
by this special handling?

Message #103 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 46351 <at> debbugs.gnu.org
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Thu, 20 Jan 2022 19:41:38 +0100

Sean Whitton <spwhitton <at> spwhitton.name> writes:

> Hello Michael,

Hi Sean,

> I could instead substitute the actual values of those variables into the
> expected return values.  It seems to me that would sacrifice readability
> of the tests, though.  Am I perhaps missing some other benefit?

I understand your intentions, they are OK.

However, if I understand your test cases, they check that the eshell
commands are manipulated as you expect. The tests do not run the
resulting command itself, checking the output. This is a little bit
unfortune, because you could check that the output is indeed what you
expect. And perhaps you could find some constellations, where the output
is different when using either *| or |. This would be another proof that
your changes work.

And this would also give some guidance, where your approach has
limitations (if exist). Showing also *failing* tests in one way or
another is always a benefit. Your tests use only should, there is no
should-not or should-error.

Another benefit of testing working examples and their output is
documentation. I always have trouble to understand documentation of
unknown (to me) features in Emacs. I have the attitude to run the
respective ert tests, and to study how the feature is applied by its
developer. Often, it is more instructive than documentation, and I can
steal the code.

> Many thanks for the review.

Best regards, Michael.

Message #106 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 46351 <at> debbugs.gnu.org, michael.albinus <at> gmx.de
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Thu, 20 Jan 2022 15:16:20 -0700

Hello,

On Thu 20 Jan 2022 at 08:53AM +02, Eli Zaretskii wrote:

>> From: Sean Whitton <spwhitton <at> spwhitton.name>
>> Cc: michael.albinus <at> gmx.de, 46351 <at> debbugs.gnu.org
>> Date: Wed, 19 Jan 2022 13:39:57 -0700
>>
>> >> >   >foo something or other
>> >> >
>> >> > in Eshell, and have the output of 'something' redirected to 'foo'?  If
>> >> > it is possible today, will it be possible after your changes, and what
>> >> > will happen with that if 'something' includes the new "*|" pipe
>> >> > symbol?
>> >>
>> >> Interestingly, it's not possible to do that with plain Eshell.  If
>> >> someone modifies Eshell's parsing to make it possible, I do not believe
>> >> it will interact with em-extpipe.el in any meaningful way.
>> >
>> > If it doesn't work now, then we cannot break it.
>>
>> Okay, so I should add something to skip over my parsing code in the case
>> that the first thing in the input is a redirection?
>
> I'm not sure I understand the intent: what do you want to accomplish
> by this special handling?

Perhaps I misunderstood your earlier message.  I thought that you were
saying that because Eshell does not support redirections at the very
beginning of the line at present, my new syntax should not support that
either, to prevent confusion.  Did you mean something else when you
wrote "if it doesn't work now, then we cannot break it?"

Thanks!

-- 
Sean Whitton

Message #109 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 46351 <at> debbugs.gnu.org
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Thu, 20 Jan 2022 15:17:49 -0700

Hello,

On Thu 20 Jan 2022 at 07:41PM +01, Michael Albinus wrote:

> However, if I understand your test cases, they check that the eshell
> commands are manipulated as you expect. The tests do not run the
> resulting command itself, checking the output. This is a little bit
> unfortune, because you could check that the output is indeed what you
> expect. And perhaps you could find some constellations, where the output
> is different when using either *| or |. This would be another proof that
> your changes work.
>
> And this would also give some guidance, where your approach has
> limitations (if exist). Showing also *failing* tests in one way or
> another is always a benefit. Your tests use only should, there is no
> should-not or should-error.
>
> Another benefit of testing working examples and their output is
> documentation. I always have trouble to understand documentation of
> unknown (to me) features in Emacs. I have the attitude to run the
> respective ert tests, and to study how the feature is applied by its
> developer. Often, it is more instructive than documentation, and I can
> steal the code.

I hadn't thought of tests as specifically useful alongside
documentation.  Thank you.  I will include some additional tests of this
nature in my next revision.

-- 
Sean Whitton

Message #112 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 46351 <at> debbugs.gnu.org, michael.albinus <at> gmx.de
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Fri, 21 Jan 2022 08:54:15 +0200

> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Cc: michael.albinus <at> gmx.de, 46351 <at> debbugs.gnu.org
> Date: Thu, 20 Jan 2022 15:16:20 -0700
> 
> >> Okay, so I should add something to skip over my parsing code in the case
> >> that the first thing in the input is a redirection?
> >
> > I'm not sure I understand the intent: what do you want to accomplish
> > by this special handling?
> 
> Perhaps I misunderstood your earlier message.  I thought that you were
> saying that because Eshell does not support redirections at the very
> beginning of the line at present, my new syntax should not support that
> either, to prevent confusion.  Did you mean something else when you
> wrote "if it doesn't work now, then we cannot break it?"

I meant that if Eshell doesn't support that syntax, you can do
whatever you want in that case.  You don't need to make any effort to
support it, and you don't need to make any effort not to support it:
whatever your code does in that case will be OK.

Message #115 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 46351 <at> debbugs.gnu.org, michael.albinus <at> gmx.de
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Fri, 21 Jan 2022 17:16:46 -0700

Hello,

On Fri 21 Jan 2022 at 08:54AM +02, Eli Zaretskii wrote:

>> From: Sean Whitton <spwhitton <at> spwhitton.name>
>> Cc: michael.albinus <at> gmx.de, 46351 <at> debbugs.gnu.org
>> Date: Thu, 20 Jan 2022 15:16:20 -0700
>>
>> >> Okay, so I should add something to skip over my parsing code in the case
>> >> that the first thing in the input is a redirection?
>> >
>> > I'm not sure I understand the intent: what do you want to accomplish
>> > by this special handling?
>>
>> Perhaps I misunderstood your earlier message.  I thought that you were
>> saying that because Eshell does not support redirections at the very
>> beginning of the line at present, my new syntax should not support that
>> either, to prevent confusion.  Did you mean something else when you
>> wrote "if it doesn't work now, then we cannot break it?"
>
> I meant that if Eshell doesn't support that syntax, you can do
> whatever you want in that case.  You don't need to make any effort to
> support it, and you don't need to make any effort not to support it:
> whatever your code does in that case will be OK.

Ah, okay, I see now.  Thanks.

-- 
Sean Whitton

Message #118 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Michael Albinus <michael.albinus <at> gmx.de>, 46351 <at> debbugs.gnu.org
Cc: Robert Pluim <rpluim <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Sun, 23 Jan 2022 15:39:02 -0700

[Message part 1 (text/plain, inline)]

Hello,

On Mon 17 Jan 2022 at 10:19PM -07, Sean Whitton wrote:

> I've got it working.  Please let me know what you think of the attached.

Attached is a revised series addressing feedback gratefully received
over the past week.

On Thu 20 Jan 2022 at 07:41PM +01, Michael Albinus wrote:

> However, if I understand your test cases, they check that the eshell
> commands are manipulated as you expect. The tests do not run the
> resulting command itself, checking the output. This is a little bit
> unfortune, because you could check that the output is indeed what you
> expect. And perhaps you could find some constellations, where the output
> is different when using either *| or |. This would be another proof that
> your changes work.
>
> And this would also give some guidance, where your approach has
> limitations (if exist). Showing also *failing* tests in one way or
> another is always a benefit. Your tests use only should, there is no
> should-not or should-error.

I've added actually running the commands and examining the results to
several of the tests.  I found a way to show different output in the
case of *| vs. | by using cl-letf to redefine some Lisp functions.

I also refactored the tests in the hope of increasing their value as a
supplement to the documentation.  Please let me know if you have any
other ideas.  Thanks!

-- 
Sean Whitton

[v4-0001-Move-Eshell-test-helpers-to-their-own-file.patch (text/x-patch, attachment)]

[v4-0002-Rework-eshell-match-result-for-testing-asynchrono.patch (text/x-patch, attachment)]

[v4-0003-Add-Eshell-syntax-to-more-easily-bypass-Eshell-s-.patch (text/x-patch, attachment)]

Message #121 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 46351 <at> debbugs.gnu.org, Robert Pluim <rpluim <at> gmail.com>,
 Michael Albinus <michael.albinus <at> gmx.de>, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Mon, 24 Jan 2022 10:55:06 +0100

Sean Whitton <spwhitton <at> spwhitton.name> writes:

> I've added actually running the commands and examining the results to
> several of the tests.  I found a way to show different output in the
> case of *| vs. | by using cl-letf to redefine some Lisp functions.

This leads to:

Test em-extpipe-test-14 condition:
    (ert-test-failed
     ((should
       (string-match-p regexp
		       (buffer-substring-no-properties ... ...)))
      :form
      (string-match-p "baz\nbar" "bazbar\n")
      :value nil))
   FAILED   6/16  em-extpipe-test-14 (0.119788 sec)


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #124 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 46351 <at> debbugs.gnu.org, Robert Pluim <rpluim <at> gmail.com>,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Mon, 24 Jan 2022 15:18:52 +0100

Sean Whitton <spwhitton <at> spwhitton.name> writes:

> Hello,

Hi Sean,

> I've added actually running the commands and examining the results to
> several of the tests.  I found a way to show different output in the
> case of *| vs. | by using cl-letf to redefine some Lisp functions.
>
> I also refactored the tests in the hope of increasing their value as a
> supplement to the documentation.  Please let me know if you have any
> other ideas.  Thanks!

Thanks. Besides the problem reported by Lars, just a minor comment:

> --- /dev/null
> +++ b/test/lisp/eshell/eshell-tests-helpers.el
> @@ -0,0 +1,90 @@
> +(provide 'eshell-tests)
> +
> +;;; eshell-tests.el ends here

This shall be eshell-tests-helpers in both cases.

Best regards, Michael.

Message #127 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Michael Albinus <michael.albinus <at> gmx.de>, Lars Ingebrigtsen
 <larsi <at> gnus.org>
Cc: 46351 <at> debbugs.gnu.org, Robert Pluim <rpluim <at> gmail.com>,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Mon, 24 Jan 2022 13:32:57 -0700

[Message part 1 (text/plain, inline)]

Hello,

On Mon 24 Jan 2022 at 10:55AM +01, Lars Ingebrigtsen wrote:

> This leads to:
>
> Test em-extpipe-test-14 condition:
>     (ert-test-failed
>      ((should
>        (string-match-p regexp
> 		       (buffer-substring-no-properties ... ...)))
>       :form
>       (string-match-p "baz\nbar" "bazbar\n")
>       :value nil))
>    FAILED   6/16  em-extpipe-test-14 (0.119788 sec)

I apologies for neglecting to run the tests in batch mode.  There was an
implicit dependency on my setting for require-final-newline.  Now fixed.

On Mon 24 Jan 2022 at 03:18PM +01, Michael Albinus wrote:

>> --- /dev/null
>> +++ b/test/lisp/eshell/eshell-tests-helpers.el
>> @@ -0,0 +1,90 @@
>> +(provide 'eshell-tests)
>> +
>> +;;; eshell-tests.el ends here
>
> This shall be eshell-tests-helpers in both cases.

Likewise fixed in the attached.

Thank you both.

-- 
Sean Whitton

[v5-0001-Move-Eshell-test-helpers-to-their-own-file.patch (text/x-patch, attachment)]

[v5-0002-Rework-eshell-match-result-for-testing-asynchrono.patch (text/x-patch, attachment)]

[v5-0003-Add-Eshell-syntax-to-more-easily-bypass-Eshell-s-.patch (text/x-patch, attachment)]

Message #130 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Michael Albinus <michael.albinus <at> gmx.de>, Lars Ingebrigtsen
 <larsi <at> gnus.org>
Cc: 46351 <at> debbugs.gnu.org, Robert Pluim <rpluim <at> gmail.com>,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Mon, 24 Jan 2022 13:44:41 -0700

[Message part 1 (text/plain, inline)]

Hello,

On Mon 24 Jan 2022 at 01:32PM -07, Sean Whitton wrote:

> On Mon 24 Jan 2022 at 03:18PM +01, Michael Albinus wrote:
>
>>> --- /dev/null
>>> +++ b/test/lisp/eshell/eshell-tests-helpers.el
>>> @@ -0,0 +1,90 @@
>>> +(provide 'eshell-tests)
>>> +
>>> +;;; eshell-tests.el ends here
>>
>> This shall be eshell-tests-helpers in both cases.
>
> Likewise fixed in the attached.

... but in the wrong patch, sigh.  Revision attached.

-- 
Sean Whitton

[v6-0001-Move-Eshell-test-helpers-to-their-own-file.patch (text/x-patch, attachment)]

[v6-0002-Rework-eshell-match-result-for-testing-asynchrono.patch (text/x-patch, attachment)]

[v6-0003-Add-Eshell-syntax-to-more-easily-bypass-Eshell-s-.patch (text/x-patch, attachment)]

Message #133 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 46351 <at> debbugs.gnu.org, Robert Pluim <rpluim <at> gmail.com>,
 Michael Albinus <michael.albinus <at> gmx.de>, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Mon, 24 Jan 2022 21:46:24 +0100

Sean Whitton <spwhitton <at> spwhitton.name> writes:

> Likewise fixed in the attached.

Thanks; pushed to Emacs 29.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #136 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 46351 <at> debbugs.gnu.org, Robert Pluim <rpluim <at> gmail.com>,
 Michael Albinus <michael.albinus <at> gmx.de>, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Mon, 24 Jan 2022 21:48:05 +0100

Sean Whitton <spwhitton <at> spwhitton.name> writes:

> ... but in the wrong patch, sigh.  Revision attached.

Oops; the old one is already pushed.  Can you supply an additional patch
for this?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #141 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 46351 <at> debbugs.gnu.org, Robert Pluim <rpluim <at> gmail.com>,
 Michael Albinus <michael.albinus <at> gmx.de>, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Mon, 24 Jan 2022 14:42:18 -0700

[Message part 1 (text/plain, inline)]

Hello,

On Mon 24 Jan 2022 at 09:48PM +01, Lars Ingebrigtsen wrote:

> Sean Whitton <spwhitton <at> spwhitton.name> writes:
>
>> ... but in the wrong patch, sigh.  Revision attached.
>
> Oops; the old one is already pushed.  Can you supply an additional patch
> for this?

I put the "... ends here" fix into 667e212048 rather than 1693423fd7
which is not now fixable.  However, I also noticed that
em-extpipe-tests.el lacks a `provide' form so here is a fix for that.
Pretty sure that's it for now.  Thank you for reviewing and applying!

-- 
Sean Whitton

[0001-Fix-missing-provide-in-recently-introduced-em-extpip.patch (text/x-patch, attachment)]

Message #144 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 46351 <at> debbugs.gnu.org, Robert Pluim <rpluim <at> gmail.com>,
 Michael Albinus <michael.albinus <at> gmx.de>, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Mon, 24 Jan 2022 22:51:01 +0100

Sean Whitton <spwhitton <at> spwhitton.name> writes:

> I put the "... ends here" fix into 667e212048 rather than 1693423fd7
> which is not now fixable.  However, I also noticed that
> em-extpipe-tests.el lacks a `provide' form so here is a fix for that.
> Pretty sure that's it for now.  Thank you for reviewing and applying!

We don't usually add provides in the test files, so that doesn't seem
necessary.  

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #147 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 46351 <at> debbugs.gnu.org, Robert Pluim <rpluim <at> gmail.com>,
 Michael Albinus <michael.albinus <at> gmx.de>, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Mon, 24 Jan 2022 15:48:42 -0700

Hello,

On Mon 24 Jan 2022 at 10:51PM +01, Lars Ingebrigtsen wrote:

> Sean Whitton <spwhitton <at> spwhitton.name> writes:
>
>> I put the "... ends here" fix into 667e212048 rather than 1693423fd7
>> which is not now fixable.  However, I also noticed that
>> em-extpipe-tests.el lacks a `provide' form so here is a fix for that.
>> Pretty sure that's it for now.  Thank you for reviewing and applying!
>
> We don't usually add provides in the test files, so that doesn't seem
> necessary.

That makes more sense to me, I just saw it in eshell-tests.el today.
Thanks again.

-- 
Sean Whitton

Message #150 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>, Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 46351 <at> debbugs.gnu.org, Robert Pluim <rpluim <at> gmail.com>,
 Michael Albinus <michael.albinus <at> gmx.de>
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Mon, 24 Jan 2022 18:39:15 -0800

On 1/24/2022 12:46 PM, Lars Ingebrigtsen wrote:
> Sean Whitton <spwhitton <at> spwhitton.name> writes:
> 
>> Likewise fixed in the attached.
> 
> Thanks; pushed to Emacs 29.

I just noticed a small bit of breakage with this. It's no longer 
possible to refer to Lisp functions in Eshell like so:

  #'upcase

Eshell explicitly supports this construct (see `eshell-lisp-regexp'), 
though it doesn't appear to be documented in the manual. Currently, this 
syntax is only occasionally useful, but I'm working on a patch series 
where it'll likely become a lot more common. My patches will add support 
for piping to Lisp functions, so that you can do the following, for example:

  $ echo hi | #'upcase
  HI

It looks like the breakage in parsing #'upcase is the result of 
`eshell-parse-external-pipeline' trying to skip over args like '*|' 
(with the quotes). However, it sees the single-quote in #'upcase and 
then calls `eshell-parse-literal-quote', resulting in the error message:

  Expecting completion of delimiter ' ...

Message #153 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 46351 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>,
 Robert Pluim <rpluim <at> gmail.com>, Sean Whitton <spwhitton <at> spwhitton.name>
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Tue, 25 Jan 2022 09:54:18 +0100

Jim Porter <jporterbugs <at> gmail.com> writes:

Hi Jim,

>   #'upcase
>
> Eshell explicitly supports this construct (see `eshell-lisp-regexp'),
> though it doesn't appear to be documented in the manual.

Shouldn't it?

Best regards, Michael.

Message #156 received at 46351 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 46351 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>,
 Robert Pluim <rpluim <at> gmail.com>, Sean Whitton <spwhitton <at> spwhitton.name>
Subject: Re: bug#46351: 28.0.50; Add convenient way to bypass Eshell's own
 pipelining
Date: Tue, 25 Jan 2022 10:22:46 -0800

On 1/25/2022 12:54 AM, Michael Albinus wrote:
> Jim Porter <jporterbugs <at> gmail.com> writes:
> 
> Hi Jim,
> 
>>    #'upcase
>>
>> Eshell explicitly supports this construct (see `eshell-lisp-regexp'),
>> though it doesn't appear to be documented in the manual.
> 
> Shouldn't it?

Do you mean "shouldn't it be documented in the manual"? If so, I agree. 
I'll be sure to explain this in the manual alongside my patch to add 
support for piping to Lisp functions in Eshell. (It seems better to do 
it all at once since if I updated the manual now, I'd probably have to 
update those parts again to account for the pipeline changes.)

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.