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

From: Jim Porter <[email-masked]>
Date: Mon, 15 Aug 2022 11:08:45 -0700

On 8/15/2022 9:49 AM, Eli Zaretskii wrote:
> None of these reverts fixes the issue.  I tried both the tests in
> eshell-tests.el and the problematic command:
> 
>     *cat $<echo $eshell-in-pipeline-p | echo> | *cat

Thanks for testing. I might have to get an MS-Windows dev environment 
set up to dig into this further. I'm a bit worried that this is related 
to bug#55590, where I had to jump through some awkward hoops to fix a 
somewhat-related issue with Eshell subcommands. I think that's due to 
'eshell-do-eval' being written before lexical binding existed, and so 
the variable scoping doesn't work quite as expected. It could be that 
you only see this failure when using external commands due to some 
differences in timing.

If that's the problem here too, it would be a pain to fix (but it would 
likely also mean that this bug has been present for a long time, and my 
tests/fixes have just brought it to the surface). As mentioned in 
bug#55590, I think it'd be nice to rip out 'eshell-do-eval' and replace 
it with the generator.el machinery, since they're conceptually pretty 
similar. That's easier said than done though...

>> I did notice one weird issue though, and maybe this has something to do
>> with the problem: on MS Windows, if I run any command with a
>> $<subcommand>, the second and subsequent times, it warns me about
>> changing the temp file, e.g.:
>>
>>     3Zz80A has changed since visited or saved.  Save anyway? (yes or no)
>>
>> This probably shouldn't do that, and it could definitely cause issues in
>> the tests, which can't prompt the user for things like that.
> 
> I guess this means you somehow reuse the same temporary file on
> MS-Windows?  Maybe change the test a bit so that the file name
> definitely differs?

I need to investigate this a bit further, since on GNU/Linux, I get a 
new temp file every time. I think that's the behavior we want, or 
failing that, we could at least kill the temp file's buffer after we're 
done with it. I don't think there's much value in leaving it around.