bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Sean Whitton]

[-- Attachment #1: Type: text/plain, Size: 1359 bytes --]

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-eshell-restore-unexpanded-input.patch --]
[-- Type: text/x-diff, Size: 2458 bytes --]

From ccc4427ff06bf6dc63840517aefbb7fb899d4b8c Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Sat, 6 Feb 2021 00:01:50 -0700
Subject: [PATCH 1/2] Add eshell-restore-unexpanded-input

* lisp/eshell/esh-mode.el (eshell-send-input): Store the original
input before running eshell-expand-input-functions.
* lisp/eshell/esh-mode.el (eshell-restore-unexpanded-input): Define
new function and register as a customization option for
eshell-input-filter-functions.
---
 lisp/eshell/esh-mode.el | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/lisp/eshell/esh-mode.el b/lisp/eshell/esh-mode.el
index d29b010ea0..6036829941 100644
--- a/lisp/eshell/esh-mode.el
+++ b/lisp/eshell/esh-mode.el
@@ -197,6 +197,7 @@ This is used by `eshell-watch-for-password-prompt'."
 ;; byte-compiler, when compiling other files which `require' this one
 (defvar eshell-mode nil)
 (defvar eshell-command-running-string "--")
+(defvar eshell-last-unexpanded-input nil)
 (defvar eshell-last-input-start nil)
 (defvar eshell-last-input-end nil)
 (defvar eshell-last-output-start nil)
@@ -631,6 +632,7 @@ newline."
 		(progn
 		  (setq input (buffer-substring-no-properties
 			       eshell-last-output-end (1- (point))))
+                  (setq eshell-last-unexpanded-input input)
 		  (run-hook-with-args 'eshell-expand-input-functions
 				      eshell-last-output-end (1- (point)))
 		  (let ((cmd (eshell-parse-command-input
@@ -664,6 +666,21 @@ newline."
 
 (custom-add-option 'eshell-input-filter-functions 'eshell-kill-new)
 
+(defun eshell-restore-unexpanded-input ()
+  "Restore the input text before `eshell-expand-input-functions' ran.
+Useful when you want to see the unexpanded input rather than the
+expanded input in the Eshell buffer, and when you want later
+entries in `eshell-input-filter-functions' to use the unexpanded
+input.  For example, you might want the unexpanded input to be
+what gets stored in the history list."
+  (setf (buffer-substring eshell-last-input-start
+                          (1- eshell-last-input-end))
+        eshell-last-unexpanded-input)
+  (eshell-update-markers eshell-last-output-end))
+
+(custom-add-option 'eshell-input-filter-functions
+                   'eshell-restore-unexpanded-input)
+
 (defun eshell-output-filter (process string)
   "Send the output from PROCESS (STRING) to the interactive display.
 This is done after all necessary filtering has been done."
-- 
2.29.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Add-eshell-shell-command-and-eshell-expand-to-eshell.patch --]
[-- Type: text/x-diff, Size: 4354 bytes --]

From d5d48427c75555cd5cf2f6eb5b89e0c832cf7edf Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Sat, 6 Feb 2021 00:48:32 -0700
Subject: [PATCH 2/2] Add eshell-shell-command and
 eshell-expand-to-eshell-shell-command

* lisp/eshell/esh-mode.el (eshell-shell-command,
eshell-expand-to-eshell-shell-command): Define new functions.
Register eshell-expand-to-eshell-shell-command as a customization
option for eshell-expand-input-functions.
---
 lisp/eshell/esh-mode.el | 77 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)

diff --git a/lisp/eshell/esh-mode.el b/lisp/eshell/esh-mode.el
index 6036829941..b3132393a5 100644
--- a/lisp/eshell/esh-mode.el
+++ b/lisp/eshell/esh-mode.el
@@ -62,6 +62,7 @@
 (require 'esh-module)
 (require 'esh-cmd)
 (require 'esh-arg)                      ;For eshell-parse-arguments
+(require 'subr-x)
 
 (defgroup eshell-mode nil
   "This module contains code for handling input from the user."
@@ -660,6 +661,82 @@ newline."
 	       (run-hooks 'eshell-post-command-hook)
 	       (insert-and-inherit input)))))))))
 
+(defun eshell-shell-command (command)
+  "Execute COMMAND using the operating system shell."
+  (throw 'eshell-replace-command
+	 (eshell-parse-command shell-file-name
+			       (list shell-command-switch command))))
+
+(defun eshell-expand-to-eshell-shell-command (beg end)
+  "Expand Eshell input starting with '!!' to use `eshell-shell-command'.
+
+This is intended to provide a convenient way to bypass Eshell's
+own pipelining which can be slow when executing shell pipelines
+which move a lot of data.  It is also useful to avoid
+roundtripping data when running pipelines on remote hosts.
+
+To enable, add this function to `eshell-expand-input-functions'.
+
+For example, this function expands the input
+
+    !! cat *.ogg | my-cool-decoder >file
+
+to
+
+    eshell-shell-command \"cat *.ogg | my-cool-decoder >file\"
+
+Executing the latter command will not copy all the data in the
+*.ogg files into Emacs buffers, as would normally happen with
+Eshell's own pipelining.
+
+This function tries to extract Eshell-specific redirects and
+avoids passing these to the operating system shell.  For example,
+
+    !! foo | bar >>#<buffer scratch>
+
+will be expanded to
+
+    eshell-shell-command \"foo | bar\" >>#<buffer scratch>
+
+This function works well in combination with adding
+`eshell-restore-unexpanded-input' to `eshell-input-filter-functions'."
+  (save-excursion
+    (goto-char beg)
+    (when (looking-at "!!\\s-*")
+      (let ((end (copy-marker end)) ; needs to be a marker
+            redirects)
+        ;; drop the !!
+        (delete-region beg (match-end 0))
+        ;; extract Eshell-specific redirects
+        (while (search-forward-regexp "[0-9]?>+&?[0-9]?\\s-*\\S-" nil t)
+          (let ((beg (match-beginning 0)))
+            (forward-char -1) ; start from the redirect target
+            (when-let ((end (cond
+                             ;; this is a redirect to a process or a
+                             ;; buffer
+                             ((looking-at "#<")
+                              (forward-char 1)
+                              (1+ (eshell-find-delimiter ?\< ?\>)))
+                             ;; this is a redirect to a virtual target
+                             ((and (looking-at "/\\S-+")
+                                   (assoc (match-string 0)
+                                          eshell-virtual-targets))
+                              (match-end 0)))))
+              (push (buffer-substring-no-properties beg end) redirects)
+              (delete-region beg end)
+              (just-one-space))))
+        ;; wrap the remaining text and reinstate Eshell redirects
+        (let ((pipeline (string-trim
+                         (buffer-substring-no-properties beg end))))
+          (delete-region beg end)
+          (insert "eshell-shell-command ")
+          (prin1 pipeline (current-buffer))
+          (when redirects
+            (insert " " (string-join (nreverse redirects) " "))))))))
+
+(custom-add-option 'eshell-expand-input-functions
+                   'eshell-expand-to-eshell-shell-command)
+
 (defsubst eshell-kill-new ()
   "Add the last input text to the kill ring."
   (kill-ring-save eshell-last-input-start eshell-last-input-end))
-- 
2.29.2

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Michael Albinus]

Sean Whitton <spwhitton@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.

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Sean Whitton]

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

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Michael Albinus]

Sean Whitton <spwhitton@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.

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Sean Whitton]

Hello,

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

> Sean Whitton <spwhitton@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

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Michael Albinus]

Sean Whitton <spwhitton@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.

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Sean Whitton]

[-- Attachment #1: Type: text/plain, Size: 1134 bytes --]

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v2-0001-Add-eshell-restore-unexpanded-input.patch --]
[-- Type: text/x-patch, Size: 2720 bytes --]

From d47446cbe7db5c7fb4eba71ab022cfae3de07799 Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Sat, 6 Feb 2021 00:01:50 -0700
Subject: [PATCH v2 1/2] Add eshell-restore-unexpanded-input

* lisp/eshell/esh-mode.el (eshell-send-input): Store the original
input before running eshell-expand-input-functions.
* lisp/eshell/esh-mode.el (eshell-restore-unexpanded-input): Define
new function and register as a customization option for
eshell-input-filter-functions.
---
 lisp/eshell/esh-mode.el | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/lisp/eshell/esh-mode.el b/lisp/eshell/esh-mode.el
index cae5236d89..87166ef3bf 100644
--- a/lisp/eshell/esh-mode.el
+++ b/lisp/eshell/esh-mode.el
@@ -62,6 +62,7 @@
 (require 'esh-module)
 (require 'esh-cmd)
 (require 'esh-arg)                      ;For eshell-parse-arguments
+(require 'cl-lib)
 
 (defgroup eshell-mode nil
   "This module contains code for handling input from the user."
@@ -197,6 +198,7 @@ eshell-first-time-p
 ;; byte-compiler, when compiling other files which `require' this one
 (defvar eshell-mode nil)
 (defvar eshell-command-running-string "--")
+(defvar eshell-last-unexpanded-input nil)
 (defvar eshell-last-input-start nil)
 (defvar eshell-last-input-end nil)
 (defvar eshell-last-output-start nil)
@@ -641,6 +643,7 @@ eshell-send-input
 		(progn
 		  (setq input (buffer-substring-no-properties
 			       eshell-last-output-end (1- (point))))
+                  (setq eshell-last-unexpanded-input input)
 		  (run-hook-with-args 'eshell-expand-input-functions
 				      eshell-last-output-end (1- (point)))
 		  (let ((cmd (eshell-parse-command-input
@@ -674,6 +677,23 @@ eshell-kill-new
 
 (custom-add-option 'eshell-input-filter-functions 'eshell-kill-new)
 
+(defun eshell-restore-unexpanded-input ()
+  "Restore the input text before `eshell-expand-input-functions' ran.
+Useful when you want to see the unexpanded input rather than the
+expanded input in the Eshell buffer, and when you want later
+entries in `eshell-input-filter-functions' to use the unexpanded
+input."
+  (setf (buffer-substring eshell-last-input-start
+                          (1- eshell-last-input-end))
+        eshell-last-unexpanded-input)
+  ;; We have to move point for compatibility with smart display.
+  (save-excursion
+    (goto-char eshell-last-input-end)
+    (eshell-update-markers eshell-last-output-end)))
+
+(custom-add-option 'eshell-input-filter-functions
+                   'eshell-restore-unexpanded-input)
+
 (defun eshell-output-filter (process string)
   "Send the output from PROCESS (STRING) to the interactive display.
 This is done after all necessary filtering has been done."
-- 
2.30.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: v2-0002-Add-eshell-shell-command-and-eshell-expand-to-esh.patch --]
[-- Type: text/x-patch, Size: 7533 bytes --]

From 98df5163018b9c99ad8e65f03422f908c6068c78 Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Sat, 6 Feb 2021 00:48:32 -0700
Subject: [PATCH v2 2/2] Add eshell-shell-command and
 eshell-expand-to-eshell-shell-command

* lisp/eshell/esh-mode.el (eshell-shell-command,
eshell-expand-to-eshell-shell-command): Define new functions.
Register eshell-expand-to-eshell-shell-command as a customization
option for eshell-expand-input-functions, and add it by default.
* etc/NEWS:
* doc/misc/eshell.texi: Document the new syntax.
---
 doc/misc/eshell.texi    | 35 ++++++++++++++++++
 etc/NEWS                |  9 +++++
 lisp/eshell/esh-mode.el | 79 ++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 122 insertions(+), 1 deletion(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index a87dd4308c..7e9233c09e 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -875,6 +875,7 @@ Expansion
 @menu
 * Dollars Expansion::
 * Globbing::
+* Running Shell Pipelines Natively::
 @end menu
 
 @node Dollars Expansion
@@ -945,6 +946,40 @@ Globbing
 The GNU Emacs Manual}.}
 groups ``eshell-glob'' and ``eshell-pred''.
 
+@node Running Shell Pipelines Natively
+@section Running Shell Pipelines Natively
+When constructing shell pipelines that will move a lot of data, it is
+a good idea to bypass Eshell's own pipelining support and use the
+operating system shell's instead.  This is especially relevant when
+executing commands on a remote machine using Eshell's Tramp
+integration: using the remote shell's pipelining avoids copying the
+data which will flow through the pipeline to local Emacs buffers and
+then right back again.
+
+To quickly construct a command for the native shell, prefix your
+Eshell input with the string @code{||}.  For example,
+
+@example
+|| cat *.ogg | my-cool-decoder >file
+@end example
+
+Executing this command will not copy all the data in the *.ogg files
+into Emacs buffers, as would normally happen with Eshell's own
+pipelining.
+
+As this feature is implemented as an expansion, your original input
+will be replaced with something like this:
+
+@example
+eshell-shell-command \"cat *.ogg | my-cool-decoder >file\"
+@end example
+
+This makes it clear what Eshell is doing, but has the disadvantage of
+being harder to edit, especially for complex pipelines with a lot of
+quoting.  If you would prefer that the @code{||}-prefixed input be
+restored, customize the variable @code{eshell-input-filter-functions}
+to include the function @code{eshell-restore-unexpanded-input}.
+
 @node Input/Output
 @chapter Input/Output
 Since Eshell does not communicate with a terminal like most command
diff --git a/etc/NEWS b/etc/NEWS
index 57fe40c488..767763098b 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -749,6 +749,15 @@ the Netscape web browser was released in February, 2008.
 This support has been obsolete since Emacs 25.1.  The final version of
 the Galeon web browser was released in September, 2008.
 
+** Eshell
+
++++
+*** New feature to easily bypass Eshell's own pipelining.
+Prefixing commands with '||' causes them to be executed using the
+operating system shell, which is particularly useful to bypass
+Eshell's own pipelining for pipelines which will move a lot of data.
+See "Running Shell Pipelines Natively" in the Eshell manual.
+
 \f
 * New Modes and Packages in Emacs 29.1
 
diff --git a/lisp/eshell/esh-mode.el b/lisp/eshell/esh-mode.el
index 87166ef3bf..04680e4305 100644
--- a/lisp/eshell/esh-mode.el
+++ b/lisp/eshell/esh-mode.el
@@ -63,6 +63,7 @@
 (require 'esh-cmd)
 (require 'esh-arg)                      ;For eshell-parse-arguments
 (require 'cl-lib)
+(require 'subr-x)
 
 (defgroup eshell-mode nil
   "This module contains code for handling input from the user."
@@ -105,7 +106,8 @@ eshell-send-direct-to-subprocesses
   "If t, send any input immediately to a subprocess."
   :type 'boolean)
 
-(defcustom eshell-expand-input-functions nil
+(defcustom eshell-expand-input-functions
+  '(eshell-expand-to-eshell-shell-command)
   "Functions to call before input is parsed.
 Each function is passed two arguments, which bounds the region of the
 current input text."
@@ -671,6 +673,81 @@ eshell-send-input
 	       (run-hooks 'eshell-post-command-hook)
 	       (insert-and-inherit input)))))))))
 
+(defun eshell-shell-command (command)
+  "Execute COMMAND using the operating system shell."
+  (throw 'eshell-replace-command
+         (with-connection-local-variables
+	  (eshell-parse-command shell-file-name
+			        (list shell-command-switch command)))))
+
+(defun eshell-expand-to-eshell-shell-command (beg end)
+  "Expand Eshell input starting with '||' to use `eshell-shell-command'.
+
+This is intended to provide a convenient way to bypass Eshell's
+own pipelining which can be slow when executing shell pipelines
+which move a lot of data.  It is also useful to avoid
+roundtripping data when running pipelines on remote hosts.
+
+For example, this function expands the input
+
+    || cat *.ogg | my-cool-decoder >file
+
+to
+
+    eshell-shell-command \"cat *.ogg | my-cool-decoder >file\"
+
+Executing the latter command will not copy all the data in the
+*.ogg files into Emacs buffers, as would normally happen with
+Eshell's own pipelining.
+
+This function tries to extract Eshell-specific redirects and
+avoids passing these to the operating system shell.  For example,
+
+    || foo | bar >>#<buffer scratch>
+
+will be expanded to
+
+    eshell-shell-command \"foo | bar\" >>#<buffer scratch>
+
+This function works well in combination with adding
+`eshell-restore-unexpanded-input' to `eshell-input-filter-functions'."
+  (save-excursion
+    (goto-char beg)
+    (when (looking-at "||\\s-*")
+      (let ((end (copy-marker end)) ; needs to be a marker
+            redirects)
+        ;; drop the ||
+        (delete-region beg (match-end 0))
+        ;; extract Eshell-specific redirects
+        (while (search-forward-regexp "[0-9]?>+&?[0-9]?\\s-*\\S-" nil t)
+          (let ((beg (match-beginning 0)))
+            (forward-char -1) ; start from the redirect target
+            (when-let ((end (cond
+                             ;; this is a redirect to a process or a
+                             ;; buffer
+                             ((looking-at "#<")
+                              (forward-char 1)
+                              (1+ (eshell-find-delimiter ?\< ?\>)))
+                             ;; this is a redirect to a virtual target
+                             ((and (looking-at "/\\S-+")
+                                   (assoc (match-string 0)
+                                          eshell-virtual-targets))
+                              (match-end 0)))))
+              (push (buffer-substring-no-properties beg end) redirects)
+              (delete-region beg end)
+              (just-one-space))))
+        ;; wrap the remaining text and reinstate Eshell redirects
+        (let ((pipeline (string-trim
+                         (buffer-substring-no-properties beg end))))
+          (delete-region beg end)
+          (insert "eshell-shell-command ")
+          (prin1 pipeline (current-buffer))
+          (when redirects
+            (insert " " (string-join (nreverse redirects) " "))))))))
+
+(custom-add-option 'eshell-expand-input-functions
+                   'eshell-expand-to-eshell-shell-command)
+
 (defsubst eshell-kill-new ()
   "Add the last input text to the kill ring."
   (kill-ring-save eshell-last-input-start eshell-last-input-end))
-- 
2.30.2

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Michael Albinus]

Sean Whitton <spwhitton@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.

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Sean Whitton]

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

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Michael Albinus]

Sean Whitton <spwhitton@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.

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Sean Whitton]

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

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Eli Zaretskii]

