GNU bug report logs - #38966
27.0.60; Assertion failure in set_cursor_from_row

Previous Next

Full log

View this message in rfc822 format

From: Eli Zaretskii <eliz <at> gnu.org>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 38966 <at> 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

> From: martin rudalics <rudalics <at> 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)

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.