GNU bug report logs -
#71049
async-shell-command ends with "Process *Async Shell Command* finished" when remote "direct-async-process"
Previous Next
Reported by: Dmitry Gutov <dmitry <at> gutov.dev>
Date: Sun, 19 May 2024 00:20:02 UTC
Severity: normal
Done: Dmitry Gutov <dmitry <at> gutov.dev>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 71049 in the body.
You can then email your comments to 71049 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Sun, 19 May 2024 00:20:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Dmitry Gutov <dmitry <at> gutov.dev>:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org.
(Sun, 19 May 2024 00:20:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
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 <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Sun, 19 May 2024 06:18:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 71049 <at> debbugs.gnu.org (full text, mbox):
> Date: Sun, 19 May 2024 03:19:00 +0300
> From: Dmitry Gutov <dmitry <at> 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 <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Sun, 19 May 2024 12:41:01 GMT)
Full text and
rfc822 format available.
Message #11 received at 71049 <at> debbugs.gnu.org (full text, mbox):
On 19/05/2024 09:17, Eli Zaretskii wrote:
>> Date: Sun, 19 May 2024 03:19:00 +0300
>> From: Dmitry Gutov<dmitry <at> 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 <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Fri, 24 May 2024 11:16:02 GMT)
Full text and
rfc822 format available.
Message #14 received at 71049 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dmitry <at> 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 <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Fri, 24 May 2024 14:07:02 GMT)
Full text and
rfc822 format available.
Message #17 received at 71049 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Michael Albinus <michael.albinus <at> 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 <at> 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.
[Message part 2 (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Fri, 24 May 2024 14:51:01 GMT)
Full text and
rfc822 format available.
Message #20 received at 71049 <at> debbugs.gnu.org (full text, mbox):
> Cc: 71049 <at> 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 <at> gnu.org>
>
> >> The command can be simple, like 'ls' or 'echo 123'. I also see this
> >> added to *Messages*:
> >>
> >> Tramp: Inserting
> >> ‘/ssh:dgutov <at> 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?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Fri, 24 May 2024 16:40:01 GMT)
Full text and
rfc822 format available.
Message #23 received at 71049 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> 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 <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Fri, 24 May 2024 17:19:01 GMT)
Full text and
rfc822 format available.
Message #26 received at 71049 <at> debbugs.gnu.org (full text, mbox):
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 <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Fri, 24 May 2024 17:42:02 GMT)
Full text and
rfc822 format available.
Message #29 received at 71049 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dmitry <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Fri, 24 May 2024 17:51:01 GMT)
Full text and
rfc822 format available.
Message #32 received at 71049 <at> debbugs.gnu.org (full text, mbox):
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...
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Fri, 24 May 2024 18:11:02 GMT)
Full text and
rfc822 format available.
Message #35 received at 71049 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dmitry <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Fri, 24 May 2024 18:59:01 GMT)
Full text and
rfc822 format available.
Message #38 received at 71049 <at> debbugs.gnu.org (full text, mbox):
> From: Michael Albinus <michael.albinus <at> gmx.de>
> Cc: dmitry <at> gutov.dev, 71049 <at> debbugs.gnu.org
> Date: Fri, 24 May 2024 18:39:21 +0200
>
> Eli Zaretskii <eliz <at> 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?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Fri, 24 May 2024 19:22:01 GMT)
Full text and
rfc822 format available.
Message #41 received at 71049 <at> debbugs.gnu.org (full text, mbox):
On 24/05/2024 21:55, Eli Zaretskii wrote:
>> From: Michael Albinus <michael.albinus <at> gmx.de>
>> Cc: dmitry <at> gutov.dev, 71049 <at> debbugs.gnu.org
>> Date: Fri, 24 May 2024 18:39:21 +0200
>>
>> Eli Zaretskii <eliz <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Sat, 25 May 2024 07:24:02 GMT)
Full text and
rfc822 format available.
Message #44 received at 71049 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Sat, 25 May 2024 10:51:02 GMT)
Full text and
rfc822 format available.
Message #47 received at 71049 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dmitry <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Sat, 25 May 2024 13:05:02 GMT)
Full text and
rfc822 format available.
Message #50 received at 71049 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Dmitry Gutov <dmitry <at> 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.
[Message part 2 (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Sat, 25 May 2024 13:56:02 GMT)
Full text and
rfc822 format available.
Message #53 received at 71049 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
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.
[shell-setup-input-ring.diff (text/x-patch, attachment)]
[shell-no-start-prog.diff (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Sat, 25 May 2024 14:41:02 GMT)
Full text and
rfc822 format available.
Message #56 received at 71049 <at> debbugs.gnu.org (full text, mbox):
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!
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Sat, 25 May 2024 15:27:01 GMT)
Full text and
rfc822 format available.
Message #59 received at 71049 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dmitry <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Sat, 25 May 2024 15:52:01 GMT)
Full text and
rfc822 format available.
Message #62 received at 71049 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dmitry <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Sat, 25 May 2024 16:18:02 GMT)
Full text and
rfc822 format available.
Message #65 received at 71049 <at> debbugs.gnu.org (full text, mbox):
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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Sat, 25 May 2024 17:01:01 GMT)
Full text and
rfc822 format available.
Message #68 received at 71049 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dmitry <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Sat, 25 May 2024 17:12:02 GMT)
Full text and
rfc822 format available.
Message #71 received at 71049 <at> debbugs.gnu.org (full text, mbox):
> From: Michael Albinus <michael.albinus <at> gmx.de>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 71049 <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Sat, 25 May 2024 17:32:02 GMT)
Full text and
rfc822 format available.
Message #74 received at 71049 <at> debbugs.gnu.org (full text, mbox):
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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Sat, 25 May 2024 17:45:01 GMT)
Full text and
rfc822 format available.
Message #77 received at 71049 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dmitry <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Sun, 26 May 2024 14:19:01 GMT)
Full text and
rfc822 format available.
Message #80 received at 71049 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Michael Albinus <michael.albinus <at> 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.
[Message part 2 (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Wed, 29 May 2024 02:01:02 GMT)
Full text and
rfc822 format available.
Message #83 received at 71049 <at> debbugs.gnu.org (full text, mbox):
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
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Wed, 29 May 2024 07:43:01 GMT)
Full text and
rfc822 format available.
Message #86 received at 71049 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dmitry <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Wed, 29 May 2024 11:54:01 GMT)
Full text and
rfc822 format available.
Message #89 received at 71049 <at> debbugs.gnu.org (full text, mbox):
> Date: Wed, 29 May 2024 04:59:43 +0300
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 71049 <at> debbugs.gnu.org
> From: Dmitry Gutov <dmitry <at> 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?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Wed, 29 May 2024 11:57:02 GMT)
Full text and
rfc822 format available.
Message #92 received at 71049 <at> debbugs.gnu.org (full text, mbox):
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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Wed, 29 May 2024 11:58:01 GMT)
Full text and
rfc822 format available.
Message #95 received at 71049 <at> debbugs.gnu.org (full text, mbox):
On 29/05/2024 14:53, Eli Zaretskii wrote:
>> Date: Wed, 29 May 2024 04:59:43 +0300
>> Cc: Eli Zaretskii<eliz <at> gnu.org>,71049 <at> debbugs.gnu.org
>> From: Dmitry Gutov<dmitry <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Wed, 29 May 2024 12:47:02 GMT)
Full text and
rfc822 format available.
Message #98 received at 71049 <at> debbugs.gnu.org (full text, mbox):
> Date: Wed, 29 May 2024 14:57:08 +0300
> Cc: michael.albinus <at> gmx.de, 71049 <at> debbugs.gnu.org
> From: Dmitry Gutov <dmitry <at> gutov.dev>
>
> On 29/05/2024 14:53, Eli Zaretskii wrote:
> >> Date: Wed, 29 May 2024 04:59:43 +0300
> >> Cc: Eli Zaretskii<eliz <at> gnu.org>,71049 <at> debbugs.gnu.org
> >> From: Dmitry Gutov<dmitry <at> 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?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Wed, 29 May 2024 15:20:01 GMT)
Full text and
rfc822 format available.
Message #101 received at 71049 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dmitry <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Wed, 29 May 2024 17:27:01 GMT)
Full text and
rfc822 format available.
Message #104 received at 71049 <at> debbugs.gnu.org (full text, mbox):
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).
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Wed, 29 May 2024 17:43:01 GMT)
Full text and
rfc822 format available.
Message #107 received at 71049 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dmitry <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Wed, 29 May 2024 18:14:02 GMT)
Full text and
rfc822 format available.
Message #110 received at 71049 <at> debbugs.gnu.org (full text, mbox):
> Date: Wed, 29 May 2024 20:26:26 +0300
> Cc: michael.albinus <at> gmx.de, 71049 <at> debbugs.gnu.org
> From: Dmitry Gutov <dmitry <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Wed, 29 May 2024 18:16:02 GMT)
Full text and
rfc822 format available.
Message #113 received at 71049 <at> debbugs.gnu.org (full text, mbox):
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?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Wed, 29 May 2024 18:39:01 GMT)
Full text and
rfc822 format available.
Message #116 received at 71049 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Dmitry Gutov <dmitry <at> 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:
[Message part 2 (text/x-patch, inline)]
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
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Wed, 29 May 2024 20:41:02 GMT)
Full text and
rfc822 format available.
Message #119 received at 71049 <at> debbugs.gnu.org (full text, mbox):
On 29/05/2024 21:38, Michael Albinus wrote:
> Dmitry Gutov<dmitry <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Thu, 30 May 2024 08:51:02 GMT)
Full text and
rfc822 format available.
Message #122 received at 71049 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dmitry <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Fri, 31 May 2024 00:26:02 GMT)
Full text and
rfc822 format available.
Message #125 received at 71049 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
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.
[shell-command-mode.diff (text/x-patch, attachment)]
[shell-start-prog-none.diff (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Fri, 31 May 2024 05:54:01 GMT)
Full text and
rfc822 format available.
Message #128 received at 71049 <at> debbugs.gnu.org (full text, mbox):
> Date: Fri, 31 May 2024 03:24:49 +0300
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 71049 <at> debbugs.gnu.org
> From: Dmitry Gutov <dmitry <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Fri, 31 May 2024 07:28:02 GMT)
Full text and
rfc822 format available.
Message #131 received at 71049 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dmitry <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Fri, 31 May 2024 12:14:01 GMT)
Full text and
rfc822 format available.
Message #134 received at 71049 <at> debbugs.gnu.org (full text, mbox):
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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Fri, 31 May 2024 16:25:01 GMT)
Full text and
rfc822 format available.
Message #137 received at 71049 <at> debbugs.gnu.org (full text, mbox):
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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Fri, 31 May 2024 18:09:02 GMT)
Full text and
rfc822 format available.
Message #140 received at 71049 <at> debbugs.gnu.org (full text, mbox):
> Date: Fri, 31 May 2024 19:24:10 +0300
> Cc: michael.albinus <at> gmx.de, 71049 <at> debbugs.gnu.org
> From: Dmitry Gutov <dmitry <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Sat, 01 Jun 2024 01:23:02 GMT)
Full text and
rfc822 format available.
Message #143 received at 71049 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On 31/05/2024 21:05, Eli Zaretskii wrote:
>> Date: Fri, 31 May 2024 19:24:10 +0300
>> Cc:michael.albinus <at> gmx.de,71049 <at> debbugs.gnu.org
>> From: Dmitry Gutov<dmitry <at> 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?
[shell-command-mode.diff (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Sat, 01 Jun 2024 06:08:01 GMT)
Full text and
rfc822 format available.
Message #146 received at 71049 <at> debbugs.gnu.org (full text, mbox):
> Date: Sat, 1 Jun 2024 04:21:46 +0300
> Cc: michael.albinus <at> gmx.de, 71049 <at> debbugs.gnu.org
> From: Dmitry Gutov <dmitry <at> 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.
Reply sent
to
Dmitry Gutov <dmitry <at> gutov.dev>:
You have taken responsibility.
(Sat, 01 Jun 2024 15:34:02 GMT)
Full text and
rfc822 format available.
Notification sent
to
Dmitry Gutov <dmitry <at> gutov.dev>:
bug acknowledged by developer.
(Sat, 01 Jun 2024 15:34:02 GMT)
Full text and
rfc822 format available.
Message #151 received at 71049-done <at> debbugs.gnu.org (full text, mbox):
Hi Eli,
On 01/06/2024 09:07, Eli Zaretskii wrote:
>> Date: Sat, 1 Jun 2024 04:21:46 +0300
>> Cc:michael.albinus <at> gmx.de,71049 <at> debbugs.gnu.org
>> From: Dmitry Gutov<dmitry <at> 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!
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Sat, 01 Jun 2024 15:48:01 GMT)
Full text and
rfc822 format available.
Message #154 received at 71049-done <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dmitry <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Sun, 02 Jun 2024 01:40:02 GMT)
Full text and
rfc822 format available.
Message #157 received at 71049-done <at> debbugs.gnu.org (full text, mbox):
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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Sun, 02 Jun 2024 08:37:02 GMT)
Full text and
rfc822 format available.
Message #160 received at 71049-done <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Dmitry Gutov <dmitry <at> 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?
[Message part 2 (text/x-patch, attachment)]
[Message part 3 (text/plain, inline)]
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Sun, 02 Jun 2024 14:11:01 GMT)
Full text and
rfc822 format available.
Message #163 received at 71049-done <at> debbugs.gnu.org (full text, mbox):
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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Sun, 02 Jun 2024 14:48:02 GMT)
Full text and
rfc822 format available.
Message #166 received at 71049-done <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dmitry <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Sun, 02 Jun 2024 15:03:02 GMT)
Full text and
rfc822 format available.
Message #169 received at 71049-done <at> debbugs.gnu.org (full text, mbox):
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!
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#71049; Package
emacs.
(Sun, 02 Jun 2024 17:32:02 GMT)
Full text and
rfc822 format available.
Message #172 received at 71049 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dmitry <at> 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 archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Mon, 01 Jul 2024 11:24:08 GMT)
Full text and
rfc822 format available.
This bug report was last modified 129 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.