From: Robert Pluim <rpluim@gmail.com>
To: Helmut Eller <eller.helmut@gmail.com>
Cc: 64975@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org>
Subject: bug#64975: 30.0.50; accept-process-output and async connect
Date: Tue, 08 Aug 2023 17:16:52 +0200 [thread overview]
Message-ID: <87350ta7iz.fsf@gmail.com> (raw)
In-Reply-To: <m2pm3x1u0f.fsf@gmail.com> (Helmut Eller's message of "Tue, 08 Aug 2023 16:36:00 +0200")
>>>>> On Tue, 08 Aug 2023 16:36:00 +0200, Helmut Eller <eller.helmut@gmail.com> said:
Helmut> On Tue, Aug 08 2023, Robert Pluim wrote:
>> I think itʼs correct, as I have a change locally setting
>> got_some_output for a different test case, but Iʼm going to be a pain,
>> and ask Helmut to explain why, and see if I agree with his explanation
>> (thatʼs a very hairy loop)
Helmut> Setting got_some_output=1, was the first thing that came to mind that
Helmut> made the test case pass :-). A reasonable strategy, if we have a
Helmut> comprehensive test suite. If the test suite is lacking, then writing
Helmut> more tests is a good investment too. Ahem.
Submitting new tests is always good :-)
Helmut> Setting got_some_output=1 will terminate the while(1) loop, but only on
Helmut> the next iteration (around line process.c:5753) and only after another
Helmut> useless call to xg_select. So maybe a change like below might be
Helmut> better.
It also means we finish looping through all the channels, unlike your
patch below. I think thatʼs a smaller and thus better change, and
aligns more with the docstring:
Allow any pending output from subprocesses to be read by Emacs.
It is given to their filter functions.
So if the rest of the test cases pass, I think we should apply your
original patch.
Helmut> The variable got_some_output is also the return value of
Helmut> wait_reading_process_output. So I thought that 1 is a reasonable value
Helmut> to indicate "some event happened". 0 and negative values are converted
Helmut> to nil in accept-process-output, so there isn't an obvious way to
Helmut> indicate "not a timeout, 0 bytes read, but some other event". Maybe
Helmut> MAX_INT could be used.
1 is ok as a value.
Helmut> If you're asking why accept-process-output should return at all, then
Helmut> the answer is that the socket is now writable and the caller probably
Helmut> want's to know that.
I think thatʼs ok, since any actual input received from the process
will get passed to the filter function anyway.
Eli, the docstring also says
Optional argument PROCESS means to return only after output is
received from PROCESS or PROCESS closes the connection.
Do we need to add something like "or the underlying network connection
becomes available"? (I wonder if thatʼs too strong a guarantee).
Thanks
Robert
--
next prev parent reply other threads:[~2023-08-08 15:16 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-31 13:31 bug#64975: 30.0.50; accept-process-output and async connect Helmut Eller
2023-08-05 9:26 ` Eli Zaretskii
2023-08-08 9:10 ` Robert Pluim
2023-08-08 12:15 ` Eli Zaretskii
2023-08-08 12:32 ` Robert Pluim
2023-08-08 14:36 ` Helmut Eller
2023-08-08 15:16 ` Robert Pluim [this message]
2023-08-08 15:33 ` Eli Zaretskii
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87350ta7iz.fsf@gmail.com \
--to=rpluim@gmail.com \
--cc=64975@debbugs.gnu.org \
--cc=eliz@gnu.org \
--cc=eller.helmut@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).