> From: Sean Whitton <spwhitton@spwhitton.name>
> Date: Mon, 27 Dec 2021 11:13:29 -0700
> Cc: 46351@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.

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Michael Albinus]

Sean Whitton <spwhitton@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.

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Sean Whitton]

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

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Eli Zaretskii]

> From: Sean Whitton <spwhitton@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.

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Michael Albinus]

Sean Whitton <spwhitton@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.

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Sean Whitton]

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

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Sean Whitton]

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

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Michael Albinus]

Sean Whitton <spwhitton@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.

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Sean Whitton]

[-- Attachment #1: Type: text/plain, Size: 307 bytes --]

Hello,

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

> Sean Whitton <spwhitton@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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v3-0001-Add-Eshell-syntax-to-more-easily-bypass-Eshell-s-.patch --]
[-- Type: text/x-patch, Size: 15907 bytes --]

From 09c2cb2b43e9b988b682d13da8bf486a0a354333 Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Mon, 17 Jan 2022 15:15:36 -0700
Subject: [PATCH v3] Add Eshell syntax to more easily bypass Eshell's own
 pipelining

* etc/NEWS:
* doc/misc/eshell.texi (Input/Output): Document the new syntax.
* lisp/eshell/em-extpipe.el: New module.
* test/lisp/eshell/em-extpipe-tests.el: New tests.
* lisp/eshell/esh-module.el (eshell-modules-list): Add `eshell-extpipe'.
---
 doc/misc/eshell.texi                 |  39 ++++++
 etc/NEWS                             |  10 ++
 lisp/eshell/em-extpipe.el            | 175 +++++++++++++++++++++++++++
 lisp/eshell/esh-module.el            |   1 +
 test/lisp/eshell/em-extpipe-tests.el | 122 +++++++++++++++++++
 5 files changed, 347 insertions(+)
 create mode 100644 lisp/eshell/em-extpipe.el
 create mode 100644 test/lisp/eshell/em-extpipe-tests.el

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index f1d7c63805..d19fe6fa80 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1135,6 +1135,45 @@ Input/Output
 The output function is called once on each line of output until
 @code{nil} is passed, indicating end of output.
 
+@section Running Shell Pipelines Natively
+When constructing shell pipelines that will move a lot of data, it is
+a good idea to bypass Eshell's own pipelining support and use the
+operating system shell's instead.  This is especially relevant when
+executing commands on a remote machine using Eshell's Tramp
+integration: using the remote shell's pipelining avoids copying the
+data which will flow through the pipeline to local Emacs buffers and
+then right back again.
+
+Eshell recognises a special syntax to make it easier to convert
+pipelines so as to bypass Eshell's pipelining.  Prefixing at least one
+@code{|}, @code{<} or @code{>} with an asterisk marks a command as
+intended for the operating system shell.  For example,
+
+@example
+ cat *.ogg *| my-cool-decoder >file
+@end example
+
+Executing this command will not copy all the data in the *.ogg files,
+nor the decoded data, into Emacs buffers, as would normally happen.
+
+The command is interpreted as extending up to the next
+asterisk-unprefixed @code{|} character, or the end of the input if
+there is no such character.  Thus, all @code{<} and @code{>}
+redirections occuring before the next asterisk-unprefixed @code{|} are
+implicitly prefixed with asterisks.  An exception is that
+Eshell-specific redirects right at the end of the command are
+excluded.  This allows input like this:
+
+@example
+ foo *| baz >#<buffer quux>
+@end example
+
+@noindent which is equivalent to input like this:
+
+@example
+ sh -c "foo | baz" >#<buffer quux>
+@end example
+
 @node Extension modules
 @chapter Extension modules
 Eshell provides a facility for defining extension modules so that they
diff --git a/etc/NEWS b/etc/NEWS
index fdbfd9b1be..144844dc6a 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -834,6 +834,16 @@ the Netscape web browser was released in February, 2008.
 This support has been obsolete since Emacs 25.1.  The final version of
 the Galeon web browser was released in September, 2008.
 
+** Eshell
+
++++
+*** New feature to easily bypass Eshell's own pipelining.
+Prefixing '|', '<' or '>' with an asterisk, i.e. '*|', '*<' or '*>',
+will cause the whole command to be passed to the operating system
+shell.  This is particularly useful to bypass Eshell's own pipelining
+support for pipelines which will move a lot of data.  See "Running
+Shell Pipelines Natively" in the Eshell manual.
+
 ** Miscellaneous
 
 ---
diff --git a/lisp/eshell/em-extpipe.el b/lisp/eshell/em-extpipe.el
new file mode 100644
index 0000000000..a877c5624e
--- /dev/null
+++ b/lisp/eshell/em-extpipe.el
@@ -0,0 +1,175 @@
+;;; em-extpipe.el --- external shell pipelines  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2022 Free Software Foundation, Inc.
+
+;; Author: Sean Whitton <spwhitton@spwhitton.name>
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; When constructing shell pipelines that will move a lot of data, it
+;; is a good idea to bypass Eshell's own pipelining support and use
+;; the operating system shell's instead.  This module tries to make
+;; that easy to do.
+
+;;; Code:
+
+(require 'cl-lib)
+(require 'esh-arg)
+(require 'esh-io)
+(require 'esh-util)
+
+(eval-when-compile (require 'files-x))
+
+;;; Functions:
+
+(defun eshell-extpipe-initialize () ;Called from `eshell-mode' via intern-soft!
+  "Initialize external pipelines support."
+  (when (boundp 'eshell-special-chars-outside-quoting)
+    (setq-local
+     eshell-special-chars-outside-quoting
+     (append eshell-special-chars-outside-quoting (list ?\*))))
+  (add-hook 'eshell-parse-argument-hook
+            #'eshell-parse-external-pipeline -20 t)
+  (add-hook 'eshell-pre-rewrite-command-hook
+            #'eshell-rewrite-external-pipeline -20 t))
+
+(defun eshell-parse-external-pipeline ()
+  "Parse a pipeline intended for execution by the external shell.
+
+A sequence of arguments is rewritten to use the operating system
+shell when it contains `*|', `*<' or `*>'.  The command extends
+to the next `|' character which is not preceded by an unescaped
+asterisk, or the end of input, except that any Eshell-specific
+output redirections occurring at the end are excluded.  Any other
+`<' or `>' appearing before the end of the command are treated as
+though preceded by an asterisk.
+
+For example,
+
+    foo <bar *| baz >#<buffer quux>
+
+is equivalent to
+
+    sh -c \"foo <bar | baz\" >#<buffer quux>
+
+when `shell-file-name' is `sh' and `shell-command-switch' is
+`-c', but in
+
+    foo >#<buffer quux> *| baz
+
+and
+
+    foo *| baz >#<buffer quux> --some-argument
+
+the Eshell-specific redirect will be passed on to the operating
+system shell, probably leading to undesired results.
+
+This function must appear early in `eshell-parse-argument-hook'
+to ensure that operating system shell syntax is not interpreted
+as though it were Eshell syntax."
+  ;; Our aim is to wrap the external command to protect it from the
+  ;; other members of `eshell-parse-argument-hook'.  We must avoid
+  ;; misinterpreting a quoted `*|', `*<' or `*>' as indicating an
+  ;; external pipeline, hence the structure of the loop in `findbeg'.
+  (cl-flet
+      ((findbeg (pat &optional go (bound (point-max)))
+         (let* ((start (point))
+                (result
+                 (catch 'found
+                   (while (> bound (point))
+                     (let* ((found
+                             (save-excursion
+                               (re-search-forward "['\"\\]" bound t)))
+                            (next (or (and found (match-beginning 0))
+                                      bound)))
+                       (if (re-search-forward pat next t)
+                           (throw 'found (match-beginning 0))
+                         (goto-char next)
+                         (while (or (eshell-parse-backslash)
+                                    (eshell-parse-double-quote)
+                                    (eshell-parse-literal-quote)))))))))
+           (goto-char (if (and result go) (match-end 0) start))
+           result)))
+    (unless (or eshell-current-argument eshell-current-quoted)
+      (let ((beg (point)) end
+            (next-marked (findbeg "\\*[|<>]"))
+            (next-unmarked
+             (or (findbeg "\\(\\=\\|[^*]\\)|") (point-max))))
+        (when (and next-marked (> next-unmarked next-marked))
+          ;; Skip to the final segment of the external pipeline.
+          (while (findbeg "\\*|" t))
+          ;; Find output redirections.
+          (while (findbeg "[0-9]?>+&?[0-9]?\\s-*\\S-" t next-unmarked)
+            ;; Is the output redirection Eshell-specific?  We have our
+            ;; own logic, rather than calling `eshell-parse-argument',
+            ;; to avoid specifying here all the possible cars of
+            ;; parsed special references -- `get-buffer-create' etc.
+            (forward-char -1)
+            (let ((this-end
+                   (save-match-data
+                     (cond ((looking-at "#<")
+                            (forward-char 1)
+                            (1+ (eshell-find-delimiter ?\< ?\>)))
+                           ((and (looking-at "/\\S-+")
+                                 (assoc (match-string 0)
+                                        eshell-virtual-targets))
+                            (match-end 0))))))
+              (cond ((and this-end end)
+                     (goto-char this-end))
+                    (this-end
+                     (goto-char this-end)
+                     (setq end (match-beginning 0)))
+                    (t
+                     (setq end nil)))))
+          ;; We've moved past all Eshell-specific output redirections
+          ;; we could find.  If there is only whitespace left, then
+          ;; `end' is right before redirections we should exclude;
+          ;; otherwise, we must include everything.
+          (unless (and end (skip-syntax-forward "\s" next-unmarked)
+                       (= next-unmarked (point)))
+            (setq end next-unmarked))
+          (let ((cmd (string-trim
+                      (buffer-substring-no-properties beg end))))
+            (goto-char end)
+            ;; We must now drop the asterisks, unless quoted/escaped.
+            (with-temp-buffer
+              (insert cmd)
+              (goto-char (point-min))
+              (cl-loop for next = (findbeg "\\*[|<>]" t) while next
+                       do (forward-char -2) (delete-char 1))
+              (eshell-finish-arg
+               `(eshell-external-pipeline ,(buffer-string))))))))))
+
+(defun eshell-rewrite-external-pipeline (terms)
+  "Rewrite an external pipeline in TERMS as parsed by
+`eshell-parse-external-pipeline', which see."
+  (while terms
+    (when (and (listp (car terms))
+               (eq (caar terms) 'eshell-external-pipeline))
+      (with-connection-local-variables
+       (setcdr terms (cl-list*
+                      shell-command-switch (cadar terms) (cdr terms)))
+       (setcar terms shell-file-name)))
+    (setq terms (cdr terms))))
+
+(defsubst eshell-external-pipeline (&rest _args)
+  "Stub to generate an error if a pipeline is not rewritten."
+  (error "Unhandled external pipeline in input text"))
+
+(provide 'em-extpipe)
+;;; esh-extpipe.el ends here
diff --git a/lisp/eshell/esh-module.el b/lisp/eshell/esh-module.el
index ade151d7cd..14e91912d1 100644
--- a/lisp/eshell/esh-module.el
+++ b/lisp/eshell/esh-module.el
@@ -54,6 +54,7 @@ eshell-modules-list
     eshell-basic
     eshell-cmpl
     eshell-dirs
+    eshell-extpipe
     eshell-glob
     eshell-hist
     eshell-ls
diff --git a/test/lisp/eshell/em-extpipe-tests.el b/test/lisp/eshell/em-extpipe-tests.el
new file mode 100644
index 0000000000..01074c95c8
--- /dev/null
+++ b/test/lisp/eshell/em-extpipe-tests.el
@@ -0,0 +1,122 @@
+;;; em-extpipe-tests.el --- em-extpipe test suite  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2022 Free Software Foundation, Inc.
+
+;; Author: Sean Whitton <spwhitton@spwhitton.name>
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+
+;;; Code:
+
+(require 'cl-lib)
+(require 'ert)
+(require 'em-extpipe)
+(load (expand-file-name "eshell-tests"
+                        (file-name-directory (or load-file-name
+                                                 default-directory))))
+
+(declare-function with-temp-eshell "eshell-tests")
+
+(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")))
+
+  (deftest em-extpipe-test-2
+           "foo | bar *>baz"
+           '(eshell-execute-pipeline
+             '((eshell-named-command "foo")
+               (eshell-named-command "sh" (list "-c" "bar >baz")))))
+
+  (deftest em-extpipe-test-3
+           "foo *| bar | baz -d"
+           '(eshell-execute-pipeline
+             '((eshell-named-command "sh" (list "-c" "foo | bar"))
+               (eshell-named-command "baz" (list "-d")))))
+
+  (deftest em-extpipe-test-4
+           "foo *| bar >#<buffer quux>"
+           '(progn
+	      (ignore
+	       (eshell-set-output-handle 1 'overwrite
+				         (get-buffer-create "quux")))
+	      (eshell-named-command "sh"
+			            (list "-c" "foo | bar"))))
+  (deftest em-extpipe-test-5
+           "foo *| bar >#<buffer quux> baz"
+           '(eshell-named-command
+             "sh"
+             (list "-c" "foo | bar >#<buffer quux> baz")))
+
+  (deftest em-extpipe-test-5
+           "foo >#<buffer quux> *| bar baz"
+           '(eshell-named-command
+             "sh"
+             (list "-c" "foo >#<buffer quux> | bar baz")))
+
+  (deftest em-extpipe-test-7
+           "foo *| bar >#<buffer quux> >>#<process other>"
+           '(progn
+	      (ignore
+	       (eshell-set-output-handle 1 'overwrite
+				         (get-buffer-create "quux")))
+              (ignore
+	       (eshell-set-output-handle 1 'append
+				         (get-process "other")))
+	      (eshell-named-command "sh"
+			            (list "-c" "foo | bar"))))
+
+  (deftest em-extpipe-test-8
+           "foo *| bar >/dev/kill | baz"
+           '(eshell-execute-pipeline
+             '((progn
+	         (ignore
+	          (eshell-set-output-handle 1 'overwrite "/dev/kill"))
+	         (eshell-named-command "sh"
+			               (list "-c" "foo | bar")))
+               (eshell-named-command "baz"))))
+
+  (deftest em-extpipe-test-9
+           "foo \\*| bar"
+           '(eshell-execute-pipeline
+             '((eshell-named-command "foo"
+                                     (list (eshell-escape-arg "*")))
+               (eshell-named-command "bar"))))
+
+  (deftest em-extpipe-test-10
+           "foo \"*|\" *>bar"
+           '(eshell-named-command "sh"
+                                  (list "-c" "foo \"*|\" >bar")))
+
+  (deftest em-extpipe-test-11
+           "foo '*|' bar"
+           '(eshell-named-command
+             "foo" (list (eshell-escape-arg "*|") "bar"))))
+
+;;; em-extpipe-tests.el ends here
-- 
2.30.2

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Robert Pluim]

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

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

    >> Sean Whitton <spwhitton@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
-- 

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Eli Zaretskii]

> From: Sean Whitton <spwhitton@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?

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Sean Whitton]

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

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Sean Whitton]

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

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Sean Whitton]

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

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Eli Zaretskii]

> From: Sean Whitton <spwhitton@spwhitton.name>
> Cc: michael.albinus@gmx.de, 46351@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?

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Sean Whitton]

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

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Eli Zaretskii]

> From: Sean Whitton <spwhitton@spwhitton.name>
> Cc: michael.albinus@gmx.de, 46351@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.

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Michael Albinus]

Sean Whitton <spwhitton@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.

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Sean Whitton]

Hello,

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

>> From: Sean Whitton <spwhitton@spwhitton.name>
>> Cc: michael.albinus@gmx.de, 46351@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

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Sean Whitton]

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

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Eli Zaretskii]

> From: Sean Whitton <spwhitton@spwhitton.name>
> Cc: michael.albinus@gmx.de, 46351@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?

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Michael Albinus]

Sean Whitton <spwhitton@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.

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Sean Whitton]

Hello,

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

>> From: Sean Whitton <spwhitton@spwhitton.name>
>> Cc: michael.albinus@gmx.de, 46351@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

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Sean Whitton]

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

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Eli Zaretskii]

> From: Sean Whitton <spwhitton@spwhitton.name>
> Cc: michael.albinus@gmx.de, 46351@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.

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Sean Whitton]

Hello,

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

>> From: Sean Whitton <spwhitton@spwhitton.name>
>> Cc: michael.albinus@gmx.de, 46351@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

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Sean Whitton]

[-- Attachment #1: Type: text/plain, Size: 1366 bytes --]

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v4-0001-Move-Eshell-test-helpers-to-their-own-file.patch --]
[-- Type: text/x-patch, Size: 6953 bytes --]

From 1cfae74bbf78093c84bebaa6dc97bc887238287c Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Fri, 21 Jan 2022 22:32:22 -0700
Subject: [PATCH v4 1/3] Move Eshell test helpers to their own file

* test/lisp/eshell/eshell-tests.el:
* test/lisp/eshell/eshell-tests-helpers.el: Move helpers to own file.
---
 test/lisp/eshell/eshell-tests-helpers.el | 90 ++++++++++++++++++++++++
 test/lisp/eshell/eshell-tests.el         | 61 +++-------------
 2 files changed, 98 insertions(+), 53 deletions(-)
 create mode 100644 test/lisp/eshell/eshell-tests-helpers.el

diff --git a/test/lisp/eshell/eshell-tests-helpers.el b/test/lisp/eshell/eshell-tests-helpers.el
new file mode 100644
index 0000000000..2afa63ae51
--- /dev/null
+++ b/test/lisp/eshell/eshell-tests-helpers.el
@@ -0,0 +1,90 @@
+;;; eshell-tests-helpers.el --- Eshell test suite helpers  -*- lexical-binding:t -*-
+
+;; Copyright (C) 1999-2022 Free Software Foundation, Inc.
+
+;; Author: John Wiegley <johnw@gnu.org>
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; Eshell test suite helpers.
+
+;;; Code:
+
+(require 'ert)
+(require 'ert-x)
+(require 'esh-mode)
+(require 'eshell)
+
+(defvar eshell-test--max-subprocess-time 5
+  "The maximum amount of time to wait for a subprocess to finish, in seconds.
+See `eshell-wait-for-subprocess'.")
+
+(defmacro with-temp-eshell (&rest body)
+  "Evaluate BODY in a temporary Eshell buffer."
+  `(ert-with-temp-directory eshell-directory-name
+     (let* (;; We want no history file, so prevent Eshell from falling
+            ;; back on $HISTFILE.
+            (process-environment (cons "HISTFILE" process-environment))
+            (eshell-history-file-name nil)
+            (eshell-buffer (eshell t)))
+       (unwind-protect
+           (with-current-buffer eshell-buffer
+             ,@body)
+         (let (kill-buffer-query-functions)
+           (kill-buffer eshell-buffer))))))
+
+(defun eshell-wait-for-subprocess ()
+  "Wait until there is no interactive subprocess running in Eshell.
+If this takes longer than `eshell-test--max-subprocess-time',
+raise an error."
+  (let ((start (current-time)))
+    (while (eshell-interactive-process)
+      (when (> (float-time (time-since start))
+               eshell-test--max-subprocess-time)
+        (error "timed out waiting for subprocess"))
+      (sit-for 0.1))))
+
+(defun eshell-insert-command (text &optional func)
+  "Insert a command at the end of the buffer."
+  (goto-char eshell-last-output-end)
+  (insert-and-inherit text)
+  (funcall (or func 'eshell-send-input)))
+
+(defun eshell-match-result (regexp)
+  "Check that text after `eshell-last-input-end' matches REGEXP."
+  (goto-char eshell-last-input-end)
+  (should (string-match-p regexp (buffer-substring-no-properties
+                                  (point) (point-max)))))
+
+(defun eshell-command-result-p (text regexp &optional func)
+  "Insert a command at the end of the buffer."
+  (eshell-insert-command text func)
+  (eshell-wait-for-subprocess)
+  (eshell-match-result regexp))
+
+(defvar eshell-history-file-name)
+
+(defun eshell-test-command-result (command)
+  "Like `eshell-command-result', but not using HOME."
+  (ert-with-temp-directory eshell-directory-name
+    (let ((eshell-history-file-name nil))
+      (eshell-command-result command))))
+
+(provide 'eshell-tests)
+
+;;; eshell-tests.el ends here
diff --git a/test/lisp/eshell/eshell-tests.el b/test/lisp/eshell/eshell-tests.el
index 1a7ab0ab06..6aeefdfde2 100644
--- a/test/lisp/eshell/eshell-tests.el
+++ b/test/lisp/eshell/eshell-tests.el
@@ -29,61 +29,16 @@
 (require 'ert-x)
 (require 'esh-mode)
 (require 'eshell)
-
-(defvar eshell-test--max-subprocess-time 5
-  "The maximum amount of time to wait for a subprocess to finish, in seconds.
-See `eshell-wait-for-subprocess'.")
-
-(defmacro with-temp-eshell (&rest body)
-  "Evaluate BODY in a temporary Eshell buffer."
-  `(ert-with-temp-directory eshell-directory-name
-     (let* (;; We want no history file, so prevent Eshell from falling
-            ;; back on $HISTFILE.
-            (process-environment (cons "HISTFILE" process-environment))
-            (eshell-history-file-name nil)
-            (eshell-buffer (eshell t)))
-       (unwind-protect
-           (with-current-buffer eshell-buffer
-             ,@body)
-         (let (kill-buffer-query-functions)
-           (kill-buffer eshell-buffer))))))
-
-(defun eshell-wait-for-subprocess ()
-  "Wait until there is no interactive subprocess running in Eshell.
-If this takes longer than `eshell-test--max-subprocess-time',
-raise an error."
-  (let ((start (current-time)))
-    (while (eshell-interactive-process)
-      (when (> (float-time (time-since start))
-               eshell-test--max-subprocess-time)
-        (error "timed out waiting for subprocess"))
-      (sit-for 0.1))))
-
-(defun eshell-insert-command (text &optional func)
-  "Insert a command at the end of the buffer."
-  (goto-char eshell-last-output-end)
-  (insert-and-inherit text)
-  (funcall (or func 'eshell-send-input)))
-
-(defun eshell-match-result (regexp)
-  "Check that text after `eshell-last-input-end' matches REGEXP."
-  (goto-char eshell-last-input-end)
-  (should (string-match-p regexp (buffer-substring-no-properties
-                                  (point) (point-max)))))
-
-(defun eshell-command-result-p (text regexp &optional func)
-  "Insert a command at the end of the buffer."
-  (eshell-insert-command text func)
-  (eshell-wait-for-subprocess)
-  (eshell-match-result regexp))
+(eval-and-compile
+  (load (expand-file-name "eshell-tests-helpers"
+                          (file-name-directory (or load-file-name
+                                                   default-directory)))))
 
 (defvar eshell-history-file-name)
-
-(defun eshell-test-command-result (command)
-  "Like `eshell-command-result', but not using HOME."
-  (ert-with-temp-directory eshell-directory-name
-    (let ((eshell-history-file-name nil))
-      (eshell-command-result command))))
+(defvar eshell-test--max-subprocess-time)
+(declare-function eshell-insert-command "eshell-tests-helpers")
+(declare-function eshell-match-result "eshell-tests-helpers")
+(declare-function eshell-command-result-p "eshell-tests-helpers")
 
 ;;; Tests:
 
-- 
2.30.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: v4-0002-Rework-eshell-match-result-for-testing-asynchrono.patch --]
[-- Type: text/x-patch, Size: 2634 bytes --]

From 7d8dc8906973371fe5ce38f676bba39d57af17cc Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Sat, 22 Jan 2022 18:54:55 -0700
Subject: [PATCH v4 2/3] Rework eshell-match-result for testing asynchronous
 commands

When using eshell-match-result via eshell-command-result-p to examine
the output of asynchronous Eshell commands, a newly emitted prompt is
included in the text against which the regexp is matched.  This makes
it awkward to match against the whole output; for example, to check
whether it is empty.  Rework the function to exclude the prompt.

* test/lisp/eshell/eshell-tests-helpers.el (eshell-match-result):
Exclude any newly emitted prompt from the text against which the
regexp is matched.  Additionally, the function no longer moves point.
* test/lisp/eshell/eshell-tests.el (eshell-test/flush-output): Update
and simplify test given how eshell-match-result no longer moves point.
---
 test/lisp/eshell/eshell-tests-helpers.el | 9 +++++----
 test/lisp/eshell/eshell-tests.el         | 5 ++---
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/test/lisp/eshell/eshell-tests-helpers.el b/test/lisp/eshell/eshell-tests-helpers.el
index 2afa63ae51..a150adb144 100644
--- a/test/lisp/eshell/eshell-tests-helpers.el
+++ b/test/lisp/eshell/eshell-tests-helpers.el
@@ -66,10 +66,11 @@ eshell-insert-command
   (funcall (or func 'eshell-send-input)))
 
 (defun eshell-match-result (regexp)
-  "Check that text after `eshell-last-input-end' matches REGEXP."
-  (goto-char eshell-last-input-end)
-  (should (string-match-p regexp (buffer-substring-no-properties
-                                  (point) (point-max)))))
+  "Check that output of last command matches REGEXP."
+  (should
+   (string-match-p
+    regexp (buffer-substring-no-properties
+            (eshell-beginning-of-output) (eshell-end-of-output)))))
 
 (defun eshell-command-result-p (text regexp &optional func)
   "Insert a command at the end of the buffer."
diff --git a/test/lisp/eshell/eshell-tests.el b/test/lisp/eshell/eshell-tests.el
index 6aeefdfde2..542815df80 100644
--- a/test/lisp/eshell/eshell-tests.el
+++ b/test/lisp/eshell/eshell-tests.el
@@ -232,9 +232,8 @@ eshell-test/flush-output
   (with-temp-eshell
    (eshell-insert-command "echo alpha")
    (eshell-kill-output)
-   (eshell-match-result (regexp-quote "*** output flushed ***\n"))
-   (should (forward-line))
-   (should (= (point) eshell-last-output-start))))
+   (eshell-match-result
+    (concat "^" (regexp-quote "*** output flushed ***\n") "$"))))
 
 (ert-deftest eshell-test/run-old-command ()
   "Re-run an old command"
-- 
2.30.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: v4-0003-Add-Eshell-syntax-to-more-easily-bypass-Eshell-s-.patch --]
[-- Type: text/x-patch, Size: 20218 bytes --]

From f12e596aa3112397a26ad9675ca1de091f04c140 Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Mon, 17 Jan 2022 15:15:36 -0700
Subject: [PATCH v4 3/3] Add Eshell syntax to more easily bypass Eshell's own
 pipelining

* etc/NEWS:
* doc/misc/eshell.texi (Input/Output): Document the new syntax.
* lisp/eshell/em-extpipe.el: New module.
* test/lisp/eshell/em-extpipe-tests.el: New tests.
* lisp/eshell/esh-module.el (eshell-modules-list): Add `eshell-extpipe'.
---
 doc/misc/eshell.texi                 |  42 ++++++
 etc/NEWS                             |  10 ++
 lisp/eshell/em-extpipe.el            | 183 ++++++++++++++++++++++++
 lisp/eshell/esh-module.el            |   1 +
 test/lisp/eshell/em-extpipe-tests.el | 205 +++++++++++++++++++++++++++
 5 files changed, 441 insertions(+)
 create mode 100644 lisp/eshell/em-extpipe.el
 create mode 100644 test/lisp/eshell/em-extpipe-tests.el

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index df6e3b861e..261e88d00c 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1142,6 +1142,48 @@ Input/Output
 The output function is called once on each line of output until
 @code{nil} is passed, indicating end of output.
 
+@section Running Shell Pipelines Natively
+When constructing shell pipelines that will move a lot of data, it is
+a good idea to bypass Eshell's own pipelining support and use the
+operating system shell's instead.  This is especially relevant when
+executing commands on a remote machine using Eshell's Tramp
+integration: using the remote shell's pipelining avoids copying the
+data which will flow through the pipeline to local Emacs buffers and
+then right back again.
+
+Eshell recognises a special syntax to make it easier to convert
+pipelines so as to bypass Eshell's pipelining.  Prefixing at least one
+@code{|}, @code{<} or @code{>} with an asterisk marks a command as
+intended for the operating system shell.  To make it harder to invoke
+this functionality accidentally, it is also required that the asterisk
+be preceded by whitespace or located at the start of input.  For
+example,
+
+@example
+ cat *.ogg *| my-cool-decoder >file
+@end example
+
+Executing this command will not copy all the data in the *.ogg files,
+nor the decoded data, into Emacs buffers, as would normally happen.
+
+The command is interpreted as extending up to the next @code{|}
+character which is not preceded by an unescaped asterisk following
+whitespace, or the end of the input if there is no such character.
+Thus, all @code{<} and @code{>} redirections occuring before the next
+asterisk-unprefixed @code{|} are implicitly prefixed with (whitespace
+and) asterisks.  An exception is that Eshell-specific redirects right
+at the end of the command are excluded.  This allows input like this:
+
+@example
+ foo *| baz >#<buffer quux>
+@end example
+
+@noindent which is equivalent to input like this:
+
+@example
+ sh -c "foo | baz" >#<buffer quux>
+@end example
+
 @node Extension modules
 @chapter Extension modules
 Eshell provides a facility for defining extension modules so that they
diff --git a/etc/NEWS b/etc/NEWS
index 5297db3e2d..68c0eba866 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -858,6 +858,16 @@ the Galeon web browser was released in September, 2008.
 
 *** New user option 'ruby-toggle-block-space-before-parameters'.
 
+** Eshell
+
++++
+*** New feature to easily bypass Eshell's own pipelining.
+Prefixing '|', '<' or '>' with an asterisk, i.e. '*|', '*<' or '*>',
+will cause the whole command to be passed to the operating system
+shell.  This is particularly useful to bypass Eshell's own pipelining
+support for pipelines which will move a lot of data.  See "Running
+Shell Pipelines Natively" in the Eshell manual.
+
 ** Miscellaneous
 
 ---
diff --git a/lisp/eshell/em-extpipe.el b/lisp/eshell/em-extpipe.el
new file mode 100644
index 0000000000..57aeec38ff
--- /dev/null
+++ b/lisp/eshell/em-extpipe.el
@@ -0,0 +1,183 @@
+;;; em-extpipe.el --- external shell pipelines  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2022 Free Software Foundation, Inc.
+
+;; Author: Sean Whitton <spwhitton@spwhitton.name>
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; When constructing shell pipelines that will move a lot of data, it
+;; is a good idea to bypass Eshell's own pipelining support and use
+;; the operating system shell's instead.  This module tries to make
+;; that easy to do.
+
+;;; Code:
+
+(require 'cl-lib)
+(require 'esh-arg)
+(require 'esh-io)
+(require 'esh-util)
+
+(eval-when-compile (require 'files-x))
+
+;;; Functions:
+
+(defun eshell-extpipe-initialize () ;Called from `eshell-mode' via intern-soft!
+  "Initialize external pipelines support."
+  (when (boundp 'eshell-special-chars-outside-quoting)
+    (setq-local
+     eshell-special-chars-outside-quoting
+     (append eshell-special-chars-outside-quoting (list ?\*))))
+  (add-hook 'eshell-parse-argument-hook
+            #'eshell-parse-external-pipeline -20 t)
+  (add-hook 'eshell-pre-rewrite-command-hook
+            #'eshell-rewrite-external-pipeline -20 t))
+
+(defun eshell-parse-external-pipeline ()
+  "Parse a pipeline intended for execution by the external shell.
+
+A sequence of arguments is rewritten to use the operating system
+shell when it contains `*|', `*<' or `*>', where the asterisk is
+preceded by whitespace or located at the start of input.
+
+The command extends to the next `|' character which is not
+preceded by an unescaped asterisk following whitespace, or the
+end of input, except that any Eshell-specific output redirections
+occurring at the end are excluded.  Any other `<' or `>'
+appearing before the end of the command are treated as though
+preceded by (whitespace and) an asterisk.
+
+For example,
+
+    foo <bar *| baz >#<buffer quux>
+
+is equivalent to
+
+    sh -c \"foo <bar | baz\" >#<buffer quux>
+
+when `shell-file-name' is `sh' and `shell-command-switch' is
+`-c', but in
+
+    foo >#<buffer quux> *| baz
+
+and
+
+    foo *| baz >#<buffer quux> --some-argument
+
+the Eshell-specific redirect will be passed on to the operating
+system shell, probably leading to undesired results.
+
+This function must appear early in `eshell-parse-argument-hook'
+to ensure that operating system shell syntax is not interpreted
+as though it were Eshell syntax."
+  ;; Our goal is to wrap the external command to protect it from the
+  ;; other members of `eshell-parse-argument-hook'.  We must avoid
+  ;; misinterpreting a quoted `*|', `*<' or `*>' as indicating an
+  ;; external pipeline, hence the structure of the loop in `findbeg1'.
+  (cl-flet
+      ((findbeg1 (pat &optional go (bound (point-max)))
+         (let* ((start (point))
+                (result
+                 (catch 'found
+                   (while (> bound (point))
+                     (let* ((found
+                             (save-excursion
+                               (re-search-forward "['\"\\]" bound t)))
+                            (next (or (and found (match-beginning 0))
+                                      bound)))
+                       (if (re-search-forward pat next t)
+                           (throw 'found (match-beginning 1))
+                         (goto-char next)
+                         (while (or (eshell-parse-backslash)
+                                    (eshell-parse-double-quote)
+                                    (eshell-parse-literal-quote)))))))))
+           (goto-char (if (and result go) (match-end 0) start))
+           result)))
+    (unless (or eshell-current-argument eshell-current-quoted)
+      (let ((beg (point)) end
+            (next-marked (findbeg1 "\\(?:\\=\\|\\s-\\)\\(\\*[|<>]\\)"))
+            (next-unmarked
+             (or (findbeg1 "\\(?:\\=\\|[^*]\\|\\S-\\*\\)\\(|\\)")
+                 (point-max))))
+        (when (and next-marked (> next-unmarked next-marked)
+                   (or (> next-marked (point))
+                       (looking-back "\\`\\|\\s-" nil)))
+          ;; Skip to the final segment of the external pipeline.
+          (while (findbeg1 "\\(?:\\=\\|\\s-\\)\\(\\*|\\)" t))
+          ;; Find output redirections.
+          (while (findbeg1
+                  "\\([0-9]?>+&?[0-9]?\\s-*\\S-\\)" t next-unmarked)
+            ;; Is the output redirection Eshell-specific?  We have our
+            ;; own logic, rather than calling `eshell-parse-argument',
+            ;; to avoid specifying here all the possible cars of
+            ;; parsed special references -- `get-buffer-create' etc.
+            (forward-char -1)
+            (let ((this-end
+                   (save-match-data
+                     (cond ((looking-at "#<")
+                            (forward-char 1)
+                            (1+ (eshell-find-delimiter ?\< ?\>)))
+                           ((and (looking-at "/\\S-+")
+                                 (assoc (match-string 0)
+                                        eshell-virtual-targets))
+                            (match-end 0))))))
+              (cond ((and this-end end)
+                     (goto-char this-end))
+                    (this-end
+                     (goto-char this-end)
+                     (setq end (match-beginning 0)))
+                    (t
+                     (setq end nil)))))
+          ;; We've moved past all Eshell-specific output redirections
+          ;; we could find.  If there is only whitespace left, then
+          ;; `end' is right before redirections we should exclude;
+          ;; otherwise, we must include everything.
+          (unless (and end (skip-syntax-forward "\s" next-unmarked)
+                       (= next-unmarked (point)))
+            (setq end next-unmarked))
+          (let ((cmd (string-trim
+                      (buffer-substring-no-properties beg end))))
+            (goto-char end)
+            ;; We must now drop the asterisks, unless quoted/escaped.
+            (with-temp-buffer
+              (insert cmd)
+              (goto-char (point-min))
+              (cl-loop
+               for next = (findbeg1 "\\(?:\\=\\|\\s-\\)\\(\\*[|<>]\\)" t)
+               while next do (forward-char -2) (delete-char 1))
+              (eshell-finish-arg
+               `(eshell-external-pipeline ,(buffer-string))))))))))
+
+(defun eshell-rewrite-external-pipeline (terms)
+  "Rewrite an external pipeline in TERMS as parsed by
+`eshell-parse-external-pipeline', which see."
+  (while terms
+    (when (and (listp (car terms))
+               (eq (caar terms) 'eshell-external-pipeline))
+      (with-connection-local-variables
+       (setcdr terms (cl-list*
+                      shell-command-switch (cadar terms) (cdr terms)))
+       (setcar terms shell-file-name)))
+    (setq terms (cdr terms))))
+
+(defsubst eshell-external-pipeline (&rest _args)
+  "Stub to generate an error if a pipeline is not rewritten."
+  (error "Unhandled external pipeline in input text"))
+
+(provide 'em-extpipe)
+;;; esh-extpipe.el ends here
diff --git a/lisp/eshell/esh-module.el b/lisp/eshell/esh-module.el
index ade151d7cd..14e91912d1 100644
--- a/lisp/eshell/esh-module.el
+++ b/lisp/eshell/esh-module.el
@@ -54,6 +54,7 @@ eshell-modules-list
     eshell-basic
     eshell-cmpl
     eshell-dirs
+    eshell-extpipe
     eshell-glob
     eshell-hist
     eshell-ls
diff --git a/test/lisp/eshell/em-extpipe-tests.el b/test/lisp/eshell/em-extpipe-tests.el
new file mode 100644
index 0000000000..2c3f11522b
--- /dev/null
+++ b/test/lisp/eshell/em-extpipe-tests.el
@@ -0,0 +1,205 @@
+;;; em-extpipe-tests.el --- em-extpipe test suite  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2022 Free Software Foundation, Inc.
+
+;; Author: Sean Whitton <spwhitton@spwhitton.name>
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+
+;;; Code:
+
+(require 'cl-lib)
+(require 'ert)
+(require 'ert-x)
+(require 'em-extpipe)
+(eval-and-compile
+  (load (expand-file-name "eshell-tests-helpers"
+                          (file-name-directory (or load-file-name
+                                                   default-directory)))))
+
+(defvar eshell-history-file-name)
+(defvar eshell-test--max-subprocess-time)
+(declare-function eshell-command-result-p "eshell-tests-helpers")
+
+(defmacro em-extpipe-tests--deftest (name input &rest body)
+  (declare (indent 2))
+  `(ert-deftest ,name ()
+     (cl-macrolet
+         ((should-parse (expected)
+            `(let ((shell-file-name "sh")
+                   (shell-command-switch "-c"))
+               ;; Strip `eshell-trap-errors'.
+               (should (equal ,expected
+                              (cadr (eshell-parse-command input))))))
+          (with-substitute-for-temp (&rest body)
+            ;; Substitute name of an actual temporary file and/or
+            ;; buffer into `input'.  The substitution logic is
+            ;; appropriate for only the use we put it to in this file.
+            `(ert-with-temp-file temp
+               (let ((temp-buffer (generate-new-buffer " *temp*" t)))
+                 (unwind-protect
+                     (let ((input
+                            (replace-regexp-in-string
+                             "temp\\([^>]\\|\\'\\)" temp
+                             (string-replace "#<buffer temp>"
+                                             (buffer-name temp-buffer)
+                                             input))))
+                       ,@body)
+                   (when (buffer-name temp-buffer)
+                     (kill-buffer temp-buffer))))))
+          (temp-should-string= (expected)
+            `(string= ,expected (string-trim-right
+                                 (with-temp-buffer
+                                   (insert-file-contents temp)
+                                   (buffer-string)))))
+          (temp-buffer-should-string= (expected)
+            `(string= ,expected (string-trim-right
+                                 (with-current-buffer temp-buffer
+                                   (buffer-string))))))
+       (skip-unless shell-file-name)
+       (skip-unless shell-command-switch)
+       (skip-unless (executable-find shell-file-name))
+       (let ((input ,input))
+         (with-temp-eshell ,@body)))))
+
+(em-extpipe-tests--deftest em-extpipe-test-1
+    "echo \"bar\" *| rev >temp"
+  (skip-unless (executable-find "rev"))
+  (should-parse '(eshell-named-command
+                  "sh" (list "-c" "echo \"bar\" | rev >temp")))
+  (with-substitute-for-temp
+   (eshell-command-result-p input "^$")
+   (temp-should-string= "rab")))
+
+(em-extpipe-tests--deftest em-extpipe-test-2
+    "echo \"bar\" | rev *>temp"
+  (skip-unless (executable-find "rev"))
+  (should-parse
+   '(eshell-execute-pipeline
+     '((eshell-named-command "echo" (list (eshell-escape-arg "bar")))
+       (eshell-named-command "sh" (list "-c" "rev >temp")))))
+  (with-substitute-for-temp
+   (eshell-command-result-p input "^$")
+   (temp-should-string= "rab")))
+
+(em-extpipe-tests--deftest em-extpipe-test-3 "foo *| bar | baz -d"
+  (should-parse
+   '(eshell-execute-pipeline
+     '((eshell-named-command "sh" (list "-c" "foo | bar"))
+       (eshell-named-command "baz" (list "-d"))))))
+
+(em-extpipe-tests--deftest em-extpipe-test-4
+    "echo \"bar\" *| rev >#<buffer temp>"
+  (skip-unless (executable-find "rev"))
+  (should-parse
+   '(progn
+      (ignore
+       (eshell-set-output-handle 1 'overwrite
+				 (get-buffer-create "temp")))
+      (eshell-named-command "sh"
+			    (list "-c" "echo \"bar\" | rev"))))
+  (with-substitute-for-temp
+   (eshell-command-result-p input "^$")
+   (temp-buffer-should-string= "rab")))
+
+(em-extpipe-tests--deftest em-extpipe-test-5
+    "foo *| bar >#<buffer quux> baz"
+  (should-parse '(eshell-named-command
+                  "sh" (list "-c" "foo | bar >#<buffer quux> baz"))))
+
+(em-extpipe-tests--deftest em-extpipe-test-6
+    "foo >#<buffer quux> *| bar baz"
+  (should-parse '(eshell-named-command
+                  "sh" (list "-c" "foo >#<buffer quux> | bar baz"))))
+
+(em-extpipe-tests--deftest em-extpipe-test-7
+    "foo *| bar >#<buffer quux> >>#<process other>"
+  (should-parse
+   '(progn
+      (ignore
+       (eshell-set-output-handle 1 'overwrite
+				 (get-buffer-create "quux")))
+      (ignore
+       (eshell-set-output-handle 1 'append
+				 (get-process "other")))
+      (eshell-named-command "sh"
+			    (list "-c" "foo | bar")))))
+
+(em-extpipe-tests--deftest em-extpipe-test-8
+    "foo *| bar >/dev/kill | baz"
+  (should-parse
+   '(eshell-execute-pipeline
+     '((progn
+	 (ignore
+	  (eshell-set-output-handle 1 'overwrite "/dev/kill"))
+	 (eshell-named-command "sh"
+			       (list "-c" "foo | bar")))
+       (eshell-named-command "baz")))))
+
+(em-extpipe-tests--deftest em-extpipe-test-9 "foo \\*| bar"
+  (should-parse
+   '(eshell-execute-pipeline
+     '((eshell-named-command "foo"
+                             (list (eshell-escape-arg "*")))
+       (eshell-named-command "bar")))))
+
+(em-extpipe-tests--deftest em-extpipe-test-10 "foo \"*|\" *>bar"
+  (should-parse
+   '(eshell-named-command "sh" (list "-c" "foo \"*|\" >bar"))))
+
+(em-extpipe-tests--deftest em-extpipe-test-11 "foo '*|' bar"
+  (should-parse '(eshell-named-command
+                  "foo" (list (eshell-escape-arg "*|") "bar"))))
+
+(em-extpipe-tests--deftest em-extpipe-test-12 ">foo bar *| baz"
+  (should-parse
+   '(eshell-named-command "sh" (list "-c" ">foo bar | baz"))))
+
+(em-extpipe-tests--deftest em-extpipe-test-13 "foo*|bar"
+  (should-parse '(eshell-execute-pipeline
+                  '((eshell-named-command (concat "foo" "*"))
+                    (eshell-named-command "bar")))))
+
+(em-extpipe-tests--deftest em-extpipe-test-14 "tac *<temp"
+  (skip-unless (executable-find "tac"))
+  (should-parse '(eshell-named-command "sh" (list "-c" "tac <temp")))
+  (with-substitute-for-temp
+   (with-temp-buffer (insert "bar\nbaz") (write-file temp))
+   (eshell-command-result-p input "baz\nbar")))
+
+(em-extpipe-tests--deftest em-extpipe-test-15 "echo \"bar\" *| cat"
+  (skip-unless (executable-find "cat"))
+  (should-parse
+   '(eshell-named-command "sh" (list "-c" "echo \"bar\" | cat")))
+  (cl-letf (((symbol-function 'eshell/cat)
+             (lambda (&rest _args) (eshell-print "nonsense"))))
+    (eshell-command-result-p input "bar")
+    (eshell-command-result-p "echo \"bar\" | cat" "nonsense")))
+
+(em-extpipe-tests--deftest em-extpipe-test-16 "echo \"bar\" *| rev"
+  (skip-unless (executable-find "rev"))
+  (should-parse
+   '(eshell-named-command "sh" (list "-c" "echo \"bar\" | rev")))
+  (let ((eshell-prefer-lisp-functions t))
+    (cl-letf (((symbol-function 'rev)
+               (lambda (&rest _args) (eshell-print "nonsense"))))
+      (eshell-command-result-p input "rab")
+      (eshell-command-result-p "echo \"bar\" | rev" "nonsense"))))
+
+;;; em-extpipe-tests.el ends here
-- 
2.30.2

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Lars Ingebrigtsen]

Sean Whitton <spwhitton@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

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Michael Albinus]

Sean Whitton <spwhitton@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.

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Sean Whitton]

[-- Attachment #1: Type: text/plain, Size: 869 bytes --]

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v5-0001-Move-Eshell-test-helpers-to-their-own-file.patch --]
[-- Type: text/x-patch, Size: 6953 bytes --]

From 1cfae74bbf78093c84bebaa6dc97bc887238287c Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Fri, 21 Jan 2022 22:32:22 -0700
Subject: [PATCH v5 1/3] Move Eshell test helpers to their own file

* test/lisp/eshell/eshell-tests.el:
* test/lisp/eshell/eshell-tests-helpers.el: Move helpers to own file.
---
 test/lisp/eshell/eshell-tests-helpers.el | 90 ++++++++++++++++++++++++
 test/lisp/eshell/eshell-tests.el         | 61 +++-------------
 2 files changed, 98 insertions(+), 53 deletions(-)
 create mode 100644 test/lisp/eshell/eshell-tests-helpers.el

diff --git a/test/lisp/eshell/eshell-tests-helpers.el b/test/lisp/eshell/eshell-tests-helpers.el
new file mode 100644
index 0000000000..2afa63ae51
--- /dev/null
+++ b/test/lisp/eshell/eshell-tests-helpers.el
@@ -0,0 +1,90 @@
+;;; eshell-tests-helpers.el --- Eshell test suite helpers  -*- lexical-binding:t -*-
+
+;; Copyright (C) 1999-2022 Free Software Foundation, Inc.
+
+;; Author: John Wiegley <johnw@gnu.org>
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; Eshell test suite helpers.
+
+;;; Code:
+
+(require 'ert)
+(require 'ert-x)
+(require 'esh-mode)
+(require 'eshell)
+
+(defvar eshell-test--max-subprocess-time 5
+  "The maximum amount of time to wait for a subprocess to finish, in seconds.
+See `eshell-wait-for-subprocess'.")
+
+(defmacro with-temp-eshell (&rest body)
+  "Evaluate BODY in a temporary Eshell buffer."
+  `(ert-with-temp-directory eshell-directory-name
+     (let* (;; We want no history file, so prevent Eshell from falling
+            ;; back on $HISTFILE.
+            (process-environment (cons "HISTFILE" process-environment))
+            (eshell-history-file-name nil)
+            (eshell-buffer (eshell t)))
+       (unwind-protect
+           (with-current-buffer eshell-buffer
+             ,@body)
+         (let (kill-buffer-query-functions)
+           (kill-buffer eshell-buffer))))))
+
+(defun eshell-wait-for-subprocess ()
+  "Wait until there is no interactive subprocess running in Eshell.
+If this takes longer than `eshell-test--max-subprocess-time',
+raise an error."
+  (let ((start (current-time)))
+    (while (eshell-interactive-process)
+      (when (> (float-time (time-since start))
+               eshell-test--max-subprocess-time)
+        (error "timed out waiting for subprocess"))
+      (sit-for 0.1))))
+
+(defun eshell-insert-command (text &optional func)
+  "Insert a command at the end of the buffer."
+  (goto-char eshell-last-output-end)
+  (insert-and-inherit text)
+  (funcall (or func 'eshell-send-input)))
+
+(defun eshell-match-result (regexp)
+  "Check that text after `eshell-last-input-end' matches REGEXP."
+  (goto-char eshell-last-input-end)
+  (should (string-match-p regexp (buffer-substring-no-properties
+                                  (point) (point-max)))))
+
+(defun eshell-command-result-p (text regexp &optional func)
+  "Insert a command at the end of the buffer."
+  (eshell-insert-command text func)
+  (eshell-wait-for-subprocess)
+  (eshell-match-result regexp))
+
+(defvar eshell-history-file-name)
+
+(defun eshell-test-command-result (command)
+  "Like `eshell-command-result', but not using HOME."
+  (ert-with-temp-directory eshell-directory-name
+    (let ((eshell-history-file-name nil))
+      (eshell-command-result command))))
+
+(provide 'eshell-tests)
+
+;;; eshell-tests.el ends here
diff --git a/test/lisp/eshell/eshell-tests.el b/test/lisp/eshell/eshell-tests.el
index 1a7ab0ab06..6aeefdfde2 100644
--- a/test/lisp/eshell/eshell-tests.el
+++ b/test/lisp/eshell/eshell-tests.el
@@ -29,61 +29,16 @@
 (require 'ert-x)
 (require 'esh-mode)
 (require 'eshell)
-
-(defvar eshell-test--max-subprocess-time 5
-  "The maximum amount of time to wait for a subprocess to finish, in seconds.
-See `eshell-wait-for-subprocess'.")
-
-(defmacro with-temp-eshell (&rest body)
-  "Evaluate BODY in a temporary Eshell buffer."
-  `(ert-with-temp-directory eshell-directory-name
-     (let* (;; We want no history file, so prevent Eshell from falling
-            ;; back on $HISTFILE.
-            (process-environment (cons "HISTFILE" process-environment))
-            (eshell-history-file-name nil)
-            (eshell-buffer (eshell t)))
-       (unwind-protect
-           (with-current-buffer eshell-buffer
-             ,@body)
-         (let (kill-buffer-query-functions)
-           (kill-buffer eshell-buffer))))))
-
-(defun eshell-wait-for-subprocess ()
-  "Wait until there is no interactive subprocess running in Eshell.
-If this takes longer than `eshell-test--max-subprocess-time',
-raise an error."
-  (let ((start (current-time)))
-    (while (eshell-interactive-process)
-      (when (> (float-time (time-since start))
-               eshell-test--max-subprocess-time)
-        (error "timed out waiting for subprocess"))
-      (sit-for 0.1))))
-
-(defun eshell-insert-command (text &optional func)
-  "Insert a command at the end of the buffer."
-  (goto-char eshell-last-output-end)
-  (insert-and-inherit text)
-  (funcall (or func 'eshell-send-input)))
-
-(defun eshell-match-result (regexp)
-  "Check that text after `eshell-last-input-end' matches REGEXP."
-  (goto-char eshell-last-input-end)
-  (should (string-match-p regexp (buffer-substring-no-properties
-                                  (point) (point-max)))))
-
-(defun eshell-command-result-p (text regexp &optional func)
-  "Insert a command at the end of the buffer."
-  (eshell-insert-command text func)
-  (eshell-wait-for-subprocess)
-  (eshell-match-result regexp))
+(eval-and-compile
+  (load (expand-file-name "eshell-tests-helpers"
+                          (file-name-directory (or load-file-name
+                                                   default-directory)))))
 
 (defvar eshell-history-file-name)
-
-(defun eshell-test-command-result (command)
-  "Like `eshell-command-result', but not using HOME."
-  (ert-with-temp-directory eshell-directory-name
-    (let ((eshell-history-file-name nil))
-      (eshell-command-result command))))
+(defvar eshell-test--max-subprocess-time)
+(declare-function eshell-insert-command "eshell-tests-helpers")
+(declare-function eshell-match-result "eshell-tests-helpers")
+(declare-function eshell-command-result-p "eshell-tests-helpers")
 
 ;;; Tests:
 
-- 
2.30.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: v5-0002-Rework-eshell-match-result-for-testing-asynchrono.patch --]
[-- Type: text/x-patch, Size: 2634 bytes --]

From 7d8dc8906973371fe5ce38f676bba39d57af17cc Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Sat, 22 Jan 2022 18:54:55 -0700
Subject: [PATCH v5 2/3] Rework eshell-match-result for testing asynchronous
 commands

When using eshell-match-result via eshell-command-result-p to examine
the output of asynchronous Eshell commands, a newly emitted prompt is
included in the text against which the regexp is matched.  This makes
it awkward to match against the whole output; for example, to check
whether it is empty.  Rework the function to exclude the prompt.

* test/lisp/eshell/eshell-tests-helpers.el (eshell-match-result):
Exclude any newly emitted prompt from the text against which the
regexp is matched.  Additionally, the function no longer moves point.
* test/lisp/eshell/eshell-tests.el (eshell-test/flush-output): Update
and simplify test given how eshell-match-result no longer moves point.
---
 test/lisp/eshell/eshell-tests-helpers.el | 9 +++++----
 test/lisp/eshell/eshell-tests.el         | 5 ++---
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/test/lisp/eshell/eshell-tests-helpers.el b/test/lisp/eshell/eshell-tests-helpers.el
index 2afa63ae51..a150adb144 100644
--- a/test/lisp/eshell/eshell-tests-helpers.el
+++ b/test/lisp/eshell/eshell-tests-helpers.el
@@ -66,10 +66,11 @@ eshell-insert-command
   (funcall (or func 'eshell-send-input)))
 
 (defun eshell-match-result (regexp)
-  "Check that text after `eshell-last-input-end' matches REGEXP."
-  (goto-char eshell-last-input-end)
-  (should (string-match-p regexp (buffer-substring-no-properties
-                                  (point) (point-max)))))
+  "Check that output of last command matches REGEXP."
+  (should
+   (string-match-p
+    regexp (buffer-substring-no-properties
+            (eshell-beginning-of-output) (eshell-end-of-output)))))
 
 (defun eshell-command-result-p (text regexp &optional func)
   "Insert a command at the end of the buffer."
diff --git a/test/lisp/eshell/eshell-tests.el b/test/lisp/eshell/eshell-tests.el
index 6aeefdfde2..542815df80 100644
--- a/test/lisp/eshell/eshell-tests.el
+++ b/test/lisp/eshell/eshell-tests.el
@@ -232,9 +232,8 @@ eshell-test/flush-output
   (with-temp-eshell
    (eshell-insert-command "echo alpha")
    (eshell-kill-output)
-   (eshell-match-result (regexp-quote "*** output flushed ***\n"))
-   (should (forward-line))
-   (should (= (point) eshell-last-output-start))))
+   (eshell-match-result
+    (concat "^" (regexp-quote "*** output flushed ***\n") "$"))))
 
 (ert-deftest eshell-test/run-old-command ()
   "Re-run an old command"
-- 
2.30.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: v5-0003-Add-Eshell-syntax-to-more-easily-bypass-Eshell-s-.patch --]
[-- Type: text/x-patch, Size: 20799 bytes --]

From abd8fd9b6ea7936c56cde538202c7f3eeb24aa76 Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Mon, 17 Jan 2022 15:15:36 -0700
Subject: [PATCH v5 3/3] Add Eshell syntax to more easily bypass Eshell's own
 pipelining

* etc/NEWS:
* doc/misc/eshell.texi (Input/Output): Document the new syntax.
* lisp/eshell/em-extpipe.el: New module (Bug#46351).
* test/lisp/eshell/em-extpipe-tests.el: New tests.
* lisp/eshell/esh-module.el (eshell-modules-list): Add `eshell-extpipe'.
---
 doc/misc/eshell.texi                     |  42 +++++
 etc/NEWS                                 |  10 ++
 lisp/eshell/em-extpipe.el                | 183 ++++++++++++++++++++
 lisp/eshell/esh-module.el                |   1 +
 test/lisp/eshell/em-extpipe-tests.el     | 205 +++++++++++++++++++++++
 test/lisp/eshell/eshell-tests-helpers.el |   4 +-
 6 files changed, 443 insertions(+), 2 deletions(-)
 create mode 100644 lisp/eshell/em-extpipe.el
 create mode 100644 test/lisp/eshell/em-extpipe-tests.el

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index df6e3b861e..261e88d00c 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1142,6 +1142,48 @@ Input/Output
 The output function is called once on each line of output until
 @code{nil} is passed, indicating end of output.
 
+@section Running Shell Pipelines Natively
+When constructing shell pipelines that will move a lot of data, it is
+a good idea to bypass Eshell's own pipelining support and use the
+operating system shell's instead.  This is especially relevant when
+executing commands on a remote machine using Eshell's Tramp
+integration: using the remote shell's pipelining avoids copying the
+data which will flow through the pipeline to local Emacs buffers and
+then right back again.
+
+Eshell recognises a special syntax to make it easier to convert
+pipelines so as to bypass Eshell's pipelining.  Prefixing at least one
+@code{|}, @code{<} or @code{>} with an asterisk marks a command as
+intended for the operating system shell.  To make it harder to invoke
+this functionality accidentally, it is also required that the asterisk
+be preceded by whitespace or located at the start of input.  For
+example,
+
+@example
+ cat *.ogg *| my-cool-decoder >file
+@end example
+
+Executing this command will not copy all the data in the *.ogg files,
+nor the decoded data, into Emacs buffers, as would normally happen.
+
+The command is interpreted as extending up to the next @code{|}
+character which is not preceded by an unescaped asterisk following
+whitespace, or the end of the input if there is no such character.
+Thus, all @code{<} and @code{>} redirections occuring before the next
+asterisk-unprefixed @code{|} are implicitly prefixed with (whitespace
+and) asterisks.  An exception is that Eshell-specific redirects right
+at the end of the command are excluded.  This allows input like this:
+
+@example
+ foo *| baz >#<buffer quux>
+@end example
+
+@noindent which is equivalent to input like this:
+
+@example
+ sh -c "foo | baz" >#<buffer quux>
+@end example
+
 @node Extension modules
 @chapter Extension modules
 Eshell provides a facility for defining extension modules so that they
diff --git a/etc/NEWS b/etc/NEWS
index 5297db3e2d..68c0eba866 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -858,6 +858,16 @@ the Galeon web browser was released in September, 2008.
 
 *** New user option 'ruby-toggle-block-space-before-parameters'.
 
+** Eshell
+
++++
+*** New feature to easily bypass Eshell's own pipelining.
+Prefixing '|', '<' or '>' with an asterisk, i.e. '*|', '*<' or '*>',
+will cause the whole command to be passed to the operating system
+shell.  This is particularly useful to bypass Eshell's own pipelining
+support for pipelines which will move a lot of data.  See "Running
+Shell Pipelines Natively" in the Eshell manual.
+
 ** Miscellaneous
 
 ---
diff --git a/lisp/eshell/em-extpipe.el b/lisp/eshell/em-extpipe.el
new file mode 100644
index 0000000000..57aeec38ff
--- /dev/null
+++ b/lisp/eshell/em-extpipe.el
@@ -0,0 +1,183 @@
+;;; em-extpipe.el --- external shell pipelines  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2022 Free Software Foundation, Inc.
+
+;; Author: Sean Whitton <spwhitton@spwhitton.name>
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; When constructing shell pipelines that will move a lot of data, it
+;; is a good idea to bypass Eshell's own pipelining support and use
+;; the operating system shell's instead.  This module tries to make
+;; that easy to do.
+
+;;; Code:
+
+(require 'cl-lib)
+(require 'esh-arg)
+(require 'esh-io)
+(require 'esh-util)
+
+(eval-when-compile (require 'files-x))
+
+;;; Functions:
+
+(defun eshell-extpipe-initialize () ;Called from `eshell-mode' via intern-soft!
+  "Initialize external pipelines support."
+  (when (boundp 'eshell-special-chars-outside-quoting)
+    (setq-local
+     eshell-special-chars-outside-quoting
+     (append eshell-special-chars-outside-quoting (list ?\*))))
+  (add-hook 'eshell-parse-argument-hook
+            #'eshell-parse-external-pipeline -20 t)
+  (add-hook 'eshell-pre-rewrite-command-hook
+            #'eshell-rewrite-external-pipeline -20 t))
+
+(defun eshell-parse-external-pipeline ()
+  "Parse a pipeline intended for execution by the external shell.
+
+A sequence of arguments is rewritten to use the operating system
+shell when it contains `*|', `*<' or `*>', where the asterisk is
+preceded by whitespace or located at the start of input.
+
+The command extends to the next `|' character which is not
+preceded by an unescaped asterisk following whitespace, or the
+end of input, except that any Eshell-specific output redirections
+occurring at the end are excluded.  Any other `<' or `>'
+appearing before the end of the command are treated as though
+preceded by (whitespace and) an asterisk.
+
+For example,
+
+    foo <bar *| baz >#<buffer quux>
+
+is equivalent to
+
+    sh -c \"foo <bar | baz\" >#<buffer quux>
+
+when `shell-file-name' is `sh' and `shell-command-switch' is
+`-c', but in
+
+    foo >#<buffer quux> *| baz
+
+and
+
+    foo *| baz >#<buffer quux> --some-argument
+
+the Eshell-specific redirect will be passed on to the operating
+system shell, probably leading to undesired results.
+
+This function must appear early in `eshell-parse-argument-hook'
+to ensure that operating system shell syntax is not interpreted
+as though it were Eshell syntax."
+  ;; Our goal is to wrap the external command to protect it from the
+  ;; other members of `eshell-parse-argument-hook'.  We must avoid
+  ;; misinterpreting a quoted `*|', `*<' or `*>' as indicating an
+  ;; external pipeline, hence the structure of the loop in `findbeg1'.
+  (cl-flet
+      ((findbeg1 (pat &optional go (bound (point-max)))
+         (let* ((start (point))
+                (result
+                 (catch 'found
+                   (while (> bound (point))
+                     (let* ((found
+                             (save-excursion
+                               (re-search-forward "['\"\\]" bound t)))
+                            (next (or (and found (match-beginning 0))
+                                      bound)))
+                       (if (re-search-forward pat next t)
+                           (throw 'found (match-beginning 1))
+                         (goto-char next)
+                         (while (or (eshell-parse-backslash)
+                                    (eshell-parse-double-quote)
+                                    (eshell-parse-literal-quote)))))))))
+           (goto-char (if (and result go) (match-end 0) start))
+           result)))
+    (unless (or eshell-current-argument eshell-current-quoted)
+      (let ((beg (point)) end
+            (next-marked (findbeg1 "\\(?:\\=\\|\\s-\\)\\(\\*[|<>]\\)"))
+            (next-unmarked
+             (or (findbeg1 "\\(?:\\=\\|[^*]\\|\\S-\\*\\)\\(|\\)")
+                 (point-max))))
+        (when (and next-marked (> next-unmarked next-marked)
+                   (or (> next-marked (point))
+                       (looking-back "\\`\\|\\s-" nil)))
+          ;; Skip to the final segment of the external pipeline.
+          (while (findbeg1 "\\(?:\\=\\|\\s-\\)\\(\\*|\\)" t))
+          ;; Find output redirections.
+          (while (findbeg1
+                  "\\([0-9]?>+&?[0-9]?\\s-*\\S-\\)" t next-unmarked)
+            ;; Is the output redirection Eshell-specific?  We have our
+            ;; own logic, rather than calling `eshell-parse-argument',
+            ;; to avoid specifying here all the possible cars of
+            ;; parsed special references -- `get-buffer-create' etc.
+            (forward-char -1)
+            (let ((this-end
+                   (save-match-data
+                     (cond ((looking-at "#<")
+                            (forward-char 1)
+                            (1+ (eshell-find-delimiter ?\< ?\>)))
+                           ((and (looking-at "/\\S-+")
+                                 (assoc (match-string 0)
+                                        eshell-virtual-targets))
+                            (match-end 0))))))
+              (cond ((and this-end end)
+                     (goto-char this-end))
+                    (this-end
+                     (goto-char this-end)
+                     (setq end (match-beginning 0)))
+                    (t
+                     (setq end nil)))))
+          ;; We've moved past all Eshell-specific output redirections
+          ;; we could find.  If there is only whitespace left, then
+          ;; `end' is right before redirections we should exclude;
+          ;; otherwise, we must include everything.
+          (unless (and end (skip-syntax-forward "\s" next-unmarked)
+                       (= next-unmarked (point)))
+            (setq end next-unmarked))
+          (let ((cmd (string-trim
+                      (buffer-substring-no-properties beg end))))
+            (goto-char end)
+            ;; We must now drop the asterisks, unless quoted/escaped.
+            (with-temp-buffer
+              (insert cmd)
+              (goto-char (point-min))
+              (cl-loop
+               for next = (findbeg1 "\\(?:\\=\\|\\s-\\)\\(\\*[|<>]\\)" t)
+               while next do (forward-char -2) (delete-char 1))
+              (eshell-finish-arg
+               `(eshell-external-pipeline ,(buffer-string))))))))))
+
+(defun eshell-rewrite-external-pipeline (terms)
+  "Rewrite an external pipeline in TERMS as parsed by
+`eshell-parse-external-pipeline', which see."
+  (while terms
+    (when (and (listp (car terms))
+               (eq (caar terms) 'eshell-external-pipeline))
+      (with-connection-local-variables
+       (setcdr terms (cl-list*
+                      shell-command-switch (cadar terms) (cdr terms)))
+       (setcar terms shell-file-name)))
+    (setq terms (cdr terms))))
+
+(defsubst eshell-external-pipeline (&rest _args)
+  "Stub to generate an error if a pipeline is not rewritten."
+  (error "Unhandled external pipeline in input text"))
+
+(provide 'em-extpipe)
+;;; esh-extpipe.el ends here
diff --git a/lisp/eshell/esh-module.el b/lisp/eshell/esh-module.el
index ade151d7cd..14e91912d1 100644
--- a/lisp/eshell/esh-module.el
+++ b/lisp/eshell/esh-module.el
@@ -54,6 +54,7 @@ eshell-modules-list
     eshell-basic
     eshell-cmpl
     eshell-dirs
+    eshell-extpipe
     eshell-glob
     eshell-hist
     eshell-ls
diff --git a/test/lisp/eshell/em-extpipe-tests.el b/test/lisp/eshell/em-extpipe-tests.el
new file mode 100644
index 0000000000..1283b6b361
--- /dev/null
+++ b/test/lisp/eshell/em-extpipe-tests.el
@@ -0,0 +1,205 @@
+;;; em-extpipe-tests.el --- em-extpipe test suite  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2022 Free Software Foundation, Inc.
+
+;; Author: Sean Whitton <spwhitton@spwhitton.name>
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+
+;;; Code:
+
+(require 'cl-lib)
+(require 'ert)
+(require 'ert-x)
+(require 'em-extpipe)
+(eval-and-compile
+  (load (expand-file-name "eshell-tests-helpers"
+                          (file-name-directory (or load-file-name
+                                                   default-directory)))))
+
+(defvar eshell-history-file-name)
+(defvar eshell-test--max-subprocess-time)
+(declare-function eshell-command-result-p "eshell-tests-helpers")
+
+(defmacro em-extpipe-tests--deftest (name input &rest body)
+  (declare (indent 2))
+  `(ert-deftest ,name ()
+     (cl-macrolet
+         ((should-parse (expected)
+            `(let ((shell-file-name "sh")
+                   (shell-command-switch "-c"))
+               ;; Strip `eshell-trap-errors'.
+               (should (equal ,expected
+                              (cadr (eshell-parse-command input))))))
+          (with-substitute-for-temp (&rest body)
+            ;; Substitute name of an actual temporary file and/or
+            ;; buffer into `input'.  The substitution logic is
+            ;; appropriate for only the use we put it to in this file.
+            `(ert-with-temp-file temp
+               (let ((temp-buffer (generate-new-buffer " *temp*" t)))
+                 (unwind-protect
+                     (let ((input
+                            (replace-regexp-in-string
+                             "temp\\([^>]\\|\\'\\)" temp
+                             (string-replace "#<buffer temp>"
+                                             (buffer-name temp-buffer)
+                                             input))))
+                       ,@body)
+                   (when (buffer-name temp-buffer)
+                     (kill-buffer temp-buffer))))))
+          (temp-should-string= (expected)
+            `(string= ,expected (string-trim-right
+                                 (with-temp-buffer
+                                   (insert-file-contents temp)
+                                   (buffer-string)))))
+          (temp-buffer-should-string= (expected)
+            `(string= ,expected (string-trim-right
+                                 (with-current-buffer temp-buffer
+                                   (buffer-string))))))
+       (skip-unless shell-file-name)
+       (skip-unless shell-command-switch)
+       (skip-unless (executable-find shell-file-name))
+       (let ((input ,input))
+         (with-temp-eshell ,@body)))))
+
+(em-extpipe-tests--deftest em-extpipe-test-1
+    "echo \"bar\" *| rev >temp"
+  (skip-unless (executable-find "rev"))
+  (should-parse '(eshell-named-command
+                  "sh" (list "-c" "echo \"bar\" | rev >temp")))
+  (with-substitute-for-temp
+   (eshell-command-result-p input "^$")
+   (temp-should-string= "rab")))
+
+(em-extpipe-tests--deftest em-extpipe-test-2
+    "echo \"bar\" | rev *>temp"
+  (skip-unless (executable-find "rev"))
+  (should-parse
+   '(eshell-execute-pipeline
+     '((eshell-named-command "echo" (list (eshell-escape-arg "bar")))
+       (eshell-named-command "sh" (list "-c" "rev >temp")))))
+  (with-substitute-for-temp
+   (eshell-command-result-p input "^$")
+   (temp-should-string= "rab")))
+
+(em-extpipe-tests--deftest em-extpipe-test-3 "foo *| bar | baz -d"
+  (should-parse
+   '(eshell-execute-pipeline
+     '((eshell-named-command "sh" (list "-c" "foo | bar"))
+       (eshell-named-command "baz" (list "-d"))))))
+
+(em-extpipe-tests--deftest em-extpipe-test-4
+    "echo \"bar\" *| rev >#<buffer temp>"
+  (skip-unless (executable-find "rev"))
+  (should-parse
+   '(progn
+      (ignore
+       (eshell-set-output-handle 1 'overwrite
+				 (get-buffer-create "temp")))
+      (eshell-named-command "sh"
+			    (list "-c" "echo \"bar\" | rev"))))
+  (with-substitute-for-temp
+   (eshell-command-result-p input "^$")
+   (temp-buffer-should-string= "rab")))
+
+(em-extpipe-tests--deftest em-extpipe-test-5
+    "foo *| bar >#<buffer quux> baz"
+  (should-parse '(eshell-named-command
+                  "sh" (list "-c" "foo | bar >#<buffer quux> baz"))))
+
+(em-extpipe-tests--deftest em-extpipe-test-6
+    "foo >#<buffer quux> *| bar baz"
+  (should-parse '(eshell-named-command
+                  "sh" (list "-c" "foo >#<buffer quux> | bar baz"))))
+
+(em-extpipe-tests--deftest em-extpipe-test-7
+    "foo *| bar >#<buffer quux> >>#<process other>"
+  (should-parse
+   '(progn
+      (ignore
+       (eshell-set-output-handle 1 'overwrite
+				 (get-buffer-create "quux")))
+      (ignore
+       (eshell-set-output-handle 1 'append
+				 (get-process "other")))
+      (eshell-named-command "sh"
+			    (list "-c" "foo | bar")))))
+
+(em-extpipe-tests--deftest em-extpipe-test-8
+    "foo *| bar >/dev/kill | baz"
+  (should-parse
+   '(eshell-execute-pipeline
+     '((progn
+	 (ignore
+	  (eshell-set-output-handle 1 'overwrite "/dev/kill"))
+	 (eshell-named-command "sh"
+			       (list "-c" "foo | bar")))
+       (eshell-named-command "baz")))))
+
+(em-extpipe-tests--deftest em-extpipe-test-9 "foo \\*| bar"
+  (should-parse
+   '(eshell-execute-pipeline
+     '((eshell-named-command "foo"
+                             (list (eshell-escape-arg "*")))
+       (eshell-named-command "bar")))))
+
+(em-extpipe-tests--deftest em-extpipe-test-10 "foo \"*|\" *>bar"
+  (should-parse
+   '(eshell-named-command "sh" (list "-c" "foo \"*|\" >bar"))))
+
+(em-extpipe-tests--deftest em-extpipe-test-11 "foo '*|' bar"
+  (should-parse '(eshell-named-command
+                  "foo" (list (eshell-escape-arg "*|") "bar"))))
+
+(em-extpipe-tests--deftest em-extpipe-test-12 ">foo bar *| baz"
+  (should-parse
+   '(eshell-named-command "sh" (list "-c" ">foo bar | baz"))))
+
+(em-extpipe-tests--deftest em-extpipe-test-13 "foo*|bar"
+  (should-parse '(eshell-execute-pipeline
+                  '((eshell-named-command (concat "foo" "*"))
+                    (eshell-named-command "bar")))))
+
+(em-extpipe-tests--deftest em-extpipe-test-14 "tac *<temp"
+  (skip-unless (executable-find "tac"))
+  (should-parse '(eshell-named-command "sh" (list "-c" "tac <temp")))
+  (with-substitute-for-temp
+   (with-temp-buffer (insert "bar\nbaz\n") (write-file temp))
+   (eshell-command-result-p input "baz\nbar")))
+
+(em-extpipe-tests--deftest em-extpipe-test-15 "echo \"bar\" *| cat"
+  (skip-unless (executable-find "cat"))
+  (should-parse
+   '(eshell-named-command "sh" (list "-c" "echo \"bar\" | cat")))
+  (cl-letf (((symbol-function 'eshell/cat)
+             (lambda (&rest _args) (eshell-print "nonsense"))))
+    (eshell-command-result-p input "bar")
+    (eshell-command-result-p "echo \"bar\" | cat" "nonsense")))
+
+(em-extpipe-tests--deftest em-extpipe-test-16 "echo \"bar\" *| rev"
+  (skip-unless (executable-find "rev"))
+  (should-parse
+   '(eshell-named-command "sh" (list "-c" "echo \"bar\" | rev")))
+  (let ((eshell-prefer-lisp-functions t))
+    (cl-letf (((symbol-function 'rev)
+               (lambda (&rest _args) (eshell-print "nonsense"))))
+      (eshell-command-result-p input "rab")
+      (eshell-command-result-p "echo \"bar\" | rev" "nonsense"))))
+
+;;; em-extpipe-tests.el ends here
diff --git a/test/lisp/eshell/eshell-tests-helpers.el b/test/lisp/eshell/eshell-tests-helpers.el
index a150adb144..77f5313d57 100644
--- a/test/lisp/eshell/eshell-tests-helpers.el
+++ b/test/lisp/eshell/eshell-tests-helpers.el
@@ -86,6 +86,6 @@ eshell-test-command-result
     (let ((eshell-history-file-name nil))
       (eshell-command-result command))))
 
-(provide 'eshell-tests)
+(provide 'eshell-tests-helpers)
 
-;;; eshell-tests.el ends here
+;;; eshell-tests-helpers.el ends here
-- 
2.30.2

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Sean Whitton]

[-- Attachment #1: Type: text/plain, Size: 451 bytes --]

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v6-0001-Move-Eshell-test-helpers-to-their-own-file.patch --]
[-- Type: text/x-patch, Size: 6969 bytes --]

From 8b91bb7988f3ae905474bd80960a4e42f85108ca Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Fri, 21 Jan 2022 22:32:22 -0700
Subject: [PATCH v6 1/3] Move Eshell test helpers to their own file

* test/lisp/eshell/eshell-tests.el:
* test/lisp/eshell/eshell-tests-helpers.el: Move helpers to own file.
---
 test/lisp/eshell/eshell-tests-helpers.el | 90 ++++++++++++++++++++++++
 test/lisp/eshell/eshell-tests.el         | 61 +++-------------
 2 files changed, 98 insertions(+), 53 deletions(-)
 create mode 100644 test/lisp/eshell/eshell-tests-helpers.el

diff --git a/test/lisp/eshell/eshell-tests-helpers.el b/test/lisp/eshell/eshell-tests-helpers.el
new file mode 100644
index 0000000000..0930f82284
--- /dev/null
+++ b/test/lisp/eshell/eshell-tests-helpers.el
@@ -0,0 +1,90 @@
+;;; eshell-tests-helpers.el --- Eshell test suite helpers  -*- lexical-binding:t -*-
+
+;; Copyright (C) 1999-2022 Free Software Foundation, Inc.
+
+;; Author: John Wiegley <johnw@gnu.org>
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; Eshell test suite helpers.
+
+;;; Code:
+
+(require 'ert)
+(require 'ert-x)
+(require 'esh-mode)
+(require 'eshell)
+
+(defvar eshell-test--max-subprocess-time 5
+  "The maximum amount of time to wait for a subprocess to finish, in seconds.
+See `eshell-wait-for-subprocess'.")
+
+(defmacro with-temp-eshell (&rest body)
+  "Evaluate BODY in a temporary Eshell buffer."
+  `(ert-with-temp-directory eshell-directory-name
+     (let* (;; We want no history file, so prevent Eshell from falling
+            ;; back on $HISTFILE.
+            (process-environment (cons "HISTFILE" process-environment))
+            (eshell-history-file-name nil)
+            (eshell-buffer (eshell t)))
+       (unwind-protect
+           (with-current-buffer eshell-buffer
+             ,@body)
+         (let (kill-buffer-query-functions)
+           (kill-buffer eshell-buffer))))))
+
+(defun eshell-wait-for-subprocess ()
+  "Wait until there is no interactive subprocess running in Eshell.
+If this takes longer than `eshell-test--max-subprocess-time',
+raise an error."
+  (let ((start (current-time)))
+    (while (eshell-interactive-process)
+      (when (> (float-time (time-since start))
+               eshell-test--max-subprocess-time)
+        (error "timed out waiting for subprocess"))
+      (sit-for 0.1))))
+
+(defun eshell-insert-command (text &optional func)
+  "Insert a command at the end of the buffer."
+  (goto-char eshell-last-output-end)
+  (insert-and-inherit text)
+  (funcall (or func 'eshell-send-input)))
+
+(defun eshell-match-result (regexp)
+  "Check that text after `eshell-last-input-end' matches REGEXP."
+  (goto-char eshell-last-input-end)
+  (should (string-match-p regexp (buffer-substring-no-properties
+                                  (point) (point-max)))))
+
+(defun eshell-command-result-p (text regexp &optional func)
+  "Insert a command at the end of the buffer."
+  (eshell-insert-command text func)
+  (eshell-wait-for-subprocess)
+  (eshell-match-result regexp))
+
+(defvar eshell-history-file-name)
+
+(defun eshell-test-command-result (command)
+  "Like `eshell-command-result', but not using HOME."
+  (ert-with-temp-directory eshell-directory-name
+    (let ((eshell-history-file-name nil))
+      (eshell-command-result command))))
+
+(provide 'eshell-tests-helpers)
+
+;;; eshell-tests-helpers.el ends here
diff --git a/test/lisp/eshell/eshell-tests.el b/test/lisp/eshell/eshell-tests.el
index 1a7ab0ab06..6aeefdfde2 100644
--- a/test/lisp/eshell/eshell-tests.el
+++ b/test/lisp/eshell/eshell-tests.el
@@ -29,61 +29,16 @@
 (require 'ert-x)
 (require 'esh-mode)
 (require 'eshell)
-
-(defvar eshell-test--max-subprocess-time 5
-  "The maximum amount of time to wait for a subprocess to finish, in seconds.
-See `eshell-wait-for-subprocess'.")
-
-(defmacro with-temp-eshell (&rest body)
-  "Evaluate BODY in a temporary Eshell buffer."
-  `(ert-with-temp-directory eshell-directory-name
-     (let* (;; We want no history file, so prevent Eshell from falling
-            ;; back on $HISTFILE.
-            (process-environment (cons "HISTFILE" process-environment))
-            (eshell-history-file-name nil)
-            (eshell-buffer (eshell t)))
-       (unwind-protect
-           (with-current-buffer eshell-buffer
-             ,@body)
-         (let (kill-buffer-query-functions)
-           (kill-buffer eshell-buffer))))))
-
-(defun eshell-wait-for-subprocess ()
-  "Wait until there is no interactive subprocess running in Eshell.
-If this takes longer than `eshell-test--max-subprocess-time',
-raise an error."
-  (let ((start (current-time)))
-    (while (eshell-interactive-process)
-      (when (> (float-time (time-since start))
-               eshell-test--max-subprocess-time)
-        (error "timed out waiting for subprocess"))
-      (sit-for 0.1))))
-
-(defun eshell-insert-command (text &optional func)
-  "Insert a command at the end of the buffer."
-  (goto-char eshell-last-output-end)
-  (insert-and-inherit text)
-  (funcall (or func 'eshell-send-input)))
-
-(defun eshell-match-result (regexp)
-  "Check that text after `eshell-last-input-end' matches REGEXP."
-  (goto-char eshell-last-input-end)
-  (should (string-match-p regexp (buffer-substring-no-properties
-                                  (point) (point-max)))))
-
-(defun eshell-command-result-p (text regexp &optional func)
-  "Insert a command at the end of the buffer."
-  (eshell-insert-command text func)
-  (eshell-wait-for-subprocess)
-  (eshell-match-result regexp))
+(eval-and-compile
+  (load (expand-file-name "eshell-tests-helpers"
+                          (file-name-directory (or load-file-name
+                                                   default-directory)))))
 
 (defvar eshell-history-file-name)
-
-(defun eshell-test-command-result (command)
-  "Like `eshell-command-result', but not using HOME."
-  (ert-with-temp-directory eshell-directory-name
-    (let ((eshell-history-file-name nil))
-      (eshell-command-result command))))
+(defvar eshell-test--max-subprocess-time)
+(declare-function eshell-insert-command "eshell-tests-helpers")
+(declare-function eshell-match-result "eshell-tests-helpers")
+(declare-function eshell-command-result-p "eshell-tests-helpers")
 
 ;;; Tests:
 
-- 
2.30.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: v6-0002-Rework-eshell-match-result-for-testing-asynchrono.patch --]
[-- Type: text/x-patch, Size: 2634 bytes --]

From 40566a5d0c0ef4d30961915bac7875d078adb58e Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Sat, 22 Jan 2022 18:54:55 -0700
Subject: [PATCH v6 2/3] Rework eshell-match-result for testing asynchronous
 commands

When using eshell-match-result via eshell-command-result-p to examine
the output of asynchronous Eshell commands, a newly emitted prompt is
included in the text against which the regexp is matched.  This makes
it awkward to match against the whole output; for example, to check
whether it is empty.  Rework the function to exclude the prompt.

* test/lisp/eshell/eshell-tests-helpers.el (eshell-match-result):
Exclude any newly emitted prompt from the text against which the
regexp is matched.  Additionally, the function no longer moves point.
* test/lisp/eshell/eshell-tests.el (eshell-test/flush-output): Update
and simplify test given how eshell-match-result no longer moves point.
---
 test/lisp/eshell/eshell-tests-helpers.el | 9 +++++----
 test/lisp/eshell/eshell-tests.el         | 5 ++---
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/test/lisp/eshell/eshell-tests-helpers.el b/test/lisp/eshell/eshell-tests-helpers.el
index 0930f82284..77f5313d57 100644
--- a/test/lisp/eshell/eshell-tests-helpers.el
+++ b/test/lisp/eshell/eshell-tests-helpers.el
@@ -66,10 +66,11 @@ eshell-insert-command
   (funcall (or func 'eshell-send-input)))
 
 (defun eshell-match-result (regexp)
-  "Check that text after `eshell-last-input-end' matches REGEXP."
-  (goto-char eshell-last-input-end)
-  (should (string-match-p regexp (buffer-substring-no-properties
-                                  (point) (point-max)))))
+  "Check that output of last command matches REGEXP."
+  (should
+   (string-match-p
+    regexp (buffer-substring-no-properties
+            (eshell-beginning-of-output) (eshell-end-of-output)))))
 
 (defun eshell-command-result-p (text regexp &optional func)
   "Insert a command at the end of the buffer."
diff --git a/test/lisp/eshell/eshell-tests.el b/test/lisp/eshell/eshell-tests.el
index 6aeefdfde2..542815df80 100644
--- a/test/lisp/eshell/eshell-tests.el
+++ b/test/lisp/eshell/eshell-tests.el
@@ -232,9 +232,8 @@ eshell-test/flush-output
   (with-temp-eshell
    (eshell-insert-command "echo alpha")
    (eshell-kill-output)
-   (eshell-match-result (regexp-quote "*** output flushed ***\n"))
-   (should (forward-line))
-   (should (= (point) eshell-last-output-start))))
+   (eshell-match-result
+    (concat "^" (regexp-quote "*** output flushed ***\n") "$"))))
 
 (ert-deftest eshell-test/run-old-command ()
   "Re-run an old command"
-- 
2.30.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: v6-0003-Add-Eshell-syntax-to-more-easily-bypass-Eshell-s-.patch --]
[-- Type: text/x-patch, Size: 20262 bytes --]

From aa30bf4b8fd3b9792149d95c86a15ecf573b99e6 Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Mon, 17 Jan 2022 15:15:36 -0700
Subject: [PATCH v6 3/3] Add Eshell syntax to more easily bypass Eshell's own
 pipelining

* etc/NEWS:
* doc/misc/eshell.texi (Input/Output): Document the new syntax.
* lisp/eshell/em-extpipe.el: New module (Bug#46351).
* test/lisp/eshell/em-extpipe-tests.el: New tests.
* lisp/eshell/esh-module.el (eshell-modules-list): Add `eshell-extpipe'.
---
 doc/misc/eshell.texi                 |  42 ++++++
 etc/NEWS                             |  10 ++
 lisp/eshell/em-extpipe.el            | 183 +++++++++++++++++++++++
 lisp/eshell/esh-module.el            |   1 +
 test/lisp/eshell/em-extpipe-tests.el | 207 +++++++++++++++++++++++++++
 5 files changed, 443 insertions(+)
 create mode 100644 lisp/eshell/em-extpipe.el
 create mode 100644 test/lisp/eshell/em-extpipe-tests.el

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index df6e3b861e..261e88d00c 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1142,6 +1142,48 @@ Input/Output
 The output function is called once on each line of output until
 @code{nil} is passed, indicating end of output.
 
+@section Running Shell Pipelines Natively
+When constructing shell pipelines that will move a lot of data, it is
+a good idea to bypass Eshell's own pipelining support and use the
+operating system shell's instead.  This is especially relevant when
+executing commands on a remote machine using Eshell's Tramp
+integration: using the remote shell's pipelining avoids copying the
+data which will flow through the pipeline to local Emacs buffers and
+then right back again.
+
+Eshell recognises a special syntax to make it easier to convert
+pipelines so as to bypass Eshell's pipelining.  Prefixing at least one
+@code{|}, @code{<} or @code{>} with an asterisk marks a command as
+intended for the operating system shell.  To make it harder to invoke
+this functionality accidentally, it is also required that the asterisk
+be preceded by whitespace or located at the start of input.  For
+example,
+
+@example
+ cat *.ogg *| my-cool-decoder >file
+@end example
+
+Executing this command will not copy all the data in the *.ogg files,
+nor the decoded data, into Emacs buffers, as would normally happen.
+
+The command is interpreted as extending up to the next @code{|}
+character which is not preceded by an unescaped asterisk following
+whitespace, or the end of the input if there is no such character.
+Thus, all @code{<} and @code{>} redirections occuring before the next
+asterisk-unprefixed @code{|} are implicitly prefixed with (whitespace
+and) asterisks.  An exception is that Eshell-specific redirects right
+at the end of the command are excluded.  This allows input like this:
+
+@example
+ foo *| baz >#<buffer quux>
+@end example
+
+@noindent which is equivalent to input like this:
+
+@example
+ sh -c "foo | baz" >#<buffer quux>
+@end example
+
 @node Extension modules
 @chapter Extension modules
 Eshell provides a facility for defining extension modules so that they
diff --git a/etc/NEWS b/etc/NEWS
index 5297db3e2d..68c0eba866 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -858,6 +858,16 @@ the Galeon web browser was released in September, 2008.
 
 *** New user option 'ruby-toggle-block-space-before-parameters'.
 
+** Eshell
+
++++
+*** New feature to easily bypass Eshell's own pipelining.
+Prefixing '|', '<' or '>' with an asterisk, i.e. '*|', '*<' or '*>',
+will cause the whole command to be passed to the operating system
+shell.  This is particularly useful to bypass Eshell's own pipelining
+support for pipelines which will move a lot of data.  See "Running
+Shell Pipelines Natively" in the Eshell manual.
+
 ** Miscellaneous
 
 ---
diff --git a/lisp/eshell/em-extpipe.el b/lisp/eshell/em-extpipe.el
new file mode 100644
index 0000000000..57aeec38ff
--- /dev/null
+++ b/lisp/eshell/em-extpipe.el
@@ -0,0 +1,183 @@
+;;; em-extpipe.el --- external shell pipelines  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2022 Free Software Foundation, Inc.
+
+;; Author: Sean Whitton <spwhitton@spwhitton.name>
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; When constructing shell pipelines that will move a lot of data, it
+;; is a good idea to bypass Eshell's own pipelining support and use
+;; the operating system shell's instead.  This module tries to make
+;; that easy to do.
+
+;;; Code:
+
+(require 'cl-lib)
+(require 'esh-arg)
+(require 'esh-io)
+(require 'esh-util)
+
+(eval-when-compile (require 'files-x))
+
+;;; Functions:
+
+(defun eshell-extpipe-initialize () ;Called from `eshell-mode' via intern-soft!
+  "Initialize external pipelines support."
+  (when (boundp 'eshell-special-chars-outside-quoting)
+    (setq-local
+     eshell-special-chars-outside-quoting
+     (append eshell-special-chars-outside-quoting (list ?\*))))
+  (add-hook 'eshell-parse-argument-hook
+            #'eshell-parse-external-pipeline -20 t)
+  (add-hook 'eshell-pre-rewrite-command-hook
+            #'eshell-rewrite-external-pipeline -20 t))
+
+(defun eshell-parse-external-pipeline ()
+  "Parse a pipeline intended for execution by the external shell.
+
+A sequence of arguments is rewritten to use the operating system
+shell when it contains `*|', `*<' or `*>', where the asterisk is
+preceded by whitespace or located at the start of input.
+
+The command extends to the next `|' character which is not
+preceded by an unescaped asterisk following whitespace, or the
+end of input, except that any Eshell-specific output redirections
+occurring at the end are excluded.  Any other `<' or `>'
+appearing before the end of the command are treated as though
+preceded by (whitespace and) an asterisk.
+
+For example,
+
+    foo <bar *| baz >#<buffer quux>
+
+is equivalent to
+
+    sh -c \"foo <bar | baz\" >#<buffer quux>
+
+when `shell-file-name' is `sh' and `shell-command-switch' is
+`-c', but in
+
+    foo >#<buffer quux> *| baz
+
+and
+
+    foo *| baz >#<buffer quux> --some-argument
+
+the Eshell-specific redirect will be passed on to the operating
+system shell, probably leading to undesired results.
+
+This function must appear early in `eshell-parse-argument-hook'
+to ensure that operating system shell syntax is not interpreted
+as though it were Eshell syntax."
+  ;; Our goal is to wrap the external command to protect it from the
+  ;; other members of `eshell-parse-argument-hook'.  We must avoid
+  ;; misinterpreting a quoted `*|', `*<' or `*>' as indicating an
+  ;; external pipeline, hence the structure of the loop in `findbeg1'.
+  (cl-flet
+      ((findbeg1 (pat &optional go (bound (point-max)))
+         (let* ((start (point))
+                (result
+                 (catch 'found
+                   (while (> bound (point))
+                     (let* ((found
+                             (save-excursion
+                               (re-search-forward "['\"\\]" bound t)))
+                            (next (or (and found (match-beginning 0))
+                                      bound)))
+                       (if (re-search-forward pat next t)
+                           (throw 'found (match-beginning 1))
+                         (goto-char next)
+                         (while (or (eshell-parse-backslash)
+                                    (eshell-parse-double-quote)
+                                    (eshell-parse-literal-quote)))))))))
+           (goto-char (if (and result go) (match-end 0) start))
+           result)))
+    (unless (or eshell-current-argument eshell-current-quoted)
+      (let ((beg (point)) end
+            (next-marked (findbeg1 "\\(?:\\=\\|\\s-\\)\\(\\*[|<>]\\)"))
+            (next-unmarked
+             (or (findbeg1 "\\(?:\\=\\|[^*]\\|\\S-\\*\\)\\(|\\)")
+                 (point-max))))
+        (when (and next-marked (> next-unmarked next-marked)
+                   (or (> next-marked (point))
+                       (looking-back "\\`\\|\\s-" nil)))
+          ;; Skip to the final segment of the external pipeline.
+          (while (findbeg1 "\\(?:\\=\\|\\s-\\)\\(\\*|\\)" t))
+          ;; Find output redirections.
+          (while (findbeg1
+                  "\\([0-9]?>+&?[0-9]?\\s-*\\S-\\)" t next-unmarked)
+            ;; Is the output redirection Eshell-specific?  We have our
+            ;; own logic, rather than calling `eshell-parse-argument',
+            ;; to avoid specifying here all the possible cars of
+            ;; parsed special references -- `get-buffer-create' etc.
+            (forward-char -1)
+            (let ((this-end
+                   (save-match-data
+                     (cond ((looking-at "#<")
+                            (forward-char 1)
+                            (1+ (eshell-find-delimiter ?\< ?\>)))
+                           ((and (looking-at "/\\S-+")
+                                 (assoc (match-string 0)
+                                        eshell-virtual-targets))
+                            (match-end 0))))))
+              (cond ((and this-end end)
+                     (goto-char this-end))
+                    (this-end
+                     (goto-char this-end)
+                     (setq end (match-beginning 0)))
+                    (t
+                     (setq end nil)))))
+          ;; We've moved past all Eshell-specific output redirections
+          ;; we could find.  If there is only whitespace left, then
+          ;; `end' is right before redirections we should exclude;
+          ;; otherwise, we must include everything.
+          (unless (and end (skip-syntax-forward "\s" next-unmarked)
+                       (= next-unmarked (point)))
+            (setq end next-unmarked))
+          (let ((cmd (string-trim
+                      (buffer-substring-no-properties beg end))))
+            (goto-char end)
+            ;; We must now drop the asterisks, unless quoted/escaped.
+            (with-temp-buffer
+              (insert cmd)
+              (goto-char (point-min))
+              (cl-loop
+               for next = (findbeg1 "\\(?:\\=\\|\\s-\\)\\(\\*[|<>]\\)" t)
+               while next do (forward-char -2) (delete-char 1))
+              (eshell-finish-arg
+               `(eshell-external-pipeline ,(buffer-string))))))))))
+
+(defun eshell-rewrite-external-pipeline (terms)
+  "Rewrite an external pipeline in TERMS as parsed by
+`eshell-parse-external-pipeline', which see."
+  (while terms
+    (when (and (listp (car terms))
+               (eq (caar terms) 'eshell-external-pipeline))
+      (with-connection-local-variables
+       (setcdr terms (cl-list*
+                      shell-command-switch (cadar terms) (cdr terms)))
+       (setcar terms shell-file-name)))
+    (setq terms (cdr terms))))
+
+(defsubst eshell-external-pipeline (&rest _args)
+  "Stub to generate an error if a pipeline is not rewritten."
+  (error "Unhandled external pipeline in input text"))
+
+(provide 'em-extpipe)
+;;; esh-extpipe.el ends here
diff --git a/lisp/eshell/esh-module.el b/lisp/eshell/esh-module.el
index ade151d7cd..14e91912d1 100644
--- a/lisp/eshell/esh-module.el
+++ b/lisp/eshell/esh-module.el
@@ -54,6 +54,7 @@ eshell-modules-list
     eshell-basic
     eshell-cmpl
     eshell-dirs
+    eshell-extpipe
     eshell-glob
     eshell-hist
     eshell-ls
diff --git a/test/lisp/eshell/em-extpipe-tests.el b/test/lisp/eshell/em-extpipe-tests.el
new file mode 100644
index 0000000000..8b69e0504d
--- /dev/null
+++ b/test/lisp/eshell/em-extpipe-tests.el
@@ -0,0 +1,207 @@
+;;; em-extpipe-tests.el --- em-extpipe test suite  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2022 Free Software Foundation, Inc.
+
+;; Author: Sean Whitton <spwhitton@spwhitton.name>
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+
+;;; Code:
+
+(require 'cl-lib)
+(require 'ert)
+(require 'ert-x)
+(require 'em-extpipe)
+(eval-and-compile
+  (load (expand-file-name "eshell-tests-helpers"
+                          (file-name-directory (or load-file-name
+                                                   default-directory)))))
+
+(defvar eshell-history-file-name)
+(defvar eshell-test--max-subprocess-time)
+(declare-function eshell-command-result-p "eshell-tests-helpers")
+
+(defmacro em-extpipe-tests--deftest (name input &rest body)
+  (declare (indent 2))
+  `(ert-deftest ,name ()
+     (cl-macrolet
+         ((should-parse (expected)
+            `(let ((shell-file-name "sh")
+                   (shell-command-switch "-c"))
+               ;; Strip `eshell-trap-errors'.
+               (should (equal ,expected
+                              (cadr (eshell-parse-command input))))))
+          (with-substitute-for-temp (&rest body)
+            ;; Substitute name of an actual temporary file and/or
+            ;; buffer into `input'.  The substitution logic is
+            ;; appropriate for only the use we put it to in this file.
+            `(ert-with-temp-file temp
+               (let ((temp-buffer (generate-new-buffer " *temp*" t)))
+                 (unwind-protect
+                     (let ((input
+                            (replace-regexp-in-string
+                             "temp\\([^>]\\|\\'\\)" temp
+                             (string-replace "#<buffer temp>"
+                                             (buffer-name temp-buffer)
+                                             input))))
+                       ,@body)
+                   (when (buffer-name temp-buffer)
+                     (kill-buffer temp-buffer))))))
+          (temp-should-string= (expected)
+            `(string= ,expected (string-trim-right
+                                 (with-temp-buffer
+                                   (insert-file-contents temp)
+                                   (buffer-string)))))
+          (temp-buffer-should-string= (expected)
+            `(string= ,expected (string-trim-right
+                                 (with-current-buffer temp-buffer
+                                   (buffer-string))))))
+       (skip-unless shell-file-name)
+       (skip-unless shell-command-switch)
+       (skip-unless (executable-find shell-file-name))
+       (let ((input ,input))
+         (with-temp-eshell ,@body)))))
+
+(em-extpipe-tests--deftest em-extpipe-test-1
+    "echo \"bar\" *| rev >temp"
+  (skip-unless (executable-find "rev"))
+  (should-parse '(eshell-named-command
+                  "sh" (list "-c" "echo \"bar\" | rev >temp")))
+  (with-substitute-for-temp
+   (eshell-command-result-p input "^$")
+   (temp-should-string= "rab")))
+
+(em-extpipe-tests--deftest em-extpipe-test-2
+    "echo \"bar\" | rev *>temp"
+  (skip-unless (executable-find "rev"))
+  (should-parse
+   '(eshell-execute-pipeline
+     '((eshell-named-command "echo" (list (eshell-escape-arg "bar")))
+       (eshell-named-command "sh" (list "-c" "rev >temp")))))
+  (with-substitute-for-temp
+   (eshell-command-result-p input "^$")
+   (temp-should-string= "rab")))
+
+(em-extpipe-tests--deftest em-extpipe-test-3 "foo *| bar | baz -d"
+  (should-parse
+   '(eshell-execute-pipeline
+     '((eshell-named-command "sh" (list "-c" "foo | bar"))
+       (eshell-named-command "baz" (list "-d"))))))
+
+(em-extpipe-tests--deftest em-extpipe-test-4
+    "echo \"bar\" *| rev >#<buffer temp>"
+  (skip-unless (executable-find "rev"))
+  (should-parse
+   '(progn
+      (ignore
+       (eshell-set-output-handle 1 'overwrite
+				 (get-buffer-create "temp")))
+      (eshell-named-command "sh"
+			    (list "-c" "echo \"bar\" | rev"))))
+  (with-substitute-for-temp
+   (eshell-command-result-p input "^$")
+   (temp-buffer-should-string= "rab")))
+
+(em-extpipe-tests--deftest em-extpipe-test-5
+    "foo *| bar >#<buffer quux> baz"
+  (should-parse '(eshell-named-command
+                  "sh" (list "-c" "foo | bar >#<buffer quux> baz"))))
+
+(em-extpipe-tests--deftest em-extpipe-test-6
+    "foo >#<buffer quux> *| bar baz"
+  (should-parse '(eshell-named-command
+                  "sh" (list "-c" "foo >#<buffer quux> | bar baz"))))
+
+(em-extpipe-tests--deftest em-extpipe-test-7
+    "foo *| bar >#<buffer quux> >>#<process other>"
+  (should-parse
+   '(progn
+      (ignore
+       (eshell-set-output-handle 1 'overwrite
+				 (get-buffer-create "quux")))
+      (ignore
+       (eshell-set-output-handle 1 'append
+				 (get-process "other")))
+      (eshell-named-command "sh"
+			    (list "-c" "foo | bar")))))
+
+(em-extpipe-tests--deftest em-extpipe-test-8
+    "foo *| bar >/dev/kill | baz"
+  (should-parse
+   '(eshell-execute-pipeline
+     '((progn
+	 (ignore
+	  (eshell-set-output-handle 1 'overwrite "/dev/kill"))
+	 (eshell-named-command "sh"
+			       (list "-c" "foo | bar")))
+       (eshell-named-command "baz")))))
+
+(em-extpipe-tests--deftest em-extpipe-test-9 "foo \\*| bar"
+  (should-parse
+   '(eshell-execute-pipeline
+     '((eshell-named-command "foo"
+                             (list (eshell-escape-arg "*")))
+       (eshell-named-command "bar")))))
+
+(em-extpipe-tests--deftest em-extpipe-test-10 "foo \"*|\" *>bar"
+  (should-parse
+   '(eshell-named-command "sh" (list "-c" "foo \"*|\" >bar"))))
+
+(em-extpipe-tests--deftest em-extpipe-test-11 "foo '*|' bar"
+  (should-parse '(eshell-named-command
+                  "foo" (list (eshell-escape-arg "*|") "bar"))))
+
+(em-extpipe-tests--deftest em-extpipe-test-12 ">foo bar *| baz"
+  (should-parse
+   '(eshell-named-command "sh" (list "-c" ">foo bar | baz"))))
+
+(em-extpipe-tests--deftest em-extpipe-test-13 "foo*|bar"
+  (should-parse '(eshell-execute-pipeline
+                  '((eshell-named-command (concat "foo" "*"))
+                    (eshell-named-command "bar")))))
+
+(em-extpipe-tests--deftest em-extpipe-test-14 "tac *<temp"
+  (skip-unless (executable-find "tac"))
+  (should-parse '(eshell-named-command "sh" (list "-c" "tac <temp")))
+  (with-substitute-for-temp
+   (with-temp-buffer (insert "bar\nbaz\n") (write-file temp))
+   (eshell-command-result-p input "baz\nbar")))
+
+(em-extpipe-tests--deftest em-extpipe-test-15 "echo \"bar\" *| cat"
+  (skip-unless (executable-find "cat"))
+  (should-parse
+   '(eshell-named-command "sh" (list "-c" "echo \"bar\" | cat")))
+  (cl-letf (((symbol-function 'eshell/cat)
+             (lambda (&rest _args) (eshell-print "nonsense"))))
+    (eshell-command-result-p input "bar")
+    (eshell-command-result-p "echo \"bar\" | cat" "nonsense")))
+
+(em-extpipe-tests--deftest em-extpipe-test-16 "echo \"bar\" *| rev"
+  (skip-unless (executable-find "rev"))
+  (should-parse
+   '(eshell-named-command "sh" (list "-c" "echo \"bar\" | rev")))
+  (let ((eshell-prefer-lisp-functions t))
+    (cl-letf (((symbol-function 'rev)
+               (lambda (&rest _args) (eshell-print "nonsense"))))
+      (eshell-command-result-p input "rab")
+      (eshell-command-result-p "echo \"bar\" | rev" "nonsense"))))
+
+(provide 'em-extpipe-tests)
+
+;;; em-extpipe-tests.el ends here
-- 
2.30.2

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Lars Ingebrigtsen]

Sean Whitton <spwhitton@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

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Lars Ingebrigtsen]

Sean Whitton <spwhitton@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

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Sean Whitton]

[-- Attachment #1: Type: text/plain, Size: 551 bytes --]

Hello,

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

> Sean Whitton <spwhitton@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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-missing-provide-in-recently-introduced-em-extpip.patch --]
[-- Type: text/x-patch, Size: 767 bytes --]

From 67017d9f13ce6f4c2ae6106b84db8d883b8ed551 Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Mon, 24 Jan 2022 14:40:51 -0700
Subject: [PATCH] ; Fix missing `provide' in recently introduced
 em-extpipe-tests.el

---
 test/lisp/eshell/em-extpipe-tests.el | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/test/lisp/eshell/em-extpipe-tests.el b/test/lisp/eshell/em-extpipe-tests.el
index 1283b6b361..8b69e0504d 100644
--- a/test/lisp/eshell/em-extpipe-tests.el
+++ b/test/lisp/eshell/em-extpipe-tests.el
@@ -202,4 +202,6 @@ em-extpipe-test-16
       (eshell-command-result-p input "rab")
       (eshell-command-result-p "echo \"bar\" | rev" "nonsense"))))
 
+(provide 'em-extpipe-tests)
+
 ;;; em-extpipe-tests.el ends here
-- 
2.30.2

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Lars Ingebrigtsen]

Sean Whitton <spwhitton@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

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Sean Whitton]

Hello,

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

> Sean Whitton <spwhitton@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

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Jim Porter]

On 1/24/2022 12:46 PM, Lars Ingebrigtsen wrote:
> Sean Whitton <spwhitton@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 ' ...

bug#53518: 29.0.50; em-extpipe breaks input of sharp-quoted Lisp symbols

[Sean Whitton]

Hello Jim,

On Mon 24 Jan 2022 at 06:39pm -08, Jim Porter wrote:

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

Thanks for the report.  I have a fix; incoming once I have a bug number.

-- 
Sean Whitton

bug#53518: 29.0.50; em-extpipe breaks input of sharp-quoted Lisp symbols

[Sean Whitton]

[-- Attachment #1: Type: text/plain, Size: 1261 bytes --]

Hello again,

On Mon 24 Jan 2022 at 06:39pm -08, Jim Porter wrote:

> 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

Out of curiosity, why is there a need for the sharpquote?  Why not just
'echo hi | upcase'?  Is it to do with requesting the new piping?

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

Here is a patch which has `eshell-parse-external-pipeline' try skipping
over sharp-quoted symbols before trying to skip over single-quoted
strings.

-- 
Sean Whitton

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-input-of-sharp-quoted-symbols-in-Eshell-with-em-.patch --]
[-- Type: text/x-diff, Size: 2883 bytes --]

From 8725495cb91ebf831cd623c58f84a4347e6f1651 Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Mon, 24 Jan 2022 22:39:15 -0700
Subject: [PATCH] Fix input of sharp-quoted symbols in Eshell with em-extpipe

* lisp/eshell/em-extpipe.el (eshell-parse-external-pipeline): Fix
misinterpreting sharp-quoted symbols as the beginning of single-quoted
strings (Bug#53518).  Add protection against a possible infinite loop.
* test/lisp/eshell/em-extpipe-tests.el (em-extpipe-test-17): New test.
---
 lisp/eshell/em-extpipe.el            | 12 +++++++++---
 test/lisp/eshell/em-extpipe-tests.el |  4 ++++
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/lisp/eshell/em-extpipe.el b/lisp/eshell/em-extpipe.el
index 57aeec38ff..b0f92e41dc 100644
--- a/lisp/eshell/em-extpipe.el
+++ b/lisp/eshell/em-extpipe.el
@@ -97,15 +97,21 @@ eshell-parse-external-pipeline
                    (while (> bound (point))
                      (let* ((found
                              (save-excursion
-                               (re-search-forward "['\"\\]" bound t)))
+                               (re-search-forward
+                                "\\(?:#?'\\|\"\\|\\\\\\)" bound t)))
                             (next (or (and found (match-beginning 0))
                                       bound)))
                        (if (re-search-forward pat next t)
                            (throw 'found (match-beginning 1))
                          (goto-char next)
-                         (while (or (eshell-parse-backslash)
+                         (while (or (eshell-parse-lisp-argument)
+                                    (eshell-parse-backslash)
                                     (eshell-parse-double-quote)
-                                    (eshell-parse-literal-quote)))))))))
+                                    (eshell-parse-literal-quote)))
+                         ;; Guard against an infinite loop if none of
+                         ;; the parsers moved us forward.
+                         (unless (or (> (point) next) (eobp))
+                           (forward-char 1))))))))
            (goto-char (if (and result go) (match-end 0) start))
            result)))
     (unless (or eshell-current-argument eshell-current-quoted)
diff --git a/test/lisp/eshell/em-extpipe-tests.el b/test/lisp/eshell/em-extpipe-tests.el
index 1283b6b361..0879ad5b0c 100644
--- a/test/lisp/eshell/em-extpipe-tests.el
+++ b/test/lisp/eshell/em-extpipe-tests.el
@@ -202,4 +202,8 @@ em-extpipe-test-16
       (eshell-command-result-p input "rab")
       (eshell-command-result-p "echo \"bar\" | rev" "nonsense"))))
 
