[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Testing redisplay code in batch
From: |
Stefan Monnier |
Subject: |
Re: Testing redisplay code in batch |
Date: |
Thu, 24 Sep 2020 14:31:58 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) |
> Thanks. I have a couple of minor comments:
Based on your comments, I assume you consider that this feature is
acceptable for `master` once it's clean.
>> + if (FRAME_INITIAL_P (f))
>> + paused_p = false; /* No actual display to update! */
>
> I'd appreciate a comment here saying why setting paused_p to false is
> the consequence of "no display to update".
It's not. The other branch sets `paused_p` so we need to set it
as well. The comment explains why we don't do anything else *beside*
setting `paused_p`.
Not sure how best to explain that in the comment. I changed it to:
/* No actual display to update so the "update" is a nop and
obviously isn't interrupted by pending input. */
paused_p = false;
>> - /* 10 is arbitrary,
>> + /* 80/40 is arbitrary,
> Why not 80x25?
80x25 it is, then ;-)
>> @@ -1136,6 +1136,7 @@ make_initial_frame (void)
>> init_frame_faces (f);
>>
>> last_nonminibuf_frame = f;
>> + echo_area_window = f->minibuffer_window;
>
> Why is this bit needed?
Ah, good question.
Without it I got an assertion failure in xdisp.c:18241
if (MINI_WINDOW_P (w))
{
if (w == XWINDOW (echo_area_window)
&& !NILP (echo_area_buffer[0]))
because XWINDOW complains that `echo_area_window` is not a window.
Maybe the better fix is to change `init_xdisp` so it also sets
`echo_area_window` when `noninteractive`?
>> - move_it_by_lines (&it, 0);
>> + move_it_by_lines (&it, 0); /* bug#43519 */
>
> This is unrelated (and unneeded, IMO).
Indeed, it's just a temporary hack I added so I can quickly find that
spot in the code (via `C-x v =`) when I need to (un)comment it as I test
my patch ;-)
>> @@ -15414,8 +15414,8 @@ redisplay_internal (void)
>> /* No redisplay if running in batch mode or frame is not yet fully
>> initialized, or redisplay is explicitly turned off by setting
>> Vinhibit_redisplay. */
>> - if (FRAME_INITIAL_P (SELECTED_FRAME ())
>> - || !NILP (Vinhibit_redisplay))
>> + if (/* FRAME_INITIAL_P (SELECTED_FRAME ())
>> + * || */ !NILP (Vinhibit_redisplay))
>> return;
>
> This should be done cleaner, and should also update the commentary.
Do you think it's otherwise acceptable as-is? I was thinking of only
allowing redisplay in `FRAME_INITIAL_P` under the control of some
special config var.
> How about a small NEWS item about this?
I'll see to it,
Stefan
Re: Testing redisplay code in batch, Alan Mackenzie, 2020/09/23
Re: Testing redisplay code in batch, Eli Zaretskii, 2020/09/23