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

From: Sean Whitton <[email-masked]>
Date: Tue, 18 Jan 2022 11:40:50 -0700

Hello,

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

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

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

> I think Robert found another possible issue.

I will incorporate his suggestion to require whitespace.

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

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

-- 
Sean Whitton