+;; Confirm we don't break input of sharp-quoted symbols (Bug#53518).
+(em-extpipe-tests--deftest em-extpipe-test-17 "funcall #'upcase foo"
+  (eshell-command-result-p input "FOO"))
+
 ;;; em-extpipe-tests.el ends here
-- 
2.30.2

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Michael Albinus]

Jim Porter <jporterbugs@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.

bug#53518: 29.0.50; em-extpipe breaks input of sharp-quoted Lisp symbols

[Lars Ingebrigtsen]

Sean Whitton <spwhitton@spwhitton.name> writes:

> Here is a patch which has `eshell-parse-external-pipeline' try skipping
> over sharp-quoted symbols before trying to skip over single-quoted
> strings.

This leads to the following compilation warning:

  ELC      eshell/em-extpipe.elc
In end of data:
eshell/em-extpipe.el:107:38: Warning: the function `eshell-parse-lisp-argument' is not known to be defined.


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

bug#53518: 29.0.50; em-extpipe breaks input of sharp-quoted Lisp symbols

[Sean Whitton]

[-- Attachment #1: Type: text/plain, Size: 348 bytes --]

Hello,

On Tue 25 Jan 2022 at 01:26pm +01, Lars Ingebrigtsen wrote:

> This leads to the following compilation warning:
>
>   ELC      eshell/em-extpipe.elc
> In end of data:
> eshell/em-extpipe.el:107:38: Warning: the function `eshell-parse-lisp-argument' is not known to be defined.

Apologies for this.  Here is a fixed patch.

-- 
Sean Whitton

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v2-0001-Fix-input-of-sharp-quoted-symbols-in-Eshell-with-.patch --]
[-- Type: text/x-diff, Size: 3009 bytes --]

From 7981a5de5f06fd8879e57ac4d780370965999d7b Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Mon, 24 Jan 2022 22:39:15 -0700
Subject: [PATCH v2] Fix input of sharp-quoted symbols in Eshell with
 em-extpipe

* lisp/eshell/em-extpipe.el (eshell-parse-external-pipeline): Fix
misinterpreting sharp-quoted symbols as the beginning of single-quoted
strings (Bug#53518).  Add protection against a possible infinite loop.
* test/lisp/eshell/em-extpipe-tests.el (em-extpipe-test-17): New test.
---
 lisp/eshell/em-extpipe.el            | 13 ++++++++++---
 test/lisp/eshell/em-extpipe-tests.el |  4 ++++
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/lisp/eshell/em-extpipe.el b/lisp/eshell/em-extpipe.el
index 57aeec38ff..eb5b3bfe1d 100644
--- a/lisp/eshell/em-extpipe.el
+++ b/lisp/eshell/em-extpipe.el
@@ -30,6 +30,7 @@
 
 (require 'cl-lib)
 (require 'esh-arg)
+(require 'esh-cmd)
 (require 'esh-io)
 (require 'esh-util)
 
@@ -97,15 +98,21 @@ eshell-parse-external-pipeline
                    (while (> bound (point))
                      (let* ((found
                              (save-excursion
-                               (re-search-forward "['\"\\]" bound t)))
+                               (re-search-forward
+                                "\\(?:#?'\\|\"\\|\\\\\\)" bound t)))
                             (next (or (and found (match-beginning 0))
                                       bound)))
                        (if (re-search-forward pat next t)
                            (throw 'found (match-beginning 1))
                          (goto-char next)
-                         (while (or (eshell-parse-backslash)
+                         (while (or (eshell-parse-lisp-argument)
+                                    (eshell-parse-backslash)
                                     (eshell-parse-double-quote)
-                                    (eshell-parse-literal-quote)))))))))
+                                    (eshell-parse-literal-quote)))
+                         ;; Guard against an infinite loop if none of
+                         ;; the parsers moved us forward.
+                         (unless (or (> (point) next) (eobp))
+                           (forward-char 1))))))))
            (goto-char (if (and result go) (match-end 0) start))
            result)))
     (unless (or eshell-current-argument eshell-current-quoted)
diff --git a/test/lisp/eshell/em-extpipe-tests.el b/test/lisp/eshell/em-extpipe-tests.el
index 1283b6b361..0879ad5b0c 100644
--- a/test/lisp/eshell/em-extpipe-tests.el
+++ b/test/lisp/eshell/em-extpipe-tests.el
@@ -202,4 +202,8 @@ em-extpipe-test-16
       (eshell-command-result-p input "rab")
       (eshell-command-result-p "echo \"bar\" | rev" "nonsense"))))
 
+;; Confirm we don't break input of sharp-quoted symbols (Bug#53518).
+(em-extpipe-tests--deftest em-extpipe-test-17 "funcall #'upcase foo"
+  (eshell-command-result-p input "FOO"))
+
 ;;; em-extpipe-tests.el ends here
-- 
2.30.2

bug#53518: 29.0.50; em-extpipe breaks input of sharp-quoted Lisp symbols

[Jim Porter]

On 1/25/2022 12:50 AM, Sean Whitton wrote:
> Hello again,
> 
> On Mon 24 Jan 2022 at 06:39pm -08, Jim Porter wrote:
> 
>> 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
> 
> Out of curiosity, why is there a need for the sharpquote?  Why not just
> 'echo hi | upcase'?  Is it to do with requesting the new piping?

It becomes more relevant with my WIP patches to support piping to Lisp 
functions, but it means something different today too. "upcase" *calls* 
the function `upcase', whereas "#'upcase" evaluates to the function 
object itself. For example:

   $ upcase
   Wrong number of arguments: #<subr upcase>, 0
   $ #'upcase
   upcase

Or, for a slightly different, but more practical example today:

   $ mapcar upcase $(list "foo" "bar")
   Invalid function: "upcase"
   $ mapcar #'upcase $(list "foo" "bar")
   ("FOO" "BAR")

In this case, "upcase" in the first command is just the string "upcase", 
whereas "#'upcase" refers to the function as in the other case. The 
shortest other way I'm aware of to spell that using shell-like 
invocation would be:

   $ mapcar $(quote upcase) $(list "foo" "bar")
   ("FOO" "BAR")

For testing purposes, it would probably also be useful to ensure that 
Lisp syntax works too:

   $ (mapcar #'upcase '("foo" "bar"))
   ("FOO" "BAR")

Just for the sake of completeness, in my WIP patches, "echo hi | 
#'upcase" and "echo hi | upcase" will also do different things, 
following the above precedent. The former pipes the output of echo to 
the function upcase. The latter pipes the output of echo to the *result* 
of calling the function upcase with no arguments. In addition to being 
consistent with how Eshell currently works, this allows you to do things 
like "echo hi | less -N", where "less -N" is evaluated as an Eshell 
command and then returns a pseudo-pipe for echo to connect to.

bug#46351: 28.0.50; Add convenient way to bypass Eshell's own pipelining

[Jim Porter]

On 1/25/2022 12:54 AM, Michael Albinus wrote:
> Jim Porter <jporterbugs@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.)

bug#53518: 29.0.50; em-extpipe breaks input of sharp-quoted Lisp symbols

[Sean Whitton]

Hello,

On Tue 25 Jan 2022 at 10:14AM -08, Jim Porter wrote:

> It becomes more relevant with my WIP patches to support piping to Lisp
> functions, but it means something different today too.

Indeed.

> Just for the sake of completeness, in my WIP patches, "echo hi |
> #'upcase" and "echo hi | upcase" will also do different things,
> following the above precedent. The former pipes the output of echo to
> the function upcase. The latter pipes the output of echo to the *result*
> of calling the function upcase with no arguments.

Okay, thanks.  I asked because I thought you were only planning support
the former case.

> In addition to being consistent with how Eshell currently works, this
> allows you to do things like "echo hi | less -N", where "less -N" is
> evaluated as an Eshell command and then returns a pseudo-pipe for echo
> to connect to.

It's not quite clear to me how this is consistent with how Eshell
currently works.  When you type "echo hi | cat" is it right to say that
"cat" is evaluated to produce the thing to which the output of the first
command is piped?  Perhaps I'm not thinking broadly enough.

-- 
Sean Whitton

bug#53518: 29.0.50; em-extpipe breaks input of sharp-quoted Lisp symbols

[Jim Porter]

On 1/25/2022 12:01 PM, Sean Whitton wrote:
>> In addition to being consistent with how Eshell currently works, this
>> allows you to do things like "echo hi | less -N", where "less -N" is
>> evaluated as an Eshell command and then returns a pseudo-pipe for echo
>> to connect to.
> 
> It's not quite clear to me how this is consistent with how Eshell
> currently works.  When you type "echo hi | cat" is it right to say that
> "cat" is evaluated to produce the thing to which the output of the first
> command is piped?  Perhaps I'm not thinking broadly enough.

In general, I just mean that when running a command "foo" (in a pipeline 
or not), if Eshell finds a Lisp function for "foo", it always calls that 
function. On the other hand, "#'foo" always evaluates to the function 
object itself.

So, since "cat" points to a Lisp function, your summary is pretty close, 
I'd say. However, I think I'd describe it as "cat is evaluated to hook 
up the pipeline into what the user meant". Currently, it works like so:

1. `eshell-parse-command' turns "echo hi | cat" into:
    (eshell-commands
     (progn
       (run-hooks 'eshell-pre-command-hook)
       (catch 'top-level
         (progn
           (eshell-trap-errors
            (eshell-execute-pipeline
             '((eshell-named-command "echo" (list "hi"))
               (eshell-named-command "cat"))))))
       (run-hooks 'eshell-post-command-hook)))

2. Eshell iteratively evaluates that, eventually getting to to the "cat"
    part

3. `eshell-named-command' looks for "cat" and calls the function
    `eshell/cat'

4. `eshell/cat' notices that it's being called in a pipeline and bails
    out, throwing `eshell-replace-command' to convert "cat" into
    "/bin/cat" (or wherever your cat program lives)

5. This gets reevaluated in `eshell-do-eval' and calls
    `eshell-external-command' for "/bin/cat"

6. This calls `eshell-gather-process-output', returning a process
    object with the pipes connected

That is, Eshell evaluates `eshell/cat' (which could do anything, really) 
with the ultimate goal of hooking up the pipeline. Currently that just 
means "call /bin/cat and connect pipes to the external process", but 
`eshell/cat' can do whatever it wants to achieve the result.

For the "pipe to a Lisp function" case, like "echo hi | #'upcase", 
what's actually happening under the hood in my patch it that "#'upcase" 
is evaluated, Eshell sees that the result is a function, and then the 
function is converted into a pseudo-pipe that gets properly hooked up to 
echo.

For a case like "echo hi | less -N", where "less" has a Lisp 
implementation, Eshell evaluates `(eshell/less "-N")', which could 
return a function that gets converted into a pseudo-pipe like above. Or 
(and this is how `eshell/less' will actually work), it returns its own 
pseudo-pipe that's hooked up properly; this case is analogous to 
`eshell-gather-process-output' returning a properly-connected process 
object.

The subtleties of Lisp functions in pipelines will probably be a bit 
easier to understand once I post patches, which should hopefully be this 
week or next. :) The patches work already, but I'm still implementing a 
few more pipeable Eshell commands in Lisp so that I can be sure the core 
implementation is flexible enough to be used for practical cases.

bug#53518: 29.0.50; em-extpipe breaks input of sharp-quoted Lisp symbols

[Sean Whitton]

Hello,

On Tue 25 Jan 2022 at 12:52PM -08, Jim Porter wrote:

> The subtleties of Lisp functions in pipelines will probably be a bit
> easier to understand once I post patches, which should hopefully be this
> week or next. :) The patches work already, but I'm still implementing a
> few more pipeable Eshell commands in Lisp so that I can be sure the core
> implementation is flexible enough to be used for practical cases.

Thank you for the explanation.  I still don't see how your proposal is a
natural extension of what Eshell already does, to be honest, but I'm
sure things will become clearer once you post your docs and tests, as
you say.

-- 
Sean Whitton

bug#53518: 29.0.50; em-extpipe breaks input of sharp-quoted Lisp symbols

[Jim Porter]

On 1/25/2022 8:48 AM, Sean Whitton wrote:
> On Tue 25 Jan 2022 at 01:26pm +01, Lars Ingebrigtsen wrote:
> 
>> This leads to the following compilation warning:
>>
>>    ELC      eshell/em-extpipe.elc
>> In end of data:
>> eshell/em-extpipe.el:107:38: Warning: the function `eshell-parse-lisp-argument' is not known to be defined.
> 
> Apologies for this.  Here is a fixed patch.

Thanks for the quick fix, this resolves everything on my end. I applied 
the patch to my local branch for Lisp function pipelining and all the 
tests in test/lisp/eshell/ now pass, including the new ones I'm working on.

bug#53518: 29.0.50; em-extpipe breaks input of sharp-quoted Lisp symbols

[Lars Ingebrigtsen]

Sean Whitton <spwhitton@spwhitton.name> writes:

> Apologies for this.  Here is a fixed patch.

Thanks; applied to Emacs 29.

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