bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Dmitry Gutov]

Previously mentioned in bug#70901.

When a Tramp connection is configured as "direct-async-process",

1. If the buffer *Async Shell Command* does not exist, invoking M-& in a 
remote buffer from that connection makes it end with

   Process *Async Shell Command* finished

The command can be simple, like 'ls' or 'echo 123'. I also see this 
added to *Messages*:

   Tramp: Inserting 
‘/ssh:dgutov@fencepost.gnu.org:/home/d/dgutov/.tramp_history’...done

Which seems odd (tramp history is re-read every time like this, for some 
purpose?).

2. If the buffer such named already exists when command is invoked, then 
this doesn't happen (the output seems correct). But *Messages* says

   -l: finished.

...which is a big puzzling as well.

Also, scenario 1 (when the buffer doesn't exist yet) takes longer than 
2, but that might be a side-effect of implementation external to Tramp.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Eli Zaretskii]

> Date: Sun, 19 May 2024 03:19:00 +0300
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> Previously mentioned in bug#70901.
> 
> When a Tramp connection is configured as "direct-async-process",
> 
> 1. If the buffer *Async Shell Command* does not exist, invoking M-& in a 
> remote buffer from that connection makes it end with
> 
>    Process *Async Shell Command* finished
> 
> The command can be simple, like 'ls' or 'echo 123'. I also see this 
> added to *Messages*:
> 
>    Tramp: Inserting 
> ‘/ssh:dgutov@fencepost.gnu.org:/home/d/dgutov/.tramp_history’...done
> 
> Which seems odd (tramp history is re-read every time like this, for some 
> purpose?).
> 
> 2. If the buffer such named already exists when command is invoked, then 
> this doesn't happen (the output seems correct). But *Messages* says
> 
>    -l: finished.
> 
> ...which is a big puzzling as well.
> 
> Also, scenario 1 (when the buffer doesn't exist yet) takes longer than 
> 2, but that might be a side-effect of implementation external to Tramp.

The "finished" message comes from the default process sentinel, so if
Tramp wants to avoid that, it should install its own sentinel.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Dmitry Gutov]

On 19/05/2024 09:17, Eli Zaretskii wrote:
>> Date: Sun, 19 May 2024 03:19:00 +0300
>> From: Dmitry Gutov<dmitry@gutov.dev>
>>
>> Previously mentioned in bug#70901.
>>
>> When a Tramp connection is configured as "direct-async-process",
>>
>> 1. If the buffer*Async Shell Command*  does not exist, invoking M-& in a
>> remote buffer from that connection makes it end with
>>
>>     Process*Async Shell Command*  finished
>>
>> The command can be simple, like 'ls' or 'echo 123'. I also see this
>> added to*Messages*:
>>
>>     Tramp: Inserting
>> ‘/ssh:dgutov@fencepost.gnu.org:/home/d/dgutov/.tramp_history’...done
>>
>> Which seems odd (tramp history is re-read every time like this, for some
>> purpose?).
>>
>> 2. If the buffer such named already exists when command is invoked, then
>> this doesn't happen (the output seems correct). But*Messages*  says
>>
>>     -l: finished.
>>
>> ...which is a big puzzling as well.
>>
>> Also, scenario 1 (when the buffer doesn't exist yet) takes longer than
>> 2, but that might be a side-effect of implementation external to Tramp.
> The "finished" message comes from the default process sentinel, so if
> Tramp wants to avoid that, it should install its own sentinel.

shell-commands inserts a sentinel: shell-command-sentinel. So it never 
inserts that message when invoked locally.

Even if it did, the disparity between the two scenarios would still be 
something to investigate, I think.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Dmitry Gutov <dmitry@gutov.dev> writes:

Hi Dmitry,

> Previously mentioned in bug#70901.
>
> When a Tramp connection is configured as "direct-async-process",
>
> 1. If the buffer *Async Shell Command* does not exist, invoking M-& in
> a remote buffer from that connection makes it end with
>
>   Process *Async Shell Command* finished
>
> The command can be simple, like 'ls' or 'echo 123'. I also see this
> added to *Messages*:
>
>   Tramp: Inserting
>   ‘/ssh:dgutov@fencepost.gnu.org:/home/d/dgutov/.tramp_history’...done
>
> Which seems odd (tramp history is re-read every time like this, for
> some purpose?).
>
> 2. If the buffer such named already exists when command is invoked,
> then this doesn't happen (the output seems correct). But *Messages*
> says
>
>   -l: finished.
>
> ...which is a big puzzling as well.
>
> Also, scenario 1 (when the buffer doesn't exist yet) takes longer than
> 2, but that might be a side-effect of implementation external to
> Tramp.

Thanks for the bug report, I can reproduce it. Honestly, direct async
processes are a little bit under-implemented (by decision, they should
be fast). I'll check what could be done w/o loosing performance.

According to reading .tramp_history: this is performed in
comint-read-input-ring, called from shell-mode. comint-input-ring-file-name
is set in shell-mode, I don't see a trivial solution to suppress
this. Likely, we must extend shell-mode for this case.

Best regards, Michael.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Michael Albinus <michael.albinus@gmx.de> writes:

Hi Dmitry,

>> The command can be simple, like 'ls' or 'echo 123'. I also see this
>> added to *Messages*:
>>
>>   Tramp: Inserting
>>   ‘/ssh:dgutov@fencepost.gnu.org:/home/d/dgutov/.tramp_history’...done
>
> According to reading .tramp_history: this is performed in
> comint-read-input-ring, called from shell-mode. comint-input-ring-file-name
> is set in shell-mode, I don't see a trivial solution to suppress
> this. Likely, we must extend shell-mode for this case.

We could add a user option remote-file-name-inhibit-input-ring which
suppresses reading the remote histfile, when set to non-nil. See
appended patch.

Eli?

Best regards, Michael.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 1478 bytes --]

diff --git a/lisp/comint.el b/lisp/comint.el
index 3804932e01c..2415435546e 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -249,6 +249,11 @@ comint-input-ignoredups
   :type 'boolean
   :group 'comint)

+(defcustom remote-file-name-inhibit-input-ring nil
+  "If non-nil, inhibit `comint-input-ring'."
+  :type 'boolean
+  :version "30.1")
+
 (defcustom comint-input-ring-file-name nil
   "If non-nil, name of the file to read/write input history.
 See also `comint-read-input-ring' and `comint-write-input-ring'.
diff --git a/lisp/shell.el b/lisp/shell.el
index e6b315ee5c0..1ed04c46cf9 100644
--- a/lisp/shell.el
+++ b/lisp/shell.el
@@ -726,9 +726,13 @@ shell-mode
           (hsize (getenv "HISTSIZE"))
           (hfile (getenv "HISTFILE")))
       (when remote
-        ;; `shell-snarf-envar' does not work trustworthy.
-        (setq hsize (shell-command-to-string "echo -n $HISTSIZE")
-              hfile (shell-command-to-string "echo -n $HISTFILE")))
+        (if remote-file-name-inhibit-input-ring
+            (setq remote nil
+                  hsize nil
+                  hfile nil)
+          ;; `shell-snarf-envar' does not work trustworthy.
+          (setq hsize (shell-command-to-string "echo -n $HISTSIZE")
+                hfile (shell-command-to-string "echo -n $HISTFILE"))))
       (and (string-equal hfile "") (setq hfile nil))
       (and (stringp hsize)
 	   (integerp (setq hsize (string-to-number hsize)))

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Eli Zaretskii]

> Cc: 71049@debbugs.gnu.org
> Date: Fri, 24 May 2024 16:06:13 +0200
> From:  Michael Albinus via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> >> The command can be simple, like 'ls' or 'echo 123'. I also see this
> >> added to *Messages*:
> >>
> >>   Tramp: Inserting
> >>   ‘/ssh:dgutov@fencepost.gnu.org:/home/d/dgutov/.tramp_history’...done
> >
> > According to reading .tramp_history: this is performed in
> > comint-read-input-ring, called from shell-mode. comint-input-ring-file-name
> > is set in shell-mode, I don't see a trivial solution to suppress
> > this. Likely, we must extend shell-mode for this case.
> 
> We could add a user option remote-file-name-inhibit-input-ring which
> suppresses reading the remote histfile, when set to non-nil. See
> appended patch.
> 
> Eli?

Can you explain the effect of that option on the scenarios that
started this bug report?  I don't think I have a clear understanding
of that.

Why is the process being called by such bogus names anyway?

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

Hi Eli,

>> >> The command can be simple, like 'ls' or 'echo 123'. I also see this
>> >> added to *Messages*:
>> >>
>> >>   Tramp: Inserting
>> >>   ‘/ssh:dgutov@fencepost.gnu.org:/home/d/dgutov/.tramp_history’...done
>> >
>> > According to reading .tramp_history: this is performed in
>> > comint-read-input-ring, called from shell-mode. comint-input-ring-file-name
>> > is set in shell-mode, I don't see a trivial solution to suppress
>> > this. Likely, we must extend shell-mode for this case.
>> 
>> We could add a user option remote-file-name-inhibit-input-ring which
>> suppresses reading the remote histfile, when set to non-nil. See
>> appended patch.
>> 
>> Eli?
>
> Can you explain the effect of that option on the scenarios that
> started this bug report?  I don't think I have a clear understanding
> of that.

We're speaking about shell-mode. Let's try the command

--8<---------------cut here---------------start------------->8---
(let ((default-directory "/ssh::"))
  (async-shell-command "ls"))
--8<---------------cut here---------------end--------------->8---

Tramp calls several initialization commands. In shell-mode, we see

