bug#57129: 29.0.50; Improve behavior of conditionals in Eshell

[Jim Porter <jporterbugs@gmail.com>]

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

On 8/13/2022 10:07 PM, Eli Zaretskii wrote:
>> Cc: larsi@gnus.org, 57129@debbugs.gnu.org
>> From: Jim Porter <jporterbugs@gmail.com>
>> Date: Sat, 13 Aug 2022 11:56:20 -0700
>>
>> Sorry about that. I think that's the right fix for that case. Maybe it
>> would make sense to set 'eshell-prefer-lisp-functions' to t for most of
>> the Eshell tests to improve reproducibility on various platforms; tests
>> that want an external command can just put a "*" in front of the command
>> name.
> 
> But then this fragment from the Eshell manual:
> 
>       The command can be either an Elisp function or an external command.
>    Eshell looks first for an alias (*note Aliases::) with the same name as
>    the command, then a built-in (*note Built-ins::) or a function with the
>    same name; if there is no match, it then tries to execute it as an
>    external command.
> 
> seems to be inaccurate?  Since 'format' exists as a built-in command,
> why did Eshell in this case invoke the external command instead?

"Built-in" in that part of the manual refers to a function named 
'eshell/FOO'. If you run "FOO" as an Eshell command, it will check for 
'eshell/FOO' before an external command on your system. The manual could 
probably stand to be improved here.

>> I'm surprised that test fails on MS Windows, since it *should* be
>> testing internal Eshell logic that's not platform-specific. Based on the
>> failure, it looks like one of the following commands is returning the
>> wrong value:
>>
>>     echo {echo $eshell-in-pipeline-p | echo} | *cat
>>     echo ${echo $eshell-in-pipeline-p | echo} | *cat
>>     *cat $<echo $eshell-in-pipeline-p | echo> | *cat
>>
>> All of these should return 'first'.
> 
> The first two do; the last one returns nothing.
[snip]
> So how to investigate this failure further?

I have access to an MS Windows system (but don't have a build 
environment set up for native MS Windows builds), so I can try to 
reproduce this using the Emacs 29 pretest binaries in the next few days. 
Hopefully that works.

If you'd like to dig into this further yourself, you could try running 
this command in Eshell:

   eshell-parse-command '*cat $<echo $eshell-in-pipeline-p | echo> | *cat'

That will print the Lisp form that the command gets converted to. I've 
attached the result that I get in a recent Emacs 29 build on GNU/Linux. 
The two functions that set 'eshell-in-pipeline-p' in there are:

* 'eshell-as-subcommand', which resets it to nil so that the outer
   command's pipeline state doesn't interfere with the subcommand.

* 'eshell-execute-pipeline', which calls 'eshell-do-pipelines' (except
   on MS-DOS, I think) and should set 'eshell-in-pipeline-p' to 'first'
   in the above case.

>> If nothing else, it would probably be helpful to set up ERT explainers
>> so that the error messages are easier to understand. As it is now,
>> they're not very explanatory.
> 
> Indeed, more explanations in this and other tests will be most
> welcome.

Yeah, this test in particular is high up on the list of tests that needs 
more explanation. It's pretty subtle, and the test doesn't really serve 
as a good example of why someone would care that this behavior works the 
way the test demands it.

[-- Attachment #2: parsed-command.el --]
[-- Type: text/plain, Size: 1563 bytes --]

(eshell-trap-errors
 (eshell-execute-pipeline
  '((eshell-named-command
     (eshell-concat nil "*" "cat")
     (list
      (eshell-escape-arg
       (let
           ((indices
             (eshell-eval-indices 'nil)))
         (let
             ((eshell-current-handles
               (eshell-create-handles "/tmp/QqPFwo" 'overwrite)))
           (progn
             (eshell-as-subcommand
              (eshell-trap-errors
               (eshell-execute-pipeline
                '((eshell-named-command "echo"
                                        (list
                                         (eshell-escape-arg
                                          (let
                                              ((indices
                                                (eshell-eval-indices 'nil)))
                                            (eshell-get-variable
                                             #("eshell-in-pipeline-p" 0 20
                                               (escaped t))
                                             indices nil)))))
                  (progn
                    (ignore
                     (eshell-set-output-handle 1 'overwrite "/tmp/QqPFwo"))
                    (eshell-named-command "echo"))))))
             (ignore
              (nconc eshell-this-command-hook
                     (list
                      #'(lambda nil
                          (delete-file "/tmp/QqPFwo")))))
             (eshell-apply-indices "/tmp/QqPFwo" indices nil)))))))
    (eshell-named-command
     (eshell-concat nil "*" "cat")))))