From: Eli Zaretskii <eliz@gnu.org>
To: martin rudalics <rudalics@gmx.at>
Cc: 38966@debbugs.gnu.org
Subject: bug#38966: 27.0.60; Assertion failure in set_cursor_from_row
Date: Mon, 06 Jan 2020 19:14:49 +0200 [thread overview]
Message-ID: <83lfqkecc6.fsf@gnu.org> (raw)
In-Reply-To: <497636f5-1728-1e7e-b826-8310e2a6fe13@gmx.at> (message from martin rudalics on Mon, 6 Jan 2020 10:19:02 +0100)
> From: martin rudalics <rudalics@gmx.at>
> Date: Mon, 6 Jan 2020 10:19:02 +0100
>
> The immediate cause is that the *scratch* window has a header _and_ a
> tab line. Disabling either of these makes the failure disappear.
>
> The culprit is IIUC this part of xdisp.c
>
> /* Finally, fall back on the first row of the window after the
> header line (if any). This is slightly better than not
> displaying the cursor at all. */
> if (!row)
> {
> row = matrix->rows;
> if (row->mode_line_p)
> ++row;
> }
> set_cursor_from_row (w, row, matrix, 0, 0, 0, 0);
>
> which is no more valid because the first row is now the tab line of
> the *scratch* window. In this case the line we get by ++row is the
> header line triggering the subsequent assertion failure
>
> /* Don't even try doing anything if called for a mode-line or
> header-line row, since the rest of the code isn't prepared to
> deal with such calamities. */
> eassert (!row->mode_line_p);
> if (row->mode_line_p)
> return false;
Thanks for the analysis. I agree with you. A proposed patch is
below.
> If my analysis is correct, we should probably skip two lines to move
> to the first text line of the window. But I would not exclude the
> possibility (I have not tried to produce such behavior yet) that that
> third line does not even exist (what would the "(if any)" in the
> comment otherwise allude to?) or at least is the mode line of that
> window and the assertion failure would trigger again.
I don't expect to have a window that has no lines showing text. I
believe we don't allow creation/resizing of windows to such a small
size? If that's not guaranteed, I'm okay with adding an assertion
somewhere, but that would be a separate problem: we never expected
such a calamity even before tab-lines were added.
> So I think we should do two things there: (1) skip subsequent lines
> with row->mode_line_p enabled and (2) not call set_cursor_from_row
> when we have consumed all rows.
If we don't call set_cursor_from_row, we will not have a cursor
displayed where it should be, which is a display bug in and of itself.
> BTW: I would also recommend to rename mode_line_p to something like
> say "no_text_line_p". Presently, people like me not used to hack the
> display code, might consider mode_line_p verbatim and not apply its
> semantics to header or tab lines.
Sorry, but that ship sailed a long time ago. You will have to make do
with the comments in dispextern.h, which document this weirdness.
diff --git a/src/xdisp.c b/src/xdisp.c
index 3d286cb22f..8b20601a93 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -16244,8 +16244,8 @@ set_cursor_from_row (struct window *w, struct glyph_row *row,
bool string_from_text_prop = false;
/* Don't even try doing anything if called for a mode-line or
- header-line row, since the rest of the code isn't prepared to
- deal with such calamities. */
+ header-line or tab-line row, since the rest of the code isn't
+ prepared to deal with such calamities. */
eassert (!row->mode_line_p);
if (row->mode_line_p)
return false;
@@ -17504,6 +17504,8 @@ try_cursor_movement (Lisp_Object window, struct text_pos startp,
else
{
row = MATRIX_ROW (w->current_matrix, w->last_cursor_vpos);
+ if (row->tab_line_p)
+ ++row;
if (row->mode_line_p)
++row;
if (!row->enabled_p)
@@ -17576,6 +17578,8 @@ try_cursor_movement (Lisp_Object window, struct text_pos startp,
|| row->mode_line_p)
{
row = w->current_matrix->rows;
+ if (row->tab_line_p)
+ ++row;
if (row->mode_line_p)
++row;
}
@@ -17640,8 +17644,9 @@ try_cursor_movement (Lisp_Object window, struct text_pos startp,
;
else if (rc != CURSOR_MOVEMENT_SUCCESS
&& MATRIX_ROW_PARTIALLY_VISIBLE_P (w, row)
- /* Make sure this isn't a header line by any chance, since
- then MATRIX_ROW_PARTIALLY_VISIBLE_P might yield true. */
+ /* Make sure this isn't a header line nor a tab-line by
+ any chance, since then MATRIX_ROW_PARTIALLY_VISIBLE_P
+ might yield true. */
&& !row->mode_line_p
&& !cursor_row_fully_visible_p (w, true, true, true))
{
@@ -18769,11 +18774,13 @@ redisplay_window (Lisp_Object window, bool just_this_one_p)
}
}
/* Finally, fall back on the first row of the window after the
- header line (if any). This is slightly better than not
- displaying the cursor at all. */
+ tab-line and header line (if any). This is slightly better
+ than not displaying the cursor at all. */
if (!row)
{
row = matrix->rows;
+ if (row->tab_line_p)
+ ++row;
if (row->mode_line_p)
++row;
}
@@ -19787,7 +19794,9 @@ row_containing_pos (struct window *w, ptrdiff_t charpos,
ptrdiff_t mindif = BUF_ZV (XBUFFER (w->contents)) + 1;
int last_y;
- /* If we happen to start on a header-line, skip that. */
+ /* If we happen to start on a header-line or a tab-line, skip that. */
+ if (row->tab_line_p)
+ ++row;
if (row->mode_line_p)
++row;
@@ -22380,7 +22389,7 @@ find_row_edges (struct it *it, struct glyph_row *row,
if (STRINGP (it->object)
/* this is not the first row */
&& row > it->w->desired_matrix->rows
- /* previous row is not the header line */
+ /* previous row is not the header line or tab-line */
&& !r1->mode_line_p
/* previous row also ends in a newline from a string */
&& r1->ends_in_newline_from_string_p)
next prev parent reply other threads:[~2020-01-06 17:14 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-06 9:19 bug#38966: 27.0.60; Assertion failure in set_cursor_from_row martin rudalics
2020-01-06 17:14 ` Eli Zaretskii [this message]
2020-01-06 19:06 ` martin rudalics
2020-01-06 19:50 ` Eli Zaretskii
2020-01-07 9:52 ` martin rudalics
2020-01-07 16:01 ` Eli Zaretskii
2020-01-07 17:38 ` martin rudalics
2020-01-07 17:48 ` Eli Zaretskii
2020-01-07 18:23 ` martin rudalics
2020-01-07 18:36 ` Eli Zaretskii
2020-01-07 18:58 ` martin rudalics
2020-01-07 19:06 ` 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=83lfqkecc6.fsf@gnu.org \
--to=eliz@gnu.org \
--cc=38966@debbugs.gnu.org \
--cc=rudalics@gmx.at \
/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).