From: Noam Postavsky <npostavs@gmail.com>
To: Eli Zaretskii <eliz@gnu.org>
Cc: Lars Ingebrigtsen <larsi@gnus.org>,
abliss@gmail.com, 36591@debbugs.gnu.org
Subject: bug#36591: 26.2; Term's pager seems broken
Date: Wed, 24 Jul 2019 20:55:38 -0400 [thread overview]
Message-ID: <87ftmvvsol.fsf@gmail.com> (raw)
In-Reply-To: <83y30no4hi.fsf@gnu.org> (Eli Zaretskii's message of "Wed, 24 Jul 2019 18:07:53 +0300")
[-- Attachment #1: Type: text/plain, Size: 1289 bytes --]
Eli Zaretskii <eliz@gnu.org> writes:
> I see that it's a small mess, but I don't think I understand your
> reasoning about setting the filter to t twice: it looks to me that
> both calls will clear the bit of p->infd, because they both will
> trigger this clause:
>
> if (EQ (p->filter, Qt) && !EQ (p->status, Qlisten))
> Am I missing something?
Argh, no, I was confused. The code in Emacs 25 is correct, although
(IMO) somewhat misleading.
> . if the filter is not t, we will now call add_process_read_fd every
> time, for no good reason
(This is a moot point due to the second problem, but)
add_process_read_fd just sets some bits, which is harmless even if
repeated.
> . the patch changes how connect_network_socket works in ways that we
> don't sufficiently understand
Yes, I agree this is a serious problem.
> I would like to leave connect_network_socket alone on the release
> branch, and just fix the problem with set-process-filter. I think the
> easiest way is simply not to call set_process_filter_masks in
> set-process-filter, and instead restore the way that code worked
> before 9755b753, i.e. let it test the argument FILTER _before_ that
> filter is installed. Do you see any problems with this fix?
I think that makes sense, patch attached.
[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 1719 bytes --]
From ee13774efa2ac4b288f339c668b6002e27fbbe8f Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Wed, 24 Jul 2019 20:33:18 -0400
Subject: [PATCH] Fix subproc listening when setting filter to non-t
(Bug#36591)
* src/process.c (Fset_process_filter): Call add_process_read_fd
according to the state of process filter before it's updated. This
restores the correct functioning as it was before 2016-02-16 "Allow
setting the filter masks later". Inline the set_process_filter_masks
call instead of fixing it that function, because it is also called
from connect_network_socket, and we don't want to change the behavior
of that function so close to release.
---
src/process.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/process.c b/src/process.c
index 2df51cfd99..b602507765 100644
--- a/src/process.c
+++ b/src/process.c
@@ -1268,10 +1268,19 @@ DEFUN ("set-process-filter", Fset_process_filter, Sset_process_filter,
if (NILP (filter))
filter = Qinternal_default_process_filter;
- pset_filter (p, filter);
-
if (p->infd >= 0)
- set_process_filter_masks (p);
+ {
+ /* If filter WILL be t, stop reading output. */
+ if (EQ (filter, Qt) && !EQ (p->status, Qlisten))
+ delete_read_fd (p->infd);
+ else if (/* If filter WAS t, then resume reading output. */
+ EQ (p->filter, Qt)
+ /* Network or serial process not stopped: */
+ && !EQ (p->command, Qt))
+ add_process_read_fd (p->infd);
+ }
+
+ pset_filter (p, filter);
if (NETCONN1_P (p) || SERIALCONN1_P (p) || PIPECONN1_P (p))
pset_childp (p, Fplist_put (p->childp, QCfilter, filter));
--
2.11.0
[-- Attachment #3: Type: text/plain, Size: 640 bytes --]
> On the master branch we should clean up the confusing set of if
> clauses, both in set-process-filter and in connect_network_socket.
> Perhaps Lars could describe his reasoning for making the change which
> introduced set_process_filter_masks and what problem it tried to
> solve. (Btw, the log message for that change seems to imply that
> set-process-filter should not have called set_process_filter_masks,
> something that the change itself disagrees with. An omission?)
Hmm, true, I didn't pay that close attention to the log message.
Maybe "we may not have a socket yet" refers to the already existing
'if (p->infd >= 0)' check?
next prev parent reply other threads:[~2019-07-25 0:55 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-11 0:06 bug#36591: 26.2; Term's pager seems broken Adam Bliss
2019-07-23 13:53 ` Noam Postavsky
2019-07-23 17:40 ` Eli Zaretskii
2019-07-23 22:33 ` Adam Bliss
2019-07-24 2:07 ` Noam Postavsky
2019-07-24 15:07 ` Eli Zaretskii
2019-07-25 0:55 ` Noam Postavsky [this message]
2019-07-25 10:02 ` Lars Ingebrigtsen
2019-07-25 13:01 ` Eli Zaretskii
2019-07-25 16:58 ` Lars Ingebrigtsen
2019-07-25 13:01 ` Noam Postavsky
2019-07-25 17:01 ` Lars Ingebrigtsen
2019-07-25 17:28 ` Noam Postavsky
2019-07-25 17:44 ` Lars Ingebrigtsen
2019-07-25 17:57 ` Noam Postavsky
2019-07-25 13:11 ` Eli Zaretskii
2019-07-25 22:38 ` Noam Postavsky
2020-08-21 12:58 ` Lars Ingebrigtsen
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87ftmvvsol.fsf@gmail.com \
--to=npostavs@gmail.com \
--cc=36591@debbugs.gnu.org \
--cc=abliss@gmail.com \
--cc=eliz@gnu.org \
--cc=larsi@gnus.org \
/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 external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.