all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#38966: 27.0.60; Assertion failure in set_cursor_from_row
@ 2020-01-06  9:19 martin rudalics
  2020-01-06 17:14 ` Eli Zaretskii
  0 siblings, 1 reply; 12+ messages in thread
From: martin rudalics @ 2020-01-06  9:19 UTC (permalink / raw)
  To: 38966

[-- Attachment #1: Type: text/plain, Size: 4517 bytes --]

In GNU Emacs 27.0.60 (build 13, x86_64-w64-mingw32)
  of 2020-01-05 built on MACHNO
Repository revision: 7f01dfca5600fcd3e3548aee50734aab4b96b02f
Repository branch: emacs-27

With emacs -Q load the attached test-set-cursor-from-row.el and use
the mouse to drag the upper mode line up and down.  As soon as one of
the windows gets sufficiently small, I get an assertion failure like


Thread 1 hit Breakpoint 1, terminate_due_to_signal (sig=22, backtrace_limit=2147483647) at ../../src/emacs.c:371
371	  signal (sig, SIG_DFL);
(gdb) bt
#0  terminate_due_to_signal (sig=22, backtrace_limit=2147483647) at ../../src/emacs.c:371
#1  0x00000004002c13cd in die (msg=0x400974295 <get_next_element+3989> "!row->mode_line_p", file=0x400970642 <DEFAULT_REHASH_SIZE+4958> "../../src/xdisp.c", line=16249) at ../../src/alloc.c:7246
#2  0x000000040007ca89 in set_cursor_from_row (w=0x76c8500, row=0x76b0070, matrix=0x76cd580, delta=0, delta_bytes=0, dy=0, dvpos=0) at ../../src/xdisp.c:16249
#3  0x0000000400087177 in redisplay_window (window=XIL(0x76c8505), just_this_one_p=false) at ../../src/xdisp.c:18780
#4  0x000000040007c87c in redisplay_window_0 (window=XIL(0x76c8505)) at ../../src/xdisp.c:16190
#5  0x000000040031a182 in internal_condition_case_1 (bfun=0x40007c83d <redisplay_window_0>, arg=XIL(0x76c8505), handlers=XIL(0x6421ebb), hfun=0x40007c801 <redisplay_window_error>) at ../../src/eval.c:1379
#6  0x000000040007c7d3 in redisplay_windows (window=XIL(0x76c8505)) at ../../src/xdisp.c:16170
#7  0x000000040007c785 in redisplay_windows (window=XIL(0x76c82f5)) at ../../src/xdisp.c:16164
#8  0x000000040007b0ad in redisplay_internal () at ../../src/xdisp.c:15638
#9  0x00000004000789af in redisplay () at ../../src/xdisp.c:14865
#10 0x00000004001b6162 in read_char (commandflag=1, map=XIL(0x77dcfb3), prev_event=XIL(0), used_mouse_menu=0xbff25f, end_time=0x0) at ../../src/keyboard.c:2493
#11 0x00000004001c93dd in read_key_sequence (keybuf=0xbff490, prompt=XIL(0), dont_downcase_last=false, can_return_switch_frame=true, fix_current_buffer=true, prevent_redisplay=false) at ../../src/keyboard.c:9553
#12 0x00000004001b28fc in command_loop_1 () at ../../src/keyboard.c:1350
#13 0x000000040031a08d in internal_condition_case (bfun=0x4001b23c6 <command_loop_1>, handlers=XIL(0x90), hfun=0x4001b1807 <cmd_error>) at ../../src/eval.c:1355
#14 0x00000004001b1f3c in command_loop_2 (ignore=XIL(0)) at ../../src/keyboard.c:1091
#15 0x00000004003194e2 in internal_catch (tag=XIL(0xdfe0), func=0x4001b1f0a <command_loop_2>, arg=XIL(0)) at ../../src/eval.c:1116
#16 0x00000004001b1e92 in command_loop () at ../../src/keyboard.c:1070
#17 0x0000000000000000 in ?? ()
Backtrace stopped: previous frame inner to this frame (corrupt stack?)

Lisp Backtrace:
"redisplay_internal (C function)" (0x0)


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;

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.

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.

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.

martin

[-- Attachment #2: test-set-cursor-from-row.el --]
[-- Type: text/plain, Size: 940 bytes --]

(progn
  (tab-line-mode 1)
  (ruler-mode 1)
  (setq
   mode-line-format
   (cons (propertize
	  " " 'display
          (create-image "/* XPM */ static char * image[] = {
\"3 60 1 1\",
\"0 c #00aaff\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\"
};" 'xpm t :ascent 'center))
	 mode-line-format))
  (setq window-min-height 0)
  (setq window-min-width 0)
  (split-window))

^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#38966: 27.0.60; Assertion failure in set_cursor_from_row
  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
  2020-01-06 19:06   ` martin rudalics
  0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2020-01-06 17:14 UTC (permalink / raw)
  To: martin rudalics; +Cc: 38966

