Jim Porter <jporterbugs@gmail.com> writes:

> On 7/5/2024 7:34 AM, Thierry Volpiatto wrote:
>> Here the patch modified (only changed default value, a typo and the
>> commit message).
>
> Thanks. One question about this before I merge: is the 'cl-loop' part
> necessary? I glossed over that part when reviewing the patch
> previously, but thinking about it more (and consulting 'shell-command'
> and 'shell-command--same-buffer-confirm'), I'm not sure we need it.
>
> My understanding is that this would prompt the user if there were a
> buffer named "*Eshell Async Command Output*" (good), but also if there
> were a buffer named "*Eshell Async Command Output*<2>" (possibly
> unnecessary).

Why?

If we remove the cl-loop, we will have only:

(if (buffer-live-p (get-buffer "*Eshell Async Command Output*"))
    (do something according to eshell-command-async-buffer))

which will return nil and

(rename-buffer buf "*Eshell Async Command Output*" nil)

will run inconditionally without asking anything while the process in
"*Eshell Async Command Output*<2>" is still (maybe) running.

Did I miss something?

> I can see the reasoning for doing this for the 'confirm-kill-process'
> setting (it tries to keep the number of async Eshell commands to 1),
> but less so for the other settings. I don't think it's useful for
> 'confirm-new-buffer' or 'confirm-rename-buffer'.

Not useful for 'confirm-rename-buffer' but useful for
'confirm-new-buffer' no?

> Given the above, and that 'shell-command' doesn't do this, maybe
> Eshell shouldn't either?

As long as people don't delete their buffers manually until everything
is finished, yes we should behave as 'shell-command'.
We can remove the cl-loop if you want, anyway we will see if somebody
report a problem with this.

-- 
Thierry