--8<---------------cut here---------------start------------->8---
      (when remote
        ;; `shell-snarf-envar' does not work trustworthy.
        (setq hsize (shell-command-to-string "echo -n $HISTSIZE")
              hfile (shell-command-to-string "echo -n $HISTFILE")))
--8<---------------cut here---------------end--------------->8---

This triggers the commands in Tramp

--8<---------------cut here---------------start------------->8---
18:18:41.784634 tramp-send-command (6) # ( cd /home/albinus/ && env INSIDE_EMACS\=30.0.50\,tramp\:2.7.1-pre /bin/sh -c echo\ -n\ \$HISTSIZE </dev/null; echo tramp_exit_status $? )
18:18:41.817091 tramp-wait-for-regexp (6) # 
tramp_exit_status 0
///1ab1c259aca5ea2a0696680da9e7ac35#$
18:20:03.211358 tramp-send-command (6) # ( cd /home/albinus/ && env INSIDE_EMACS\=30.0.50\,tramp\:2.7.1-pre /bin/sh -c echo\ -n\ \$HISTFILE </dev/null; echo tramp_exit_status $? )
18:20:03.247281 tramp-wait-for-regexp (6) # 
~/.tramp_history
tramp_exit_status 0
///1ab1c259aca5ea2a0696680da9e7ac35#$
--8<---------------cut here---------------end--------------->8---

Two roundtrips. In

--8<---------------cut here---------------start------------->8---
      (setq comint-input-ring-file-name
            (concat
             remote
	     (or hfile
		 (cond ((string-equal shell "bash") "~/.bash_history")
		       ((string-equal shell "ksh") "~/.sh_history")
		       ((string-equal shell "zsh") "~/.zsh_history")
		       (t "~/.history")))))
--8<---------------cut here---------------end--------------->8---

we know, that comint-input-ring-file-name is
"/ssh:gandalf:~/.tramp_history". Next is

--8<---------------cut here---------------start------------->8---
      (if (or (equal comint-input-ring-file-name "")
	      (equal (file-truename comint-input-ring-file-name)
		     (file-truename null-device)))
--8<---------------cut here---------------end--------------->8---

which results in another roundtrip for file-truename.

--8<---------------cut here---------------start------------->8---
18:23:32.705940 tramp-send-command (6) # (if test -h "/home/albinus/.tramp_history"; then echo t; else echo nil; fi) && \readlink --canonicalize-missing /home/albinus/.tramp_history 2>/dev/null; echo tramp_exit_status $?
18:23:32.736575 tramp-wait-for-regexp (6) # 
nil
/home/albinus/.tramp_history
tramp_exit_status 0
///1ab1c259aca5ea2a0696680da9e7ac35#$
--8<---------------cut here---------------end--------------->8---

And finally, the history file is inserted into a buffer by the call of

--8<---------------cut here---------------start------------->8---
(comint-read-input-ring t)
--8<---------------cut here---------------end--------------->8---

which gives us another 6 roundtrips.

--8<---------------cut here---------------start------------->8---
18:26:07.617089 tramp-send-command (6) # test -r /home/albinus/.tramp_history 2>/dev/null; echo tramp_exit_status $?
18:26:07.649035 tramp-wait-for-regexp (6) # 
tramp_exit_status 0
///1ab1c259aca5ea2a0696680da9e7ac35#$
18:26:07.651779 tramp-send-command (6) # test -e /home/albinus/.cache/emacs/ 2>/dev/null; echo tramp_exit_status $?
18:26:07.658773 tramp-wait-for-regexp (6) # 
tramp_exit_status 0
///1ab1c259aca5ea2a0696680da9e7ac35#$
18:26:07.659542 tramp-send-command (6) # test -w /home/albinus/.cache/emacs/ 2>/dev/null; echo tramp_exit_status $?
18:26:07.724554 tramp-wait-for-regexp (6) # 
tramp_exit_status 0
///1ab1c259aca5ea2a0696680da9e7ac35#$
18:26:07.726639 tramp-send-command (6) # (if test -h "/home/albinus/.tramp_history"; then echo t; else echo nil; fi) && \readlink --canonicalize-missing /home/albinus/.tramp_history 2>/dev/null; echo tramp_exit_status $?
18:26:07.761576 tramp-wait-for-regexp (6) # 
nil
/home/albinus/.tramp_history
tramp_exit_status 0
///1ab1c259aca5ea2a0696680da9e7ac35#$
18:26:07.805806 tramp-send-command (6) # tramp_stat_file_attributes_with_selinux /home/albinus/.tramp_history 2>/dev/null; echo tramp_exit_status $?
18:26:07.839830 tramp-wait-for-regexp (6) # 
(("‘/home/albinus/.tramp_history’") 1 ("albinus" . 1000) ("albinus" . 1000) 1716544239 1715267567 1715267567 20671 "-rw-------" t 10891213 -1 "unconfined_u:object_r:user_home_t:s0")
tramp_exit_status 0
///1ab1c259aca5ea2a0696680da9e7ac35#$
///1ab1c259aca5ea2a0696680da9e7ac35#$
18:26:07.955777 tramp-send-command (6) # (env GZIP= gzip </home/albinus/.tramp_history | base64) 2>/dev/null; echo tramp_exit_status $?
18:26:07.991862 tramp-wait-for-regexp (6) # 
H4sIAO/nPGYAA+1XbVPbRhD+rl+xPdTYnsSVCcEBC5JShwSmpGF4yTRTNUaWTljDWRLSCZMp8Nu7
...
tramp_exit_status 0
///1ab1c259aca5ea2a0696680da9e7ac35#$
--8<---------------cut here---------------end--------------->8---

6 roundtrips to insert the remote history file into a buffer which we
don't need. Just for a single asynchronous "ls" command.

With the new user option, this could be avoided by a user setting.

> Why is the process being called by such bogus names anyway?

I don't understand. Which bogus names?

Best regards, Michael.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: dmitry@gutov.dev,  71049@debbugs.gnu.org
> Date: Fri, 24 May 2024 18:39:21 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Can you explain the effect of that option on the scenarios that
> > started this bug report?  I don't think I have a clear understanding
> > of that.
> 
> We're speaking about shell-mode. Let's try the command
> [...]
> 6 roundtrips to insert the remote history file into a buffer which we
> don't need. Just for a single asynchronous "ls" command.
> 
> With the new user option, this could be avoided by a user setting.

Thanks.  But that's not what I thought I was asking about, see below.

However, as long as we are talking about reading the history file: why
does async-shell-command need the history file?  (I understand why
shell-mode does, but async-shell-command is not shell-mode.)

> > Why is the process being called by such bogus names anyway?
> 
> I don't understand. Which bogus names?

I thought this was about the original complaints, whtch started this
bug report, see https://debbugs.gnu.org/cgi/bugreport.cgi?bug=71049#5.
The fact that the history file was being read sounded as a side issue,
at least at first.  So my question was about these messages:

  Process *Async Shell Command* finished
  -l: finished.

I thought the option you suggest is intended to make these "process
names" be more reasonable.  I guess I am confused, and the discussion
moved to the "side issue" of preventing the unnecessary reading of the
history file?

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Dmitry Gutov]

On 24/05/2024 21:55, Eli Zaretskii wrote:
>> From: Michael Albinus <michael.albinus@gmx.de>
>> Cc: dmitry@gutov.dev,  71049@debbugs.gnu.org
>> Date: Fri, 24 May 2024 18:39:21 +0200
>>
>> Eli Zaretskii <eliz@gnu.org> writes:
>>
>>> Can you explain the effect of that option on the scenarios that
>>> started this bug report?  I don't think I have a clear understanding
>>> of that.
>>
>> We're speaking about shell-mode. Let's try the command
>> [...]
>> 6 roundtrips to insert the remote history file into a buffer which we
>> don't need. Just for a single asynchronous "ls" command.
>>
>> With the new user option, this could be avoided by a user setting.
> 
> Thanks.  But that's not what I thought I was asking about, see below.
> 
> However, as long as we are talking about reading the history file: why
> does async-shell-command need the history file?  (I understand why
> shell-mode does, but async-shell-command is not shell-mode.)

The answer is that async-shell-command uses shell-mode as the major mode 
for the output buffer. For syntax highlighting, I guess.

You make a good point that the shell history for such buffers would 
usually make no sense - even if the running process takes user input 
(usually not, but sometimes it might) - its input history would be 
different from the shell.

So maybe we could just move the last form in shell-mode (which 
initializes comint-input-ring) to 'shell'

>>> Why is the process being called by such bogus names anyway?
>>
>> I don't understand. Which bogus names?
> 
> I thought this was about the original complaints, whtch started this
> bug report, see https://debbugs.gnu.org/cgi/bugreport.cgi?bug=71049#5.
> The fact that the history file was being read sounded as a side issue,
> at least at first.  So my question was about these messages:
> 
>    Process *Async Shell Command* finished
>    -l: finished.
> 
> I thought the option you suggest is intended to make these "process
> names" be more reasonable.  I guess I am confused, and the discussion
> moved to the "side issue" of preventing the unnecessary reading of the
> history file?

These are two separate (but correlated) issues in one bug report.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Dmitry Gutov <dmitry@gutov.dev> writes:

Hi Dmitry,

> The answer is that async-shell-command uses shell-mode as the major
> mode for the output buffer. For syntax highlighting, I guess.
>
> You make a good point that the shell history for such buffers would
> usually make no sense - even if the running process takes user input
> (usually not, but sometimes it might) - its input history would be
> different from the shell.
>
> So maybe we could just move the last form in shell-mode (which
> initializes comint-input-ring) to 'shell'

Don't know. (shell-mode) is called in shell-command since bcad49851742
(1995-07-17). And it is called in tramp-handle-shell-command since
f5e29b9b70a5 (2011-09-04).

(comint-read-input-ring) is called in shell-mode since
(1993-10-22). There might be packages which trust on the
comint-input-ring existence for buffers in shell mode, even if such
buffers are created by shell-command..

Best regards, Michael.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Dmitry Gutov]

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

Hi Michael,

On 25/05/2024 13:49, Michael Albinus wrote:

>> The answer is that async-shell-command uses shell-mode as the major
>> mode for the output buffer. For syntax highlighting, I guess.
>>
>> You make a good point that the shell history for such buffers would
>> usually make no sense - even if the running process takes user input
>> (usually not, but sometimes it might) - its input history would be
>> different from the shell.
>>
>> So maybe we could just move the last form in shell-mode (which
>> initializes comint-input-ring) to 'shell'
> Don't know. (shell-mode) is called in shell-command since bcad49851742
> (1995-07-17). And it is called in tramp-handle-shell-command since
> f5e29b9b70a5 (2011-09-04).
> 
> (comint-read-input-ring) is called in shell-mode since
> (1993-10-22). There might be packages which trust on the
> comint-input-ring existence for buffers in shell mode, even if such
> buffers are created by shell-command..

Yes, it would be an incompatibility - so we'll need to consider the 
migration path. See the attached patch - I suggest that any callers of 
'shell-mode' that need the exact same input-ring setup also call 
shell-setup-input-ring (if it's fboundp - a version check).

Or I suppose we could check the value of shell-setup-input-ring and skip 
history loading when it is empty. It's a more subtle incompatibility 
which might affect (or not) third-party code in similar ways.

Either of the attached patches solves this part of the problem for me, 
please take your pick.

[-- Attachment #2: shell-setup-input-ring.diff --]
[-- Type: text/x-patch, Size: 964 bytes --]

diff --git a/lisp/shell.el b/lisp/shell.el
index e6b315ee5c0..d485c8af567 100644
--- a/lisp/shell.el
+++ b/lisp/shell.el
@@ -718,7 +718,10 @@ shell-mode
 
   ;; This is not really correct, since the shell buffer does not really
   ;; edit this directory.  But it is useful in the buffer list and menus.
-  (setq list-buffers-directory (expand-file-name default-directory))
+  (setq list-buffers-directory (expand-file-name default-directory)))
+
+(defun shell-setup-input-ring ()
+  "Set up `comint-input-ring' for this buffer, for the chosen shell."
   ;; shell-dependent assignments.
   (when (ring-empty-p comint-input-ring)
     (let ((remote (file-remote-p default-directory))
@@ -948,6 +951,7 @@ shell
                   (symbol-value xargs-name)
                 '("-i")))
        (shell-mode)
+       (shell-setup-input-ring)
        (when (file-exists-p startfile)
          ;; Wait until the prompt has appeared.
          (while (= start-point (point))

[-- Attachment #3: shell-no-start-prog.diff --]
[-- Type: text/x-patch, Size: 690 bytes --]

diff --git a/lisp/shell.el b/lisp/shell.el
index e6b315ee5c0..ca02d602966 100644
--- a/lisp/shell.el
+++ b/lisp/shell.el
@@ -720,9 +720,10 @@ shell-mode
   ;; edit this directory.  But it is useful in the buffer list and menus.
   (setq list-buffers-directory (expand-file-name default-directory))
   ;; shell-dependent assignments.
-  (when (ring-empty-p comint-input-ring)
+  (when (and (ring-empty-p comint-input-ring)
+             shell--start-prog)
     (let ((remote (file-remote-p default-directory))
-          (shell (or shell--start-prog ""))
+          (shell shell--start-prog)
           (hsize (getenv "HISTSIZE"))
           (hfile (getenv "HISTFILE")))
       (when remote

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Dmitry Gutov <dmitry@gutov.dev> writes:

> Hi Michael,

Hi Dmitry,

>>> The answer is that async-shell-command uses shell-mode as the major
>>> mode for the output buffer. For syntax highlighting, I guess.
>>>
>>> You make a good point that the shell history for such buffers would
>>> usually make no sense - even if the running process takes user input
>>> (usually not, but sometimes it might) - its input history would be
>>> different from the shell.
>>>
>>> So maybe we could just move the last form in shell-mode (which
>>> initializes comint-input-ring) to 'shell'
>> Don't know. (shell-mode) is called in shell-command since bcad49851742
>> (1995-07-17). And it is called in tramp-handle-shell-command since
>> f5e29b9b70a5 (2011-09-04).
>> (comint-read-input-ring) is called in shell-mode since
>> (1993-10-22). There might be packages which trust on the
>> comint-input-ring existence for buffers in shell mode, even if such
>> buffers are created by shell-command..
>
> Yes, it would be an incompatibility - so we'll need to consider the
> migration path. See the attached patch - I suggest that any callers of
> 'shell-mode' that need the exact same input-ring setup also call
> shell-setup-input-ring (if it's fboundp - a version check).
>
> Or I suppose we could check the value of shell-setup-input-ring and
> skip history loading when it is empty. It's a more subtle
> incompatibility which might affect (or not) third-party code in
> similar ways.
>
> Either of the attached patches solves this part of the problem for me,
> please take your pick.

I'm really not convinced that we should change shell-mode in an
incompatible way for such a minor problem of (not) loading the remote
history file. shell-mode is one of the building blocks Emnacs consists of.

Instead we must give the user a config option to suppress this. I've
shown a possible way to do with my patch, but anything else, which keeps
compatibility of shell-mode, would do.

Eli?

Best regards, Michael.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Dmitry Gutov]

Hi Michael,

On 25/05/2024 18:51, Michael Albinus wrote:

>> Either of the attached patches solves this part of the problem for me,
>> please take your pick.
> 
> I'm really not convinced that we should change shell-mode in an
> incompatible way for such a minor problem of (not) loading the remote
> history file. shell-mode is one of the building blocks Emnacs consists of.

Right. But from what I see, shell-mode has a problem, which normally 
results in some extra work behind the scenes, but becomes more 
noticeable when working remotely.

Moving code from a major mode to a command that invokes it is often a 
good thing, so that's the route I took.

> Instead we must give the user a config option to suppress this. I've
> shown a possible way to do with my patch, but anything else, which keeps
> compatibility of shell-mode, would do.

It's not a huge issue for me personally, but then we either switch the 
new option on and regress on bug#36742 by default, or have it off 
(keeping the performance problem there by default), and only regress on 
bug#36742 when the user turns on the option.

Either of my patches would let the user avoid having to choose between 
functionality and performance. Though I suppose some (many?) would 
prefer to disable the remote history ring altogether, for extra performance.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Dmitry Gutov <dmitry@gutov.dev> writes:

> Hi Michael,

Hi Dmitry,

> Either of my patches would let the user avoid having to choose between
> functionality and performance. Though I suppose some (many?) would
> prefer to disable the remote history ring altogether, for extra
> performance.

I'll have another idea.

shell-mode checks already, whether the histfile is equal
null-device. That works for the local case only.

However, in Tramp there is tramp-histfile-override. It let's you decide
whether you want to use the remote histfile, or use a histfile given by
a file name, or don't use a histfile.

This works for normal sync and async processes, but not for direct async
processes (IIRC). Let's see whether I could extend this mechanism, and
provide shell-mode proper information.

If a user sets tramp-histfile-override to t or "/dev/null", shell-mode
should not read the remote histfile. Users are already conditioned to
set this user option :-)

WDYT? I'll work on this tomorrow.

Best regards, Michael.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Dmitry Gutov]

On 25/05/2024 20:00, Michael Albinus wrote:

> I'll have another idea.
> 
> shell-mode checks already, whether the histfile is equal
> null-device. That works for the local case only.
> 
> However, in Tramp there is tramp-histfile-override. It let's you decide
> whether you want to use the remote histfile, or use a histfile given by
> a file name, or don't use a histfile.
> 
> This works for normal sync and async processes, but not for direct async
> processes (IIRC). Let's see whether I could extend this mechanism, and
> provide shell-mode proper information.

Obeying tramp-histfile-override sounds like it will be an improvement, 
thank you.

> If a user sets tramp-histfile-override to t or "/dev/null", shell-mode
> should not read the remote histfile. Users are already conditioned to
> set this user option :-)

But when used it would still regress on bug#36742, right? I'm not 100% 
sure what most of the users want in this area, but it appears as if at 
least one wanted to have access to command history.

And unlike when using async-shell-command, users of 'M-x shell' probably 
can afford to wait a little longer for the history to load (in all 
likelihood the new buffer will live longer, over multiple user inputs).

So it seems to me that fixing shell-mode would be good for the default 
behavior, and then one could use tramp-histfile-override to add extra 
performance on top.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Dmitry Gutov <dmitry@gutov.dev> writes:

Hi Dmitry,

> So it seems to me that fixing shell-mode would be good for the default
> behavior, and then one could use tramp-histfile-override to add extra
> performance on top.

I'll see whether I could make it more fine-grained, for example by
distinguishing the shell and shell-command cases.

Best regards, Michael.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Michael Albinus <michael.albinus@gmx.de> writes:

Hi Dmitry,
>
>> So it seems to me that fixing shell-mode would be good for the default
>> behavior, and then one could use tramp-histfile-override to add extra
>> performance on top.
>
> I'll see whether I could make it more fine-grained, for example by
> distinguishing the shell and shell-command cases.

I've puzzled the appended patch together. It does the following:

- Obey 'tramp-histfile-override' also for direct async processes.

- Use 'tramp-histfile-override' in 'shell-mode', whether the remote
  history file shall be read. A value of t suppresses this.

- Support connection-local setting of 'tramp-histfile-override' in
  'shell'. Use something like

--8<---------------cut here---------------start------------->8---
  (connection-local-set-profile-variables
   'remote-tramp-histfile-override '((tramp-histfile-override . nil)))

  (connection-local-set-profiles
   '(:application tramp :machine "remotehost")
   'remote-tramp-histfile-override)
--8<---------------cut here---------------end--------------->8---

- Support connection-local setting of 'tramp-histfile-override' in
  'shell-command'. In order to distinguish this from the setting for
  'shell', another :application is used ('shell-command' instead of
  'tramp'). Use something like

--8<---------------cut here---------------start------------->8---
  (connection-local-set-profile-variables
   'another-tramp-histfile-override '((tramp-histfile-override . t)))

  (connection-local-set-profiles
   '(:application shell-command :machine "remotehost")
   'another-tramp-histfile-override)
--8<---------------cut here---------------end--------------->8---

It is recommended to set 'tramp-histfile-override' to t for
asynchronous processes. Comments?

Best regards, Michael.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 4170 bytes --]

diff --git a/lisp/net/tramp-sh.el b/lisp/net/tramp-sh.el
index c079455a444..615f9219448 100644
--- a/lisp/net/tramp-sh.el
+++ b/lisp/net/tramp-sh.el
@@ -64,6 +64,7 @@ tramp-copy-size-limit
   :group 'tramp
   :type '(choice (const nil) integer))

+;;;###tramp-autoload
 (defcustom tramp-histfile-override "~/.tramp_history"
   "When invoking a shell, override the HISTFILE with this value.
 When setting to a string, it redirects the shell history to that
@@ -80,6 +81,8 @@ tramp-histfile-override
                  (const :tag "Unset HISTFILE" t)
                  (string :tag "Redirect to a file")))

+(put 'tramp-histfile-override 'permanent-local t)
+
 ;; ksh on OpenBSD 4.5 requires that $PS1 contains a `#' character for
 ;; root users.  It uses the `$' character for other users.  In order
 ;; to guarantee a proper prompt, we use "#$ " for the prompt.
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 9385b023392..936fdd25e6c 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -4962,6 +4962,18 @@ tramp-handle-make-process
 			      (string-join (tramp-get-remote-path v) ":")))
 			(setenv-internal env "PATH" remote-path 'keep)
 		      env))
+	       ;; Add HISTFILE if indicated.
+	       (env (if-let ((sh-file-name-handler-p))
+			(cond
+			 ((stringp tramp-histfile-override)
+			  (setenv-internal env "HISTFILE" tramp-histfile-override 'keep))
+			 (tramp-histfile-override
+			  (setq env (setenv-internal env "HISTFILE" "''" 'keep))
+			  (setq env (setenv-internal env "HISTSIZE" "0" 'keep))
+			  (setenv-internal env "HISTFILESIZE" "0" 'keep))
+			 (t env))
+		      env))
+	       ;; Add INSIDE_EMACS.
 	       (env (setenv-internal
 		     env "INSIDE_EMACS" (tramp-inside-emacs) 'keep))
 	       (env (mapcar #'tramp-shell-quote-argument (delq nil env)))
@@ -5248,7 +5260,8 @@ tramp-handle-shell-command
 	      (with-current-buffer output-buffer
 		(setq mode-line-process '(":%s"))
 		(unless (eq major-mode 'shell-mode)
-		  (shell-mode))
+		  (with-connection-local-application-variables 'shell-command
+		    (shell-mode)))
 		(set-process-filter p #'comint-output-filter)
 		(set-process-sentinel p #'shell-command-sentinel)
 		(when error-file
diff --git a/lisp/shell.el b/lisp/shell.el
index e6b315ee5c0..60a805f33c9 100644
--- a/lisp/shell.el
+++ b/lisp/shell.el
@@ -609,6 +609,8 @@ sh-shell-file
 (declare-function w32-application-type "w32proc.c"
                   (program) t)

+(defvar tramp-histfile-override)
+
 (define-derived-mode shell-mode comint-mode "Shell"
   "Major mode for interacting with an inferior shell.
 \\<shell-mode-map>
@@ -726,9 +728,11 @@ shell-mode
           (hsize (getenv "HISTSIZE"))
           (hfile (getenv "HISTFILE")))
       (when remote
-        ;; `shell-snarf-envar' does not work trustworthy.
-        (setq hsize (shell-command-to-string "echo -n $HISTSIZE")
-              hfile (shell-command-to-string "echo -n $HISTFILE")))
+        (if (eq tramp-histfile-override t)
+            (setq remote "" hfile nil hsize nil)
+          ;; `shell-snarf-envar' does not work trustworthy.
+          (setq hsize (shell-command-to-string "echo -n $HISTSIZE")
+                hfile (shell-command-to-string "echo -n $HISTFILE"))))
       (and (string-equal hfile "") (setq hfile nil))
       (and (stringp hsize)
 	   (integerp (setq hsize (string-to-number hsize)))
@@ -738,10 +742,11 @@ shell-mode
             (concat
              remote
 	     (or hfile
-		 (cond ((string-equal shell "bash") "~/.bash_history")
-		       ((string-equal shell "ksh") "~/.sh_history")
-		       ((string-equal shell "zsh") "~/.zsh_history")
-		       (t "~/.history")))))
+                 (and (not (string-equal remote ""))
+		      (cond ((string-equal shell "bash") "~/.bash_history")
+		            ((string-equal shell "ksh") "~/.sh_history")
+		            ((string-equal shell "zsh") "~/.zsh_history")
+		            (t "~/.history"))))))
       (if (or (equal comint-input-ring-file-name "")
 	      (equal (file-truename comint-input-ring-file-name)
 		     (file-truename null-device)))

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Dmitry Gutov]

Hi Michael,

On 26/05/2024 17:18, Michael Albinus wrote:

>>> So it seems to me that fixing shell-mode would be good for the default
>>> behavior, and then one could use tramp-histfile-override to add extra
>>> performance on top.
>>
>> I'll see whether I could make it more fine-grained, for example by
>> distinguishing the shell and shell-command cases.
> 
> I've puzzled the appended patch together. It does the following:
> 
> - Obey 'tramp-histfile-override' also for direct async processes.

Thank you.

> - Use 'tramp-histfile-override' in 'shell-mode', whether the remote
>    history file shall be read. A value of t suppresses this.
> 
> - Support connection-local setting of 'tramp-histfile-override' in
>    'shell'. Use something like
> 
> --8<---------------cut here---------------start------------->8---
>    (connection-local-set-profile-variables
>     'remote-tramp-histfile-override '((tramp-histfile-override . nil)))
> 
>    (connection-local-set-profiles
>     '(:application tramp :machine "remotehost")
>     'remote-tramp-histfile-override)
> --8<---------------cut here---------------end--------------->8---
> 
> - Support connection-local setting of 'tramp-histfile-override' in
>    'shell-command'. In order to distinguish this from the setting for
>    'shell', another :application is used ('shell-command' instead of
>    'tramp'). Use something like
> 
> --8<---------------cut here---------------start------------->8---
>    (connection-local-set-profile-variables
>     'another-tramp-histfile-override '((tramp-histfile-override . t)))
> 
>    (connection-local-set-profiles
>     '(:application shell-command :machine "remotehost")
>     'another-tramp-histfile-override)
> --8<---------------cut here---------------end--------------->8---
> 
> It is recommended to set 'tramp-histfile-override' to t for
> asynchronous processes. Comments?

It seems like more work, and more code, to get to the same result. Also, 
it would not get the OOtB improvement for the "not M-x shell" case - 
IIUC the user would have to create a new profile to enact the 
distinction. That's a relatively complex thing to do.

So... it's not up to me, and the problem doesn't touch me too deeply, 
but I think my solution for the second part is preferable.

Maybe Eli will want to make that choice now.

Thanks,
Dmitry

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Dmitry Gutov <dmitry@gutov.dev> writes:

> Hi Michael,

Hi Dmitry,

>> I've puzzled the appended patch together. It does the following:
>> - Obey 'tramp-histfile-override' also for direct async processes.
>
> Thank you.

So that's consensus.

>> - Use 'tramp-histfile-override' in 'shell-mode', whether the remote
>>    history file shall be read. A value of t suppresses this.
>> - Support connection-local setting of 'tramp-histfile-override' in
>>    'shell'. Use something like
>> --8<---------------cut here---------------start------------->8---
>>    (connection-local-set-profile-variables
>>     'remote-tramp-histfile-override '((tramp-histfile-override . nil)))
>>    (connection-local-set-profiles
>>     '(:application tramp :machine "remotehost")
>>     'remote-tramp-histfile-override)
>> --8<---------------cut here---------------end--------------->8---
>> - Support connection-local setting of 'tramp-histfile-override' in
>>    'shell-command'. In order to distinguish this from the setting for
>>    'shell', another :application is used ('shell-command' instead of
>>    'tramp'). Use something like
>> --8<---------------cut here---------------start------------->8---
>>    (connection-local-set-profile-variables
>>     'another-tramp-histfile-override '((tramp-histfile-override . t)))
>>    (connection-local-set-profiles
>>     '(:application shell-command :machine "remotehost")
>>     'another-tramp-histfile-override)
>> --8<---------------cut here---------------end--------------->8---
>> It is recommended to set 'tramp-histfile-override' to t for
>> asynchronous processes. Comments?
>
> It seems like more work, and more code, to get to the same
> result.

For whom? The changes in Emacs are small.

A user doesn't need connection-local variables. Only in case, she wants
different settings for different applications, like shell and
shell-command. Otherwise, a global setting of tramp-histfile-override
would be sufficient.

> Also, it would not get the OOtB improvement for the "not M-x
> shell" case - IIUC the user would have to create a new profile to
> enact the distinction. That's a relatively complex thing to do.

No. Only if *different* values for *different* applications are
needed. And the other places in Emacs, which use shell-mode, wouldn't
profit (yet) from connection-local variables, but they would profit from
tramp-histfile-override setting in general. This is an improvement to
the status quo.

If we believe that tramp-histfile-override shall be set to t for remote
shell-command by default, I could add this. But this isn't my decision.

> So... it's not up to me, and the problem doesn't touch me too deeply,
> but I think my solution for the second part is preferable.

You mean shell-no-start-prog.diff? That hard-codes a different behavior
in shell-mode, and I'm not so familiar with shell-mode and comint that I
can exclude collateral damages.

> Maybe Eli will want to make that choice now.

Let's see.

> Thanks,
> Dmitry

Best regards, Michael.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Dmitry Gutov]

Hi Michael,

On 29/05/2024 10:41, Michael Albinus wrote:

>>> I've puzzled the appended patch together. It does the following:
>>> - Obey 'tramp-histfile-override' also for direct async processes.
>>
>> Thank you.
> 
> So that's consensus.

Yes, of course.

>>> - Use 'tramp-histfile-override' in 'shell-mode', whether the remote
>>>     history file shall be read. A value of t suppresses this.
>>> - Support connection-local setting of 'tramp-histfile-override' in
>>>     'shell'. Use something like
>>> --8<---------------cut here---------------start------------->8---
>>>     (connection-local-set-profile-variables
>>>      'remote-tramp-histfile-override '((tramp-histfile-override . nil)))
>>>     (connection-local-set-profiles
>>>      '(:application tramp :machine "remotehost")
>>>      'remote-tramp-histfile-override)
>>> --8<---------------cut here---------------end--------------->8---
>>> - Support connection-local setting of 'tramp-histfile-override' in
>>>     'shell-command'. In order to distinguish this from the setting for
>>>     'shell', another :application is used ('shell-command' instead of
>>>     'tramp'). Use something like
>>> --8<---------------cut here---------------start------------->8---
>>>     (connection-local-set-profile-variables
>>>      'another-tramp-histfile-override '((tramp-histfile-override . t)))
>>>     (connection-local-set-profiles
>>>      '(:application shell-command :machine "remotehost")
>>>      'another-tramp-histfile-override)
>>> --8<---------------cut here---------------end--------------->8---
>>> It is recommended to set 'tramp-histfile-override' to t for
>>> asynchronous processes. Comments?
>>
>> It seems like more work, and more code, to get to the same
>> result.
> 
> For whom? The changes in Emacs are small.

Bigger than in my patches anyway. And the user's customization is an 
extra piece that people fill have to figure out as well.

> A user doesn't need connection-local variables. Only in case, she wants
> different settings for different applications, like shell and
> shell-command. Otherwise, a global setting of tramp-histfile-override
> would be sufficient.

I think the original report and our consensus that the behavior is wrong 
more or less concludes that shell and shell-command should behave 
differently in this regard (by default). Does it not?

>> Also, it would not get the OOtB improvement for the "not M-x
>> shell" case - IIUC the user would have to create a new profile to
>> enact the distinction. That's a relatively complex thing to do.
> 
> No. Only if *different* values for *different* applications are
> needed. And the other places in Emacs, which use shell-mode, wouldn't
> profit (yet) from connection-local variables, but they would profit from
> tramp-histfile-override setting in general. This is an improvement to
> the status quo.

It's indeed an improvement, but it applied to my patches as well, except 
they don't require an additional customization for this to happen.

>> So... it's not up to me, and the problem doesn't touch me too deeply,
>> but I think my solution for the second part is preferable.
> 
> You mean shell-no-start-prog.diff? That hard-codes a different behavior
> in shell-mode, and I'm not so familiar with shell-mode and comint that I
> can exclude collateral damages.

Right. The potential for collateral seems very low to me, and there is a 
migration path for such callers anyway. But let's see what others think.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Dmitry Gutov <dmitry@gutov.dev> writes:

> Hi Michael,

Hi Dmitry,

>>>> - Obey 'tramp-histfile-override' also for direct async processes.
>>>
>>> Thank you.
>> So that's consensus.
>
> Yes, of course.

I've pushed this part of the patch to the repositories.

Best regards, Michael.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Eli Zaretskii]

> Date: Wed, 29 May 2024 04:59:43 +0300
> Cc: Eli Zaretskii <eliz@gnu.org>, 71049@debbugs.gnu.org
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> So... it's not up to me, and the problem doesn't touch me too deeply, 
> but I think my solution for the second part is preferable.
> 
> Maybe Eli will want to make that choice now.

I don't think I understand what you mean by "your solution".  Was
there a patch somewhere that I missed?

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Dmitry Gutov]

On 29/05/2024 14:53, Eli Zaretskii wrote:
>> Date: Wed, 29 May 2024 04:59:43 +0300
>> Cc: Eli Zaretskii<eliz@gnu.org>,71049@debbugs.gnu.org
>> From: Dmitry Gutov<dmitry@gutov.dev>
>>
>> So... it's not up to me, and the problem doesn't touch me too deeply,
>> but I think my solution for the second part is preferable.
>>
>> Maybe Eli will want to make that choice now.
> I don't think I understand what you mean by "your solution".  Was
> there a patch somewhere that I missed?

Seems so. The patches (two alternatives) are attached to 
https://debbugs.gnu.org/71049#53.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Eli Zaretskii]

> Date: Wed, 29 May 2024 14:57:08 +0300
> Cc: michael.albinus@gmx.de, 71049@debbugs.gnu.org
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> On 29/05/2024 14:53, Eli Zaretskii wrote:
> >> Date: Wed, 29 May 2024 04:59:43 +0300
> >> Cc: Eli Zaretskii<eliz@gnu.org>,71049@debbugs.gnu.org
> >> From: Dmitry Gutov<dmitry@gutov.dev>
> >>
> >> So... it's not up to me, and the problem doesn't touch me too deeply,
> >> but I think my solution for the second part is preferable.
> >>
> >> Maybe Eli will want to make that choice now.
> > I don't think I understand what you mean by "your solution".  Was
> > there a patch somewhere that I missed?
> 
> Seems so. The patches (two alternatives) are attached to 
> https://debbugs.gnu.org/71049#53.

Thanks.  I don't understand what they do in the context of this
discussion, probably because I don't know enough about shell-mode and
comint.  Could you explain the intent, please?

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Dmitry Gutov]

On 29/05/2024 15:46, Eli Zaretskii wrote:
>> Seems so. The patches (two alternatives) are attached to
>> https://debbugs.gnu.org/71049#53.
> Thanks.  I don't understand what they do in the context of this
> discussion, probably because I don't know enough about shell-mode and
> comint.  Could you explain the intent, please?

Okay, to summarize the previous messages:

async-shell-command calls shell-mode, which loads the history file by 
calling comint-read-input-ring.

shell-setup-input-ring.diff extracts the comint input ring setup to a 
separate function. That function is then only called by 'shell', but not 
by 'shell-mode' itself anymore, or any of its descendants. Any callers 
or descendants of it that actually need this setup (I'm not aware of 
any, but they might exist) will need to adapt by adding this call.

shell-no-start-prog.diff does not move the logic, but predicates its 
execution on the value of the variable shell--start-prog being non-nil. 
In-tree it's only set by 'shell', so the result is the same. The "how to 
adapt" recipe would be setting shell--start-prog to non-nil (in callers).

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Dmitry Gutov <dmitry@gutov.dev> writes:

> Okay, to summarize the previous messages:

Thanks. For Eli, I summarize the essential of my proposal.

> async-shell-command calls shell-mode, which loads the history file by
> calling comint-read-input-ring.
>
> shell-setup-input-ring.diff extracts the comint input ring setup to a
> separate function. That function is then only called by 'shell', but
> not by 'shell-mode' itself anymore, or any of its descendants. Any
> callers or descendants of it that actually need this setup (I'm not
> aware of any, but they might exist) will need to adapt by adding this
> call.
>
> shell-no-start-prog.diff does not move the logic, but predicates its
> execution on the value of the variable shell--start-prog being
> non-nil. In-tree it's only set by 'shell', so the result is the
> same. The "how to adapt" recipe would be setting shell--start-prog to
> non-nil (in callers).

shell-mode is changed to obey the existing tramp-histfile-override in
the remote case, which tells whether to use the default history file,
another history file (you can give the file name), or to suppress the
history file at all. Like many Tramp methods, you can customize it via
connection-local variables. I made also the proposal to suppress reading
the remote history file for shell-command by default (performance!);
every user can change the config of course.

Best regards, Michael.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Dmitry Gutov]

On 29/05/2024 20:42, Michael Albinus wrote:
> I made also the proposal to suppress reading
> the remote history file for shell-command by default (performance!)

How is that going to look, code-wise?

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Dmitry Gutov <dmitry@gutov.dev> writes:

> On 29/05/2024 20:42, Michael Albinus wrote:
>> I made also the proposal to suppress reading
>> the remote history file for shell-command by default (performance!)
>
> How is that going to look, code-wise?

Not tested yet:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 925 bytes --]

diff --git a/lisp/tramp.el b/lisp/tramp.el
index b2442f45..ba852cf6 100644
--- a/lisp/tramp.el
+++ b/lisp/tramp.el
@@ -5260,7 +5260,17 @@ support symbolic links."
 	      (with-current-buffer output-buffer
 		(setq mode-line-process '(":%s"))
 		(unless (eq major-mode 'shell-mode)
-		  (shell-mode))
+		  ;; We set `tramp-histfile-override' to t in order to
+		  ;; suppress reading the remote history file by
+		  ;; default.  Could be overridden by the user with a
+		  ;; connection-local setting.
+		  (let ((tramp-histfile-override t))
+		    ;; `with-connection-local-application-variables'
+		    ;; exists since Emacs 29.  Older Emacsen will use
+		    ;; default application `tramp'.
+		    (tramp-compat-with-connection-local-application-variables
+			'shell-command
+		      (shell-mode))))
 		(set-process-filter p #'comint-output-filter)
 		(set-process-sentinel p #'shell-command-sentinel)
 		(when error-file

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Dmitry Gutov]

On 29/05/2024 21:38, Michael Albinus wrote:
> Dmitry Gutov<dmitry@gutov.dev>  writes:
> 
>> On 29/05/2024 20:42, Michael Albinus wrote:
>>> I made also the proposal to suppress reading
>>> the remote history file for shell-command by default (performance!)
>> How is that going to look, code-wise?
> Not tested yet:
> 
> 
> diff --git a/lisp/tramp.el b/lisp/tramp.el
> index b2442f45..ba852cf6 100644
> --- a/lisp/tramp.el
> +++ b/lisp/tramp.el
> @@ -5260,7 +5260,17 @@ support symbolic links."
>   	      (with-current-buffer output-buffer
>   		(setq mode-line-process '(":%s"))
>   		(unless (eq major-mode 'shell-mode)
> -		  (shell-mode))
> +		  ;; We set `tramp-histfile-override' to t in order to
> +		  ;; suppress reading the remote history file by
> +		  ;; default.  Could be overridden by the user with a
> +		  ;; connection-local setting.
> +		  (let ((tramp-histfile-override t))
> +		    ;; `with-connection-local-application-variables'
> +		    ;; exists since Emacs 29.  Older Emacsen will use
> +		    ;; default application `tramp'.
> +		    (tramp-compat-with-connection-local-application-variables
> +			'shell-command
> +		      (shell-mode))))
>   		(set-process-filter p #'comint-output-filter)
>   		(set-process-sentinel p #'shell-command-sentinel)
>   		(when error-file

Thank you. I haven't tested this personally either, but it seems like 
this is the behavior we will want by default.

So the proposed solution should include it, I think.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Dmitry Gutov <dmitry@gutov.dev> writes:

Hi Dmitry,

> Thank you. I haven't tested this personally either, but it seems like
> this is the behavior we will want by default.
>
> So the proposed solution should include it, I think.

In order to not misunderstand: you agree the solution based on
tramp-histfile-override now, don't you?

Best regards, Michael.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Dmitry Gutov]

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

Hi Michael,

On 30/05/2024 11:49, Michael Albinus wrote:

>> Thank you. I haven't tested this personally either, but it seems like
>> this is the behavior we will want by default.
>>
>> So the proposed solution should include it, I think.
> In order to not misunderstand: you agree the solution based on
> tramp-histfile-override now, don't you?

Just to be frank: I don't like the approach, but I do think it's better 
than the status quo. In that sense, I agree with it, if none other is 
accepted.

But, well, in the meantime I came up with a couple of new solutions:

1) [first patch] We can add a new major mode, for 'M-&' to use instead 
of the full-blown 'shell-mode' - it could be very simple: just apply 
font-lock keywords and maybe set list-buffers-directory.

Problems? I suppose someone might be using shell-mode-hook to do 
something in the async-shell-command output buffer, and it won't fire 
anymore. Seemingly very minor concern.

2) [second patch] async-shell-command could set shell--start-prog (it's 
permanent-local) to `none', and then shell-mode will check for that 
value, and if set to this value, skip the input ring setup.

Downsides? Pretty ad-hoc approach. And any external code looking up this 
(private) variable could be surprised by the new value.

[-- Attachment #2: shell-command-mode.diff --]
[-- Type: text/x-patch, Size: 2105 bytes --]

diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 9385b023392..f0c64a7a90f 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -5247,8 +5247,8 @@ tramp-handle-shell-command
 	      ;; Display output.
 	      (with-current-buffer output-buffer
 		(setq mode-line-process '(":%s"))
-		(unless (eq major-mode 'shell-mode)
-		  (shell-mode))
+		(unless (eq major-mode 'shell-command-mode)
+		  (shell-command-mode))
 		(set-process-filter p #'comint-output-filter)
 		(set-process-sentinel p #'shell-command-sentinel)
 		(when error-file
diff --git a/lisp/shell.el b/lisp/shell.el
index e6b315ee5c0..7fa84a37e83 100644
--- a/lisp/shell.el
+++ b/lisp/shell.el
@@ -838,6 +838,13 @@ shell-write-history-on-exit
       (with-current-buffer buf
         (insert (format "\nProcess %s %s\n" process event))))))
 
+(define-derived-mode shell-command-mode comint-mode "Shell"
+  "Major mode for the output of `async-shell-command'."
+  (setq-local font-lock-defaults '(shell-font-lock-keywords t))
+  ;; See comments in `shell-mode'.
+  (setq-local ansi-color-apply-face-function #'shell-apply-ansi-color)
+  (setq list-buffers-directory (expand-file-name default-directory)))
+
 ;;;###autoload
 (defun shell (&optional buffer file-name)
   "Run an inferior shell, with I/O through BUFFER (which defaults to `*shell*').
diff --git a/lisp/simple.el b/lisp/simple.el
index ae8a824cb54..8618427f5c0 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -31,7 +31,7 @@
 (eval-when-compile (require 'cl-lib))
 
 (declare-function widget-convert "wid-edit" (type &rest args))
-(declare-function shell-mode "shell" ())
+(declare-function shell-command-mode "shell" ())
 
 ;;; From compile.el
 (defvar compilation-current-error)
@@ -4721,7 +4721,7 @@ shell-command
 		  (setq proc
 			(start-process-shell-command "Shell" buffer command)))
 		(setq mode-line-process '(":%s"))
-                (shell-mode)
+                (shell-command-mode)
                 (setq-local revert-buffer-function
                             (lambda (&rest _)
                               (async-shell-command command buffer)))

[-- Attachment #3: shell-start-prog-none.diff --]
[-- Type: text/x-patch, Size: 1763 bytes --]

diff --git a/lisp/shell.el b/lisp/shell.el
index e6b315ee5c0..07038832437 100644
--- a/lisp/shell.el
+++ b/lisp/shell.el
@@ -702,7 +702,7 @@ shell-mode
   (add-hook 'comint-indirect-setup-hook
             #'shell-indirect-setup-hook 'append t)
   (setq comint-indirect-setup-function
-        (let ((shell shell--start-prog))
+        (let ((shell (and (stringp shell--start-prog) shell--start-prog)))
           (lambda ()
             (require 'sh-script)
             (cl-letf
@@ -720,7 +720,8 @@ shell-mode
   ;; edit this directory.  But it is useful in the buffer list and menus.
   (setq list-buffers-directory (expand-file-name default-directory))
   ;; shell-dependent assignments.
-  (when (ring-empty-p comint-input-ring)
+  (when (and (ring-empty-p comint-input-ring)
+             (not (eq shell--start-prog 'none)))
     (let ((remote (file-remote-p default-directory))
           (shell (or shell--start-prog ""))
           (hsize (getenv "HISTSIZE"))
diff --git a/lisp/simple.el b/lisp/simple.el
index ae8a824cb54..9074f564ca1 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -4621,6 +4621,7 @@ shell-command
 			  (and filename (file-relative-name filename))))
     current-prefix-arg
     shell-command-default-error-buffer))
+  (defvar shell--start-prog)
   ;; Look for a handler in case default-directory is a remote file name.
   (let ((handler
 	 (find-file-name-handler (directory-file-name default-directory)
@@ -4721,6 +4722,7 @@ shell-command
 		  (setq proc
 			(start-process-shell-command "Shell" buffer command)))
 		(setq mode-line-process '(":%s"))
+                (setq shell--start-prog 'none)
                 (shell-mode)
                 (setq-local revert-buffer-function
                             (lambda (&rest _)

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Eli Zaretskii]

> Date: Fri, 31 May 2024 03:24:49 +0300
> Cc: Eli Zaretskii <eliz@gnu.org>, 71049@debbugs.gnu.org
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> >> Thank you. I haven't tested this personally either, but it seems like
> >> this is the behavior we will want by default.
> >>
> >> So the proposed solution should include it, I think.
> > In order to not misunderstand: you agree the solution based on
> > tramp-histfile-override now, don't you?
> 
> Just to be frank: I don't like the approach, but I do think it's better 
> than the status quo. In that sense, I agree with it, if none other is 
> accepted.
> 
> But, well, in the meantime I came up with a couple of new solutions:
> 
> 1) [first patch] We can add a new major mode, for 'M-&' to use instead 
> of the full-blown 'shell-mode' - it could be very simple: just apply 
> font-lock keywords and maybe set list-buffers-directory.

I think I'm okay with this.  (It needs to be prominently documented,
of course.)  But we need a documented way for people to get previous
behavior if they want that.  How would that work?

> Problems? I suppose someone might be using shell-mode-hook to do 
> something in the async-shell-command output buffer, and it won't fire 
> anymore. Seemingly very minor concern.

Can't we run shell-mode-hook inside this new mode's mode hook?  Then
this problem goes away.

> 2) [second patch] async-shell-command could set shell--start-prog (it's 
> permanent-local) to `none', and then shell-mode will check for that 
> value, and if set to this value, skip the input ring setup.
> 
> Downsides? Pretty ad-hoc approach. And any external code looking up this 
> (private) variable could be surprised by the new value.

Yes, which is why I like this one less.

Thanks.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Dmitry Gutov]

On 31/05/2024 08:53, Eli Zaretskii wrote:

>> 1) [first patch] We can add a new major mode, for 'M-&' to use instead
>> of the full-blown 'shell-mode' - it could be very simple: just apply
>> font-lock keywords and maybe set list-buffers-directory.
> 
> I think I'm okay with this.  (It needs to be prominently documented,
> of course.)

In NEWS?

> But we need a documented way for people to get previous
> behavior if they want that.  How would that work?

If we really needed the capability to rollback the change, I suppose we 
could add a defvar pointing to the major mode to use. I.e.

   (defvar async-shell-command-major-mode #'shell-command-mode)

>> Problems? I suppose someone might be using shell-mode-hook to do
>> something in the async-shell-command output buffer, and it won't fire
>> anymore. Seemingly very minor concern.
> 
> Can't we run shell-mode-hook inside this new mode's mode hook?  Then
> this problem goes away.

Doesn't seem worth it - it was just an offhand example I came up with. 
The syntax table and the keymap would also different, and some user, in 
theory, could depend on those as well.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Eli Zaretskii]

> Date: Fri, 31 May 2024 19:24:10 +0300
> Cc: michael.albinus@gmx.de, 71049@debbugs.gnu.org
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> On 31/05/2024 08:53, Eli Zaretskii wrote:
> 
> >> 1) [first patch] We can add a new major mode, for 'M-&' to use instead
> >> of the full-blown 'shell-mode' - it could be very simple: just apply
> >> font-lock keywords and maybe set list-buffers-directory.
> > 
> > I think I'm okay with this.  (It needs to be prominently documented,
> > of course.)
> 
> In NEWS?

There, but also in the doc string of M-&, and maybe also in the doc
string of shell-mode-hook.

> > But we need a documented way for people to get previous
> > behavior if they want that.  How would that work?
> 
> If we really needed the capability to rollback the change, I suppose we 
> could add a defvar pointing to the major mode to use. I.e.
> 
>    (defvar async-shell-command-major-mode #'shell-command-mode)

That could work, yes.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Dmitry Gutov]

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

On 31/05/2024 21:05, Eli Zaretskii wrote:
>> Date: Fri, 31 May 2024 19:24:10 +0300
>> Cc:michael.albinus@gmx.de,71049@debbugs.gnu.org
>> From: Dmitry Gutov<dmitry@gutov.dev>
>>
>> On 31/05/2024 08:53, Eli Zaretskii wrote:
>>
>>>> 1) [first patch] We can add a new major mode, for 'M-&' to use instead
>>>> of the full-blown 'shell-mode' - it could be very simple: just apply
>>>> font-lock keywords and maybe set list-buffers-directory.
>>> I think I'm okay with this.  (It needs to be prominently documented,
>>> of course.)
>> In NEWS?
> There, but also in the doc string of M-&, and maybe also in the doc
> string of shell-mode-hook.
> 
>>> But we need a documented way for people to get previous
>>> behavior if they want that.  How would that work?
>> If we really needed the capability to rollback the change, I suppose we
>> could add a defvar pointing to the major mode to use. I.e.
>>
>>     (defvar async-shell-command-major-mode #'shell-command-mode)
> That could work, yes.

How about this?

[-- Attachment #2: shell-command-mode.diff --]
[-- Type: text/x-patch, Size: 4686 bytes --]

diff --git a/etc/NEWS b/etc/NEWS
index d058acc3572..40407edf4e3 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1881,6 +1881,10 @@ than regular expressions, but less complexity than context-free
 grammars.  The Info manual "(elisp) Parsing Expression Grammars" has
 documentation and examples.
 
+** New major mode 'shell-command-mode'.
+This mode is used by default for the output of 'async-shell-command'.
+To revert to the previous behavior, set 'async-shell-command-mode' to
+'shell-mode'.
 \f
 * Incompatible Lisp Changes in Emacs 30.1
 
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 9385b023392..795a9e667c7 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -5247,8 +5247,13 @@ tramp-handle-shell-command
 	      ;; Display output.
 	      (with-current-buffer output-buffer
 		(setq mode-line-process '(":%s"))
-		(unless (eq major-mode 'shell-mode)
-		  (shell-mode))
+                (cond
+                 ((boundp 'async-shell-command-mode)
+                  ;; Emacs 30+
+                  (unless (eq major-mode async-shell-command-mode)
+                    (funcall async-shell-command-mode)))
+                 ((not (eq major-mode 'shell-mode))
+                  (shell-mode)))
 		(set-process-filter p #'comint-output-filter)
 		(set-process-sentinel p #'shell-command-sentinel)
 		(when error-file
diff --git a/lisp/shell.el b/lisp/shell.el
index e6b315ee5c0..edee46cdb4d 100644
--- a/lisp/shell.el
+++ b/lisp/shell.el
@@ -838,6 +838,13 @@ shell-write-history-on-exit
       (with-current-buffer buf
         (insert (format "\nProcess %s %s\n" process event))))))
 
+(define-derived-mode shell-command-mode comint-mode "Shell"
+  "Major mode for the output of asynchronous `shell-command'."
+  (setq-local font-lock-defaults '(shell-font-lock-keywords t))
+  ;; See comments in `shell-mode'.
+  (setq-local ansi-color-apply-face-function #'shell-apply-ansi-color)
+  (setq list-buffers-directory (expand-file-name default-directory)))
+
 ;;;###autoload
 (defun shell (&optional buffer file-name)
   "Run an inferior shell, with I/O through BUFFER (which defaults to `*shell*').
diff --git a/lisp/simple.el b/lisp/simple.el
index ae8a824cb54..fa745f10148 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -31,7 +31,6 @@
 (eval-when-compile (require 'cl-lib))
 
 (declare-function widget-convert "wid-edit" (type &rest args))
-(declare-function shell-mode "shell" ())
 
 ;;; From compile.el
 (defvar compilation-current-error)
@@ -4487,9 +4486,10 @@ async-shell-command
 
 The output appears in OUTPUT-BUFFER, which could be a buffer or
 the name of a buffer, and defaults to `shell-command-buffer-name-async'
-if nil or omitted.  That buffer is in shell mode.  Note that, unlike
-with `shell-command', OUTPUT-BUFFER can only be a buffer, a buffer's
-name (a string), or nil.
+if nil or omitted.  That buffer is in major mode specified by the
+variable `async-shell-command-mode'.  Note that, unlike with
+`shell-command', OUTPUT-BUFFER can only be a buffer, a buffer's name
+(a string), or nil.
 
 You can customize `async-shell-command-buffer' to specify what to do
 when the buffer specified by `shell-command-buffer-name-async' is
@@ -4533,6 +4533,9 @@ async-shell-command
 (declare-function comint-output-filter "comint" (process string))
 (declare-function comint-term-environment "comint" ())
 
+(defvar async-shell-command-mode 'shell-command-mode
+  "Major mode to use for the output of asynchronous `shell-command'.")
+
 (defun shell-command (command &optional output-buffer error-buffer)
   "Execute string COMMAND in inferior shell; display output, if any.
 With prefix argument, insert the COMMAND's output at point.
@@ -4543,9 +4546,9 @@ shell-command
 
 If COMMAND ends in `&', execute it asynchronously.
 The output appears in the buffer whose name is specified
-by `shell-command-buffer-name-async'.  That buffer is in shell
-mode.  You can also use `async-shell-command' that automatically
-adds `&'.
+by `shell-command-buffer-name-async'.  That buffer is in major mode
+specified by the variable `async-shell-command-mode'.  You can also use
+`async-shell-command' that automatically adds `&'.
 
 Otherwise, COMMAND is executed synchronously.  The output appears in
 the buffer named by `shell-command-buffer-name'.  If the output is
@@ -4721,7 +4724,7 @@ shell-command
 		  (setq proc
 			(start-process-shell-command "Shell" buffer command)))
 		(setq mode-line-process '(":%s"))
-                (shell-mode)
+                (funcall async-shell-command-mode)
                 (setq-local revert-buffer-function
                             (lambda (&rest _)
                               (async-shell-command command buffer)))

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Eli Zaretskii]

> Date: Sat, 1 Jun 2024 04:21:46 +0300
> Cc: michael.albinus@gmx.de, 71049@debbugs.gnu.org
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> How about this?

It LGTM, but please also mention the hook issue in NEWS (something
saying that people who had shell-mode-hook do something for async
shell commands should now consider moving that to the hook of the new
mode).

Thanks.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Dmitry Gutov]

Hi Eli,

On 01/06/2024 09:07, Eli Zaretskii wrote:
>> Date: Sat, 1 Jun 2024 04:21:46 +0300
>> Cc:michael.albinus@gmx.de,71049@debbugs.gnu.org
>> From: Dmitry Gutov<dmitry@gutov.dev>
>>
>> How about this?
> It LGTM, but please also mention the hook issue in NEWS (something
> saying that people who had shell-mode-hook do something for async
> shell commands should now consider moving that to the hook of the new
> mode).

Thank you, pushed to master. I've tried adding something about 
shell-mode-hook, but it all feels like truisms (and it's not obvious 
that this hook should be mentioned in particular) - please feel free to 
edit to your preference.

This seems to cover the last remaining issue in this report, so with 
that I'm closing. Thanks all!

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Dmitry Gutov <dmitry@gutov.dev> writes:

> Hi Eli,

Hi Dmitry & Eli,

> This seems to cover the last remaining issue in this report, so with
> that I'm closing. Thanks all!

Does that mean that my proposed change to shell-mode (obeying
tramp-histfile-override) should not be applied? It is independent from
the change Dmitry has pushed.

Best regards, Michael.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Dmitry Gutov]

Hi Michael,

On 01/06/2024 18:47, Michael Albinus wrote:
> Does that mean that my proposed change to shell-mode (obeying
> tramp-histfile-override) should not be applied? It is independent from
> the change Dmitry has pushed.

Looking at it again, it does seem like it could still be useful to 
override or disable the remote history file (right?).

But with your patch, what happens when tramp-history-override is a 
string? Both Tramp and shell will write to it its commands, and upon 
loading shell will also read history from it, including all of the 
commands that Tramp had sent to the remote host. Am I reading it correctly?

Perhaps instead we could have a connection-local variable like 
shell-history-file-name, which could be customized by the user to t to 
disable history - and that could even work per-connection.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Dmitry Gutov <dmitry@gutov.dev> writes:

> Hi Michael,

Hi Dmitry,

> Looking at it again, it does seem like it could still be useful to
> override or disable the remote history file (right?).
>
> But with your patch, what happens when tramp-history-override is a
> string? Both Tramp and shell will write to it its commands, and upon
> loading shell will also read history from it, including all of the
> commands that Tramp had sent to the remote host. Am I reading it
> correctly?
>
> Perhaps instead we could have a connection-local variable like
> shell-history-file-name, which could be customized by the user to t to
> disable history - and that could even work per-connection.

Good idea. Something like this?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 3487 bytes --]

diff --git a/lisp/shell.el b/lisp/shell.el
index 4352811912a..6fb5768c056 100644
--- a/lisp/shell.el
+++ b/lisp/shell.el
@@ -419,6 +419,21 @@ shell--start-prog
   "Shell file name started in `shell'.")
 (put 'shell--start-prog 'permanent-local t)

+(defcustom shell-history-file-name nil
+  "The history file name used in `shell-mode'.
+When it is a string, this file name will be used.
+When it is nil, the environment variable HISTFILE is used.
+When it is t, no history file name is used in `shell-mode'.
+
+The settings obey whether `shell-mode' is invoked in a remote buffer.
+In that case, HISTFILE is taken from the remote host, and the string is
+interpreted as local file name on the remote host.
+
+If `shell-mode' is invoked in a local buffer, and no history file name
+can be determined, a default according to the shell type is used."
+  :type '(choice (const :tag "Default" nil) (const :tag "Suppress" t) file)
+  :version "30.1")
+
 ;;; Basic Procedures

 (defun shell--unquote&requote-argument (qstr &optional upos)
@@ -721,27 +736,33 @@ shell-mode
   (setq list-buffers-directory (expand-file-name default-directory))
   ;; shell-dependent assignments.
   (when (ring-empty-p comint-input-ring)
-    (let ((remote (file-remote-p default-directory))
-          (shell (or shell--start-prog ""))
-          (hsize (getenv "HISTSIZE"))
-          (hfile (getenv "HISTFILE")))
-      (when remote
-        ;; `shell-snarf-envar' does not work trustworthy.
-        (setq hsize (shell-command-to-string "echo -n $HISTSIZE")
-              hfile (shell-command-to-string "echo -n $HISTFILE")))
+    (let* ((remote (file-remote-p default-directory))
+           (shell (or shell--start-prog ""))
+           (hfile (cond ((stringp shell-history-file-name)
+                         shell-history-file-name)
+                        ((null shell-history-file-name)
+                         (if remote
+                             (shell-command-to-string "echo -n $HISTFILE")
+                           (getenv "HISTFILE")))))
+           hsize)
       (and (string-equal hfile "") (setq hfile nil))
-      (and (stringp hsize)
-	   (integerp (setq hsize (string-to-number hsize)))
-	   (> hsize 0)
-           (setq-local comint-input-ring-size hsize))
-      (setq comint-input-ring-file-name
-            (concat
-             remote
-	     (or hfile
-		 (cond ((string-equal shell "bash") "~/.bash_history")
-		       ((string-equal shell "ksh") "~/.sh_history")
-		       ((string-equal shell "zsh") "~/.zsh_history")
-		       (t "~/.history")))))
+      (when (and (not remote) (not hfile))
+        (setq hfile
+              (cond ((string-equal shell "bash") "~/.bash_history")
+		    ((string-equal shell "ksh") "~/.sh_history")
+		    ((string-equal shell "zsh") "~/.zsh_history")
+		    (t "~/.history"))))
+      (when (stringp hfile)
+        (setq hsize
+              (if remote
+                  (shell-command-to-string "echo -n $HISTSIZE")
+                (getenv "HISTSIZE")))
+        (and (stringp hsize)
+	     (integerp (setq hsize (string-to-number hsize)))
+	     (> hsize 0)
+             (setq-local comint-input-ring-size hsize))
+        (setq comint-input-ring-file-name
+              (concat remote hfile)))
       (if (or (equal comint-input-ring-file-name "")
 	      (equal (file-truename comint-input-ring-file-name)
 		     (file-truename null-device)))

[-- Attachment #3: Type: text/plain, Size: 24 bytes --]


Best regards, Michael.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Dmitry Gutov]

Hi Michael,

On 02/06/2024 11:36, Michael Albinus wrote:

>> Looking at it again, it does seem like it could still be useful to
>> override or disable the remote history file (right?).
>>
>> But with your patch, what happens when tramp-history-override is a
>> string? Both Tramp and shell will write to it its commands, and upon
>> loading shell will also read history from it, including all of the
>> commands that Tramp had sent to the remote host. Am I reading it
>> correctly?
>>
>> Perhaps instead we could have a connection-local variable like
>> shell-history-file-name, which could be customized by the user to t to
>> disable history - and that could even work per-connection.
> 
> Good idea. Something like this?

Pretty much, thanks.

> diff --git a/lisp/shell.el b/lisp/shell.el
> index 4352811912a..6fb5768c056 100644
> --- a/lisp/shell.el
> +++ b/lisp/shell.el
> @@ -419,6 +419,21 @@ shell--start-prog
>     "Shell file name started in `shell'.")
>   (put 'shell--start-prog 'permanent-local t)
> 
> +(defcustom shell-history-file-name nil
> +  "The history file name used in `shell-mode'.
> +When it is a string, this file name will be used.
> +When it is nil, the environment variable HISTFILE is used.
> +When it is t, no history file name is used in `shell-mode'.
> +
> +The settings obey whether `shell-mode' is invoked in a remote buffer.
> +In that case, HISTFILE is taken from the remote host, and the string is
> +interpreted as local file name on the remote host.

I think it could be used in the local case, too? If the user sets a 
global value for this variable. That could probably simplify the 
implementation.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Dmitry Gutov <dmitry@gutov.dev> writes:

> Hi Michael,

Hi Dmitry,

>> +The settings obey whether `shell-mode' is invoked in a remote buffer.
>> +In that case, HISTFILE is taken from the remote host, and the string is
>> +interpreted as local file name on the remote host.
>
> I think it could be used in the local case, too? If the user sets a
> global value for this variable. That could probably simplify the
> implementation.

But that's already the case. If the user option is a string, it is taken
literally as the history file name.

What I meant to say is, that the value nil always means to inspect
$HISTFILE, either local or remote. If this variable isn't set, then in
the *local* case the default settings acc to the shell type are
used. This is for backward compatibility.

Best regards, Michael.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Dmitry Gutov]

On 02/06/2024 17:46, Michael Albinus wrote:
> What I meant to say is, that the value nil always means to inspect
> $HISTFILE, either local or remote. If this variable isn't set, then in
> the*local*  case the default settings acc to the shell type are
> used. This is for backward compatibility.

Ah, that makes sense. Thanks!

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Dmitry Gutov <dmitry@gutov.dev> writes:

>> What I meant to say is, that the value nil always means to inspect
>> $HISTFILE, either local or remote. If this variable isn't set, then in
>> the*local*  case the default settings acc to the shell type are
>> used. This is for backward compatibility.
>
> Ah, that makes sense. Thanks!

Pushed to master.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Dmitry Gutov <dmitry@gutov.dev> writes:

> Hi Michael,

Hi Dmitry,

> 1) [first patch] We can add a new major mode, for 'M-&' to use instead
> of the full-blown 'shell-mode' - it could be very simple: just apply
> font-lock keywords and maybe set list-buffers-directory.
>
> Problems? I suppose someone might be using shell-mode-hook to do
> something in the async-shell-command output buffer, and it won't fire
> anymore. Seemingly very minor concern.

This is not only for 'M-&', but for any place asynchronous
'shell-command' is called. You miss a lot of settings 'shell-mode'
applies. No idea whether people need this in async 'shell-command'.

> diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
> index 9385b023392..f0c64a7a90f 100644
> --- a/lisp/net/tramp.el
> +++ b/lisp/net/tramp.el
> @@ -5247,8 +5247,8 @@ tramp-handle-shell-command
>  	      ;; Display output.
>  	      (with-current-buffer output-buffer
>  		(setq mode-line-process '(":%s"))
> -		(unless (eq major-mode 'shell-mode)
> -		  (shell-mode))
> +		(unless (eq major-mode 'shell-command-mode)
> +		  (shell-command-mode))
>  		(set-process-filter p #'comint-output-filter)
>  		(set-process-sentinel p #'shell-command-sentinel)
>  		(when error-file

You want to make this backward compatible, down to Emacs 27.

> diff --git a/lisp/shell.el b/lisp/shell.el
> index e6b315ee5c0..7fa84a37e83 100644
> --- a/lisp/shell.el
> +++ b/lisp/shell.el
> @@ -838,6 +838,13 @@ shell-write-history-on-exit
>        (with-current-buffer buf
>          (insert (format "\nProcess %s %s\n" process event))))))
>
> +(define-derived-mode shell-command-mode comint-mode "Shell"
> +  "Major mode for the output of `async-shell-command'."

"... of asynchronous `shell-command'."

Best regards, Michael.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Dmitry Gutov]

Hi Michael,

On 31/05/2024 10:27, Michael Albinus wrote:
>> 1) [first patch] We can add a new major mode, for 'M-&' to use instead
>> of the full-blown 'shell-mode' - it could be very simple: just apply
>> font-lock keywords and maybe set list-buffers-directory.
>>
>> Problems? I suppose someone might be using shell-mode-hook to do
>> something in the async-shell-command output buffer, and it won't fire
>> anymore. Seemingly very minor concern.
> 
> This is not only for 'M-&', but for any place asynchronous
> 'shell-command' is called. You miss a lot of settings 'shell-mode'
> applies. No idea whether people need this in async 'shell-command'.

My conclusion is they don't, but somebody who uses it more is welcome to 
argue otherwise.

>> diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
>> index 9385b023392..f0c64a7a90f 100644
>> --- a/lisp/net/tramp.el
>> +++ b/lisp/net/tramp.el
>> @@ -5247,8 +5247,8 @@ tramp-handle-shell-command
>>   	      ;; Display output.
>>   	      (with-current-buffer output-buffer
>>   		(setq mode-line-process '(":%s"))
>> -		(unless (eq major-mode 'shell-mode)
>> -		  (shell-mode))
>> +		(unless (eq major-mode 'shell-command-mode)
>> +		  (shell-command-mode))
>>   		(set-process-filter p #'comint-output-filter)
>>   		(set-process-sentinel p #'shell-command-sentinel)
>>   		(when error-file
> 
> You want to make this backward compatible, down to Emacs 27.

Sure, makes sense.

>> diff --git a/lisp/shell.el b/lisp/shell.el
>> index e6b315ee5c0..7fa84a37e83 100644
>> --- a/lisp/shell.el
>> +++ b/lisp/shell.el
>> @@ -838,6 +838,13 @@ shell-write-history-on-exit
>>         (with-current-buffer buf
>>           (insert (format "\nProcess %s %s\n" process event))))))
>>
>> +(define-derived-mode shell-command-mode comint-mode "Shell"
>> +  "Major mode for the output of `async-shell-command'."
> 
> "... of asynchronous `shell-command'."

Thank you.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Eli Zaretskii]

> Date: Wed, 29 May 2024 20:26:26 +0300
> Cc: michael.albinus@gmx.de, 71049@debbugs.gnu.org
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> On 29/05/2024 15:46, Eli Zaretskii wrote:
> >> Seems so. The patches (two alternatives) are attached to
> >> https://debbugs.gnu.org/71049#53.
> > Thanks.  I don't understand what they do in the context of this
> > discussion, probably because I don't know enough about shell-mode and
> > comint.  Could you explain the intent, please?
> 
> Okay, to summarize the previous messages:
> 
> async-shell-command calls shell-mode, which loads the history file by 
> calling comint-read-input-ring.
> 
> shell-setup-input-ring.diff extracts the comint input ring setup to a 
> separate function. That function is then only called by 'shell', but not 
> by 'shell-mode' itself anymore, or any of its descendants. Any callers 
> or descendants of it that actually need this setup (I'm not aware of 
> any, but they might exist) will need to adapt by adding this call.
> 
> shell-no-start-prog.diff does not move the logic, but predicates its 
> execution on the value of the variable shell--start-prog being non-nil. 
> In-tree it's only set by 'shell', so the result is the same. The "how to 
> adapt" recipe would be setting shell--start-prog to non-nil (in callers).

Thanks.  I think I'd prefer the changes in this respect to affect only
remote commands.  Since reading the local history file is almost
instantaneous, risking to break some use case we don't think about
sounds unjustified.  Let's fix the problem where it exists without too
many waves.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: Eli Zaretskii <eliz@gnu.org>,  71049@debbugs.gnu.org
> Date: Sat, 25 May 2024 17:51:22 +0200
> 
> I'm really not convinced that we should change shell-mode in an
> incompatible way for such a minor problem of (not) loading the remote
> history file. shell-mode is one of the building blocks Emnacs consists of.
> 
> Instead we must give the user a config option to suppress this. I've
> shown a possible way to do with my patch, but anything else, which keeps
> compatibility of shell-mode, would do.
> 
> Eli?

Yes, I think an option should be a good starting point.  We could
document in its doc string that avoiding to load history makes remote
shell commands significantly faster.  And maybe the new option could
have a special value that disables history loading only for remote
commands.

TFH, I'm a bit surprised that the *Async Shell Command* buffer turns
on shell-mode, but then I almost never switch to that buffer.  I
understand the rationale, for those who do.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

Hi Eli,

>> > Why is the process being called by such bogus names anyway?
>>
>> I don't understand. Which bogus names?
>
> I thought this was about the original complaints, whtch started this
> bug report, see https://debbugs.gnu.org/cgi/bugreport.cgi?bug=71049#5.
> The fact that the history file was being read sounded as a side issue,
> at least at first.  So my question was about these messages:
>
>   Process *Async Shell Command* finished
>   -l: finished.
>
> I thought the option you suggest is intended to make these "process
> names" be more reasonable.  I guess I am confused, and the discussion
> moved to the "side issue" of preventing the unnecessary reading of the
> history file?

Indeed. I haven't said anything yet about the major concern of this bug,
pls give me time.

Best regards, Michael.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Dmitry Gutov]

Hi Michael,

On 24/05/2024 17:06, Michael Albinus wrote:

>>> The command can be simple, like 'ls' or 'echo 123'. I also see this
>>> added to*Messages*:
>>>
>>>    Tramp: Inserting
>>>    ‘/ssh:dgutov@fencepost.gnu.org:/home/d/dgutov/.tramp_history’...done
>> According to reading .tramp_history: this is performed in
>> comint-read-input-ring, called from shell-mode. comint-input-ring-file-name
>> is set in shell-mode, I don't see a trivial solution to suppress
>> this. Likely, we must extend shell-mode for this case.
> We could add a user option remote-file-name-inhibit-input-ring which
> suppresses reading the remote histfile, when set to non-nil. See
> appended patch.

Maybe a good middle-ground solution would be to defer the reading of the 
history file until history is actually used?

E.g. in my examples there was no reading of input from the user, and 
there will be many read-life scenarios like that.

Perhaps commands like comint-previous-input could check whether the ring 
is not initialized yet and call comint-read-input-ring, rather than have 
this call performed eagerly at the end of shell-mode.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Dmitry Gutov <dmitry@gutov.dev> writes:

> Hi Michael,

Hi Dmitry,

> Maybe a good middle-ground solution would be to defer the reading of
> the history file until history is actually used?
>
> E.g. in my examples there was no reading of input from the user, and
> there will be many read-life scenarios like that.
>
> Perhaps commands like comint-previous-input could check whether the
> ring is not initialized yet and call comint-read-input-ring, rather
> than have this call performed eagerly at the end of shell-mode.

Perhaps. I'm not an expert in comint.el; somebody else could do such a
change.

And please note, that according to my analysis sent to Eli the other
message, half of the party (3 of 6 roundtrips) happens in shell-mode. So
we must indicate something in shell-mode, too, in order to suppress the
check for the proper history file name there.

Best regards, Michael.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Dmitry Gutov]

On 24/05/2024 20:41, Michael Albinus wrote:
>> Maybe a good middle-ground solution would be to defer the reading of
>> the history file until history is actually used?
>>
>> E.g. in my examples there was no reading of input from the user, and
>> there will be many read-life scenarios like that.
>>
>> Perhaps commands like comint-previous-input could check whether the
>> ring is not initialized yet and call comint-read-input-ring, rather
>> than have this call performed eagerly at the end of shell-mode.
> Perhaps. I'm not an expert in comint.el; somebody else could do such a
> change.
> 
> And please note, that according to my analysis sent to Eli the other
> message, half of the party (3 of 6 roundtrips) happens in shell-mode. So
> we must indicate something in shell-mode, too, in order to suppress the
> check for the proper history file name there.

Right, the changes I described would be in comint.el and shell.el, not 
in Tramp (which should make things easier on your side - no new special 
variable).

But there are 6 more calls to comint-read-input-ring (in other 6 major 
modes), so it would be a wide-reaching change. Not sure which direction 
will be optimal. Hmm...

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Dmitry Gutov <dmitry@gutov.dev> writes:

Hi Dmitry,

> But there are 6 more calls to comint-read-input-ring (in other 6 major
> modes), so it would be a wide-reaching change. Not sure which
> direction will be optimal. Hmm...

Yes, I've checked them already shortly. They don't seem to care the
remote case until now, don't know whether we shall pimp them up.

Best regards, Michael.

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Dmitry Gutov <dmitry@gutov.dev> writes:

> When a Tramp connection is configured as "direct-async-process",
>
> 1. If the buffer *Async Shell Command* does not exist, invoking M-& in
> a remote buffer from that connection makes it end with
>
>   Process *Async Shell Command* finished

tramp-handle-make-process must suppress the default sentinel
internal-default-process-sentinel. The counter-part,
tramp-sh-handle-make-process, does it already, that's why you don't see
this message when direct async processes aren't enabled.

> 2. If the buffer such named already exists when command is invoked,
> then this doesn't happen (the output seems correct). But *Messages*
> says
>
>   -l: finished.
>
> ...which is a big puzzling as well.

This is due to shell-command-sentinel. It uses (process-command process)
to determine the finished command. In the local case, this returns
somethiong like ("/usr/bin/sh" "-c" "ls"). In the remote case, it looks
different:

--8<---------------cut here---------------start------------->8---
("ssh" "-q" "-o" "ControlMaster=auto" "-o" "ControlPath=/home/albinus/.cache/emacs/tramp.%C" "-o" "ControlPersist=no" "-e" "none" #("gandalf" 0 7 (tramp-default t)) "cd" "/home/albinus/" "&&" "(" "env" "INSIDE_EMACS\\=30.0.50\\,tramp\\:2.7.1-pre" "PATH\\=/home/albinus\\:/usr/share/Modules/bin\\:/usr/local/bin\\:/usr/bin\\:/usr/local/sbin\\:/usr/sbin\\:/var/lib/snapd/snap/bin\\:/home/albinus/.local/bin\\:/bin\\:/sbin" "ENV\\=\\'\\'" "TMOUT\\=0" "LC_CTYPE\\=\\'\\'" "CDPATH\\=" "HISTORY\\=" "MAIL\\=" "MAILCHECK\\=" "MAILPATH\\=" "PAGER\\=cat" "autocorrect\\=" "correct\\=" "/bin/sh -c ls" ")")
--8<---------------cut here---------------end--------------->8---

So the sentinel must inspect the process property remote-command first.

I've tried to fix both problems. Could you, pls, check the appended
patch?

Best regards, Michael.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 1952 bytes --]

diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 18116229337..9385b023392 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -5025,11 +5025,13 @@ tramp-handle-make-process
 	       ?h (or host "") ?u (or user "") ?p (or port "")
 	       ?c (format-spec (or options "") (format-spec-make ?t tmpfile))
 	       ?d (or device "") ?a (or pta "") ?l ""))))
+	   ;; Suppress `internal-default-process-sentinel', which is
+	   ;; set when :sentinel is nil.  (Bug#71049)
 	   p (make-process
 	      :name name :buffer buffer
 	      :command (append `(,login-program) login-args command)
 	      :coding coding :noquery noquery :connection-type connection-type
-	      :sentinel sentinel :stderr stderr))
+	      :sentinel (or sentinel #'ignore) :stderr stderr))
 	  ;; Set filter.  Prior Emacs 29.1, it doesn't work reliably
 	  ;; to provide it as `make-process' argument when filter is
 	  ;; t.  See Bug#51177.
diff --git a/lisp/simple.el b/lisp/simple.el
index bcd26da13ed..714accab1af 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -4863,11 +4863,14 @@ display-message-or-buffer
 ;; We have a sentinel to prevent insertion of a termination message
 ;; in the buffer itself, and to set the point in the buffer when
 ;; `shell-command-dont-erase-buffer' is non-nil.
+;; For remote shells, `process-command' does not serve the proper shell
+;; command.  We use process property `remote-command' instead.  (Bug#71049)
 (defun shell-command-sentinel (process signal)
   (when (memq (process-status process) '(exit signal))
     (shell-command-set-point-after-cmd (process-buffer process))
     (message "%s: %s."
-             (car (cdr (cdr (process-command process))))
+             (car (cdr (cdr (or (process-get process 'remote-command)
+                                (process-command process)))))
              (substring signal 0 -1))))

 (defun shell-command-on-region (start end command

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Dmitry Gutov]

On 25/05/2024 16:03, Michael Albinus wrote:
> This is due to shell-command-sentinel. It uses (process-command process)
> to determine the finished command. In the local case, this returns
> somethiong like ("/usr/bin/sh" "-c" "ls"). In the remote case, it looks
> different:
> 
> --8<---------------cut here---------------start------------->8---
> ("ssh" "-q" "-o" "ControlMaster=auto" "-o" "ControlPath=/home/albinus/.cache/emacs/tramp.%C" "-o" "ControlPersist=no" "-e" "none" #("gandalf" 0 7 (tramp-default t)) "cd" "/home/albinus/" "&&" "(" "env" "INSIDE_EMACS\\=30.0.50\\,tramp\\:2.7.1-pre" "PATH\\=/home/albinus\\:/usr/share/Modules/bin\\:/usr/local/bin\\:/usr/bin\\:/usr/local/sbin\\:/usr/sbin\\:/var/lib/snapd/snap/bin\\:/home/albinus/.local/bin\\:/bin\\:/sbin"
>   "ENV\\=\\'\\'" "TMOUT\\=0" "LC_CTYPE\\=\\'\\'" "CDPATH\\=" "HISTORY\\="
>   "MAIL\\=" "MAILCHECK\\=" "MAILPATH\\=" "PAGER\\=cat" "autocorrect\\="
> "correct\\=" "/bin/sh -c ls" ")")
> --8<---------------cut here---------------end--------------->8---
> 
> So the sentinel must inspect the process property remote-command first.
> 
> I've tried to fix both problems. Could you, pls, check the appended
> patch?

Seems to be working well. Thanks!

bug#71049: async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Dmitry Gutov <dmitry@gutov.dev> writes:

>> I've tried to fix both problems. Could you, pls, check the appended
>> patch?
>
> Seems to be working well. Thanks!

Pushed to master. I'll keep the bug report open, until we know what to
do with the histfile.