> 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)





^ permalink raw reply related	[flat|nested] 12+ messages in thread

* bug#38966: 27.0.60; Assertion failure in set_cursor_from_row
  2020-01-06 17:14 ` Eli Zaretskii
@ 2020-01-06 19:06   ` martin rudalics
  2020-01-06 19:50     ` Eli Zaretskii
  0 siblings, 1 reply; 12+ messages in thread
From: martin rudalics @ 2020-01-06 19:06 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 38966

[-- Attachment #1: Type: text/plain, Size: 2250 bytes --]

 > 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.

We did.  Try with emacs 26 loading my old test-popup-2.el (attached
once more).  Type C-x 2 and make the lower window as low as you can.
Now hit F2.  The lower window shows no text.

So we did allow windows showing no text before and I see no reason why
we should call that a calamity.  Rather, we should eliminate all
assertions that forbid zero size windows and simply not draw a cursor
when a window's body shrinks to zero pixels.  All applications I know
of tolerate such zero size "windows".

I plan a slight modification that would allow the minibuffer window to
shrink to zero lines whenever it's not used.  OTOH, zero size "normal"
windows would allow to show the minibuffer window in a frame that can
acommodate only one or two lines.

 > 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.

Isn't that statement overly conservative?  Sooner or later, Emacs will
at least have to allow a mode where the region is not destroyed when a
window is scrolled, allowing the cursor to disappear from the visible
part of its buffer.

We should keep Emacs distinctive.  But being distinctive doesn't
necessarily mean to be restrictive.

 >> 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.

Which comment?  In either case, rest assured that ordinary people will
have considerable troubles understanding code like the proposed

	  if (row->tab_line_p)
	    ++row;
	  if (row->mode_line_p)
	    ++row;

Notwithstanding my concerns, the patch fixes the bug here.

Many thanks, martin

[-- Attachment #2: test-popup-2.el --]
[-- Type: text/plain, Size: 1705 bytes --]

(set-frame-position nil 0 0)
(switch-to-buffer (find-file-noselect "c:/temp/test-popup.el"))
(setq window-resize-pixelwise t)
(setq window-min-height 0)
(set-window-scroll-bars (minibuffer-window) 0 nil 0 nil)
(defvar old-mode-line-format mode-line-format)

(defun foo-message (&optional newline)
  (message
   (concat (if newline "\n" "")
	   "total: %s, body: %s, mode: %s")
   (window-pixel-height) (window-body-height nil t)
   (window-mode-line-height)))

(defun foo ()
  (interactive)
  (foo-message))

(global-set-key [f1] 'foo)

(defun test-popup ()
  (interactive)
;;   (set-face-attribute 'mode-line nil :height 200)
;;   (set-face-attribute 'mode-line-inactive nil :height 200)
  (setq
   mode-line-format
   (cons (propertize " " 'display
                     (create-image "/* XPM */ static char * image[] = {
\"3 60 1 1\",
\"0 c #00aaff\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\"
};" 'xpm t :ascent 'center))
	 mode-line-format))
  (foo-message t))

(global-set-key [f2] 'test-popup)

(defun bar ()
  (interactive)
  (setq mode-line-format old-mode-line-format))

(global-set-key [f3] 'bar)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#38966: 27.0.60; Assertion failure in set_cursor_from_row
  2020-01-06 19:06   ` martin rudalics
@ 2020-01-06 19:50     ` Eli Zaretskii
  2020-01-07  9:52       ` martin rudalics
  0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2020-01-06 19:50 UTC (permalink / raw)
  To: martin rudalics; +Cc: 38966-done

