emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Ignore pending_signals when checking for quits.


From: Philipp Stephani
Subject: Re: [PATCH] Ignore pending_signals when checking for quits.
Date: Sun, 10 Feb 2019 20:46:22 +0100

Am So., 10. Feb. 2019 um 20:41 Uhr schrieb Eli Zaretskii <address@hidden>:
>
> > From: Philipp Stephani <address@hidden>
> > Date: Sun, 10 Feb 2019 19:49:39 +0100
> > Cc: Emacs developers <address@hidden>, Philipp Stephani <address@hidden>
> >
> > > Bother.  I see your point regarding the return value when just
> > > pending_signals is set, but disregarding pending_signals doesn't sound
> > > TRT to me, either.  It means various Emacs features based on input
> > > detection won't work while the module code runs, even if the module
> > > tries to be nice and does call module_should_quit.  For example,
> > > while-no-input and atimers won't work, and Emacs will generally be
> > > much less responsive to user input.
> > >
> > > So maybe we should indeed return true only if QUITP says so, but we
> > > should also call process_pending_signals from module_should_quit, when
> > > pending_signals is non-zero?
> >
> > Wouldn't that mean that Emacs could do something (e.g. process
> > events)? That wouldn't match the naming and intention of should_quit:
> > By its name, it should only query information and not change any
> > state.
>
> If the only problem is the name, we could change the name.  Or we
> could introduce a new function.  But let's first agree about the
> substance: a well-behaving module should from time to time call
> process_pending_signals.  Agreed?

Yes, absolutely. We can't change the name (this would break backwards
compatibility), but introducing a new function would be fine.
For should_quit itself, I still think we should make the change in
this patch. The current code is clearly buggy: it promises to quit but
doesn't in most cases.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]