> Cc: 38966@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Mon, 6 Jan 2020 20:06:21 +0100
> 
>  > 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.
> 
> We did.

OK, let me rephrase: _I_ didn't, okay?  IOW, the display code doesn't,
and AFAIR never did.

(For some reason I seem to make people angry today, and you seem to be
one of them.  Apologies -- I don't know for what.)

> Try with emacs 26 loading my old test-popup-2.el (attached once
> more).  Type C-x 2 and make the lower window as low as you can.  Now
> hit F2.  The lower window shows no text.

Why is such a window useful?  I thought we had a minimum window height
beyond which we didn't allow to resize windows.  Isn't that true
anymore?

> I plan a slight modification that would allow the minibuffer window to
> shrink to zero lines whenever it's not used.  OTOH, zero size "normal"
> windows would allow to show the minibuffer window in a frame that can
> acommodate only one or two lines.

All I'm saying is that making the display code prepared to deal with
such windows might take more changes, and this would be a separate
issue, suitable for master, not for the release branch.  OK?

>  > 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.
> 
> Isn't that statement overly conservative?  Sooner or later, Emacs will
> at least have to allow a mode where the region is not destroyed when a
> window is scrolled, allowing the cursor to disappear from the visible
> part of its buffer.

That's not the same situation.  If point is not in the window, it's OK
not to display the cursor.  But I'm talking about a situation where
point _is_ in the window.

>  > 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.
> 
> Which comment?

This one:

  /* True means row is a mode or header/tab-line.  */
  bool_bf mode_line_p : 1;

> In either case, rest assured that ordinary people will
> have considerable troubles understanding code like the proposed
> 
> 	  if (row->tab_line_p)
> 	    ++row;
> 	  if (row->mode_line_p)
> 	    ++row;

I added comments explaining what's going on.

> Notwithstanding my concerns, the patch fixes the bug here.

Thanks, pushed to the emacs-27 branch, and closing the bug.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#38966: 27.0.60; Assertion failure in set_cursor_from_row
  2020-01-06 19:50     ` Eli Zaretskii
@ 2020-01-07  9:52       ` martin rudalics
  2020-01-07 16:01         ` Eli Zaretskii
  0 siblings, 1 reply; 12+ messages in thread
From: martin rudalics @ 2020-01-07  9:52 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 38966-done

 > Why is such a window useful?

Windows with one text line like that of the ediff control panel have
been in use for quite some time.  They can also arise automatically,
for example, when fitting a one line completions window to its buffer.

Windows that are made as small as possible are useful when a user
wants to make a neigboring window temporarily as large as possible
without deleting other windows.  Zero height windows can be also
useful when they do not display any text but applications want to
sample their width in order to, for example, decide how to lay out
text in them.

 > I thought we had a minimum window height
 > beyond which we didn't allow to resize windows.  Isn't that true
 > anymore?

We currently have two minimum heights - 'window-safe-min-height' and
the option 'window-min-height'.  Both are defined in canonical frame
lines and have to be eventually re-interpreted in order to fix bugs
like Bug#14825.  In this context it would be nice to do away with
'window-safe-min-height' because it only complicates the resizing code
without being able to avoid, as we've seen, that a window's text
height can drop to zero.

 > All I'm saying is that making the display code prepared to deal with
 > such windows might take more changes, and this would be a separate
 > issue, suitable for master, not for the release branch.  OK?

Sure.  My concerns are only that we should not preclude future changes
in this area when making changes on the release branch like adding new
assertions.  To fix the present bug, for example, I would have simply
removed the offending assertion from set_cursor_from_row.  Eventually,
we might want to show a cursor in a mode line, or scroll or wrap it.

martin





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#38966: 27.0.60; Assertion failure in set_cursor_from_row
  2020-01-07  9:52       ` martin rudalics
@ 2020-01-07 16:01         ` Eli Zaretskii
  2020-01-07 17:38           ` martin rudalics
  0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2020-01-07 16:01 UTC (permalink / raw)
  To: martin rudalics; +Cc: 38966

> Cc: 38966-done@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Tue, 7 Jan 2020 10:52:18 +0100
> 
>  > Why is such a window useful?
> 
> Windows with one text line like that of the ediff control panel have
> been in use for quite some time.  They can also arise automatically,
> for example, when fitting a one line completions window to its buffer.

I thought we were talking about windows with zero lines of text.  One
text line is fine, and should not cause any trouble to the current
display code.  When testing my patch, I used your code to resized the
window to a single 1-pixel high line of text, and saw no problems,
including with displaying the cursor.

If the use cases you have in mind can make do with one 1-pixel line of
text, we will need no changes in the display engine at all to support
that.

>  > All I'm saying is that making the display code prepared to deal with
>  > such windows might take more changes, and this would be a separate
>  > issue, suitable for master, not for the release branch.  OK?
> 
> Sure.  My concerns are only that we should not preclude future changes
> in this area when making changes on the release branch like adding new
> assertions.  To fix the present bug, for example, I would have simply
> removed the offending assertion from set_cursor_from_row.  Eventually,
> we might want to show a cursor in a mode line, or scroll or wrap it.

I think we should keep that assertion as long as we don't make the
code DTRT with such text-less windows.  That assertion only affects
developers anyway.  The right time for removing the assertion is when
we modify the code to support windows with no text rows.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#38966: 27.0.60; Assertion failure in set_cursor_from_row
  2020-01-07 16:01         ` Eli Zaretskii
@ 2020-01-07 17:38           ` martin rudalics
  2020-01-07 17:48             ` Eli Zaretskii
  0 siblings, 1 reply; 12+ messages in thread
From: martin rudalics @ 2020-01-07 17:38 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 38966

 > I thought we were talking about windows with zero lines of text.  One
 > text line is fine, and should not cause any trouble to the current
 > display code.  When testing my patch, I used your code to resized the
 > window to a single 1-pixel high line of text, and saw no problems,
 > including with displaying the cursor.

We are miscommunicating here.  In your previous post you said that

 > >  > 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.
 > >
 > > We did.
 >
 > OK, let me rephrase: _I_ didn't, okay?  IOW, the display code doesn't,
 > and AFAIR never did.
 >
 > (For some reason I seem to make people angry today, and you seem to be
 > one of them.  Apologies -- I don't know for what.)

With my test-popup-2 code you can make the lower window one line high,
hit F2 and get a window without text and cursor.  Which means that the
display code (inadvertently maybe) already does produce and handle
windows with zero lines of text.  I still don't understand why you
thought I was angry when I reported that behavior.

 > I think we should keep that assertion as long as we don't make the
 > code DTRT with such text-less windows.  That assertion only affects
 > developers anyway.  The right time for removing the assertion is when
 > we modify the code to support windows with no text rows.

Agreed.

martin





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#38966: 27.0.60; Assertion failure in set_cursor_from_row
  2020-01-07 17:38           ` martin rudalics
@ 2020-01-07 17:48             ` Eli Zaretskii
  2020-01-07 18:23               ` martin rudalics
  0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2020-01-07 17:48 UTC (permalink / raw)
  To: martin rudalics; +Cc: 38966

> Cc: 38966@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Tue, 7 Jan 2020 18:38:54 +0100
> 
> With my test-popup-2 code you can make the lower window one line high,
> hit F2 and get a window without text and cursor.

I seem to be unable to do that, so please show detailed instructions
starting from "emacs -Q".  What I get here is a very thin single line
of text, where I still see the cursor (partially) and can type text.

> Which means that the display code (inadvertently maybe) already does
> produce and handle windows with zero lines of text.

Maybe so, but that is just by sheer luck, AFAICT.

> I still don't understand why you thought I was angry when I reported
> that behavior.

If I was mistaken, again, apologies.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#38966: 27.0.60; Assertion failure in set_cursor_from_row
  2020-01-07 17:48             ` Eli Zaretskii
@ 2020-01-07 18:23               ` martin rudalics
  2020-01-07 18:36                 ` Eli Zaretskii
  0 siblings, 1 reply; 12+ messages in thread
From: martin rudalics @ 2020-01-07 18:23 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 38966

[-- Attachment #1: Type: text/plain, Size: 563 bytes --]

 > I seem to be unable to do that, so please show detailed instructions
 > starting from "emacs -Q".  What I get here is a very thin single line
 > of text, where I still see the cursor (partially) and can type text.

I attach a new file test-popup-3.el.  With emacs -Q load it, type F1,
then F2.  After C-x o I can move through the lower window without
seeing any cursor or text line.  I also attached two screenshots, one
before C-x o and one after.  If you look hard enough, you can spot the
remnants of the scroll bar on the lower window's mode line.

martin

[-- Attachment #2: test-popup-3.el --]
[-- Type: text/plain, Size: 1357 bytes --]

(defun foo-message (&optional newline)
  (message
   (concat (if newline "\n" "")
	   "total: %s, body: %s, mode: %s")
   (window-pixel-height) (window-body-height nil t)
   (window-mode-line-height)))

(defun split-window-2 ()
  (interactive)
  (split-window nil -2))

(global-set-key [f1] 'split-window-2)

(defun test-popup ()
  (interactive)
;;   (set-face-attribute 'mode-line nil :height 200)
;;   (set-face-attribute 'mode-line-inactive nil :height 200)
  (setq
   mode-line-format
   (cons (propertize " " 'display
                     (create-image "/* XPM */ static char * image[] = {
\"3 60 1 1\",
\"0 c #00aaff\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\"
};" 'xpm t :ascent 'center))
	 mode-line-format))
  (foo-message t))

(global-set-key [f2] 'test-popup)

[-- Attachment #3: capture_01072020_191510.png --]
[-- Type: image/png, Size: 20805 bytes --]

[-- Attachment #4: capture_01072020_191636.png --]
[-- Type: image/png, Size: 19697 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#38966: 27.0.60; Assertion failure in set_cursor_from_row
  2020-01-07 18:23               ` martin rudalics
@ 2020-01-07 18:36                 ` Eli Zaretskii
  2020-01-07 18:58                   ` martin rudalics
  0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2020-01-07 18:36 UTC (permalink / raw)
  To: martin rudalics; +Cc: 38966

> Cc: 38966@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Tue, 7 Jan 2020 19:23:39 +0100
> 
> I attach a new file test-popup-3.el.  With emacs -Q load it, type F1,
> then F2.  After C-x o I can move through the lower window without
> seeing any cursor or text line.  I also attached two screenshots, one
> before C-x o and one after.  If you look hard enough, you can spot the
> remnants of the scroll bar on the lower window's mode line.

Don't you see that the mode lines overlap?  That's exactly the sign of
us being unprepared for such windows.  I hope you didn't mean we
should leave that alone and be happy about what we see in this case,
yes?





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#38966: 27.0.60; Assertion failure in set_cursor_from_row
  2020-01-07 18:36                 ` Eli Zaretskii
@ 2020-01-07 18:58                   ` martin rudalics
  2020-01-07 19:06                     ` Eli Zaretskii
  0 siblings, 1 reply; 12+ messages in thread
From: martin rudalics @ 2020-01-07 18:58 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 38966

 > Don't you see that the mode lines overlap?  That's exactly the sign of
 > us being unprepared for such windows.  I hope you didn't mean we
 > should leave that alone and be happy about what we see in this case,
 > yes?

Being more humble I'm already happy when Emacs doesn't crash on me in
this case (and that you can, hopefully, reproduce the recipe now).

Worse comes to worst, I'm positive that one can triple the effect with
appropriate header and tab lines' formats.

martin





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#38966: 27.0.60; Assertion failure in set_cursor_from_row
  2020-01-07 18:58                   ` martin rudalics
@ 2020-01-07 19:06                     ` Eli Zaretskii
  0 siblings, 0 replies; 12+ messages in thread
From: Eli Zaretskii @ 2020-01-07 19:06 UTC (permalink / raw)
  To: martin rudalics; +Cc: 38966

> Cc: 38966@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Tue, 7 Jan 2020 19:58:02 +0100
> 
>  > Don't you see that the mode lines overlap?  That's exactly the sign of
>  > us being unprepared for such windows.  I hope you didn't mean we
>  > should leave that alone and be happy about what we see in this case,
>  > yes?
> 
> Being more humble I'm already happy when Emacs doesn't crash on me in
> this case (and that you can, hopefully, reproduce the recipe now).

Sometimes crashing is better than showing corrupted display, IME.

Anyway, I think this issue can be safely closed.





^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-01-07 19:06 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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

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.