bug#72721: 31.0.50; Visual-wrap-prefix-mode breaks Magit log buffers

[Jim Porter <jporterbugs@gmail.com>]

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

On 8/20/2024 12:01 PM, Eli Zaretskii wrote:
>> Date: Tue, 20 Aug 2024 10:33:06 -0700
>> Cc: 72721@debbugs.gnu.org, gautier@gautierponsinet.xyz
>> From: Jim Porter <jporterbugs@gmail.com>
>>
>>> I'd appreciate a reproducer without Magit, as I don't have it
>>> installed and would prefer not to have to.
>>
>> Me too... I haven't been able to get a reduced test case yet since Magit
>> is pretty complex and I haven't figured out what it's doing exactly. It
>> *seems* to be due to overlays, but I only know that from examining
>> things in GDB. I haven't deciphered the relevant Magit code yet.
> 
> M-x describe-text-properties will show you the properties and overlays
> at point, and it should be possible to concoct some Lisp which just
> reproduces those properties.

I'd tried that but just wasn't looking at the right point. I've now 
figured it out and provided a few reduced test cases. (The "simple" and 
"consecutive" cases should already work.)

While making these test cases, I noticed a similar issue with a nested 
'display' property (see the "nested" case), and fixed that too (I hope!).

> It still doesn't feel right.  But min-width is weird, so I need to
> look into the problem deeper to understand what is going on, and if
> you will be able to come up with a reproducer without Magit, it will
> help.

Hopefully the attached reproducers help make sense of this. I've also 
updated my patch to handle 'min-width' in what I think is a simpler way. 
This implementation relies on the fact that you can't nest 'min-width' 
specs (the iterator struct can only hold one spec at a time). I'm 
guessing on some of these parts, so I may be totally off-base, but the 
test cases do what I expect anyway...

[-- Attachment #2: 0001-Fix-bad-interaction-between-min-width-display-spec-a.patch --]
[-- Type: text/plain, Size: 3421 bytes --]

From 13fe68ebc2eb2fe7ca0ee4ac4733b3abc3ed0cab Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Mon, 19 Aug 2024 17:38:47 -0700
Subject: [PATCH] Fix bad interaction between 'min-width' display spec and
 overlays

Previously, when iterating over overlays, we would pass the overlay
string and the buffer position to 'display_min_width', which would use
those values to try to get the display property.  However, the buffer
position is very likely out of bounds for the overlay string!

* src/xdisp.c (get_display_property): Rename BUFPOS to CHARPOS.
(display_min_width): Take CHARPOS instead of BUFPOS, and get BUFPOS on
our own.  This way, we can be sure that when calling
'get_display_property', we provide it with the correct kind of position.
(handle_display_prop): Pass the character pos of OBJECT's position to
'display_min_width' (bug#72721).
---
 src/xdisp.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/src/xdisp.c b/src/xdisp.c
index 30771a1c83d..af93a824bee 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -5633,17 +5633,19 @@ find_display_property (Lisp_Object disp, Lisp_Object prop)
 }
 
 static Lisp_Object
-get_display_property (ptrdiff_t bufpos, Lisp_Object prop, Lisp_Object object)
+get_display_property (ptrdiff_t charpos, Lisp_Object prop, Lisp_Object object)
 {
-  return find_display_property (Fget_text_property (make_fixnum (bufpos),
+  return find_display_property (Fget_text_property (make_fixnum (charpos),
 						    Qdisplay, object),
 				prop);
 }
 
 static void
-display_min_width (struct it *it, ptrdiff_t bufpos,
+display_min_width (struct it *it, ptrdiff_t charpos,
 		   Lisp_Object object, Lisp_Object width_spec)
 {
+  ptrdiff_t bufpos = CHARPOS (it->current.pos);
+
   /* We're being called at the end of the `min-width' sequence,
      probably. */
   if (!NILP (it->min_width_property)
@@ -5658,9 +5660,9 @@ display_min_width (struct it *it, ptrdiff_t bufpos,
 		   get_display_property (0, Qmin_width, object)))
 	  /* In a buffer -- check that we're really right after the
 	     sequence of characters covered by this `min-width'.  */
-	  || (bufpos > BEGV
+	  || (bufpos > BEGV && charpos > 0
 	      && EQ (it->min_width_property,
-		     get_display_property (bufpos - 1, Qmin_width, object))))
+		     get_display_property (charpos - 1, Qmin_width, object))))
 	{
 	  Lisp_Object w = Qnil;
 	  double width;
@@ -5713,9 +5715,9 @@ display_min_width (struct it *it, ptrdiff_t bufpos,
 	      && !EQ (it->min_width_property,
 		      get_display_property (0, Qmin_width, object)))
 	  /* Buffer.  */
-	  || (bufpos > BEGV
+	  || (bufpos > BEGV && charpos > 0
 	      && !EQ (width_spec,
-		      get_display_property (bufpos - 1, Qmin_width, object))))
+		      get_display_property (charpos - 1, Qmin_width, object))))
 	{
 	  it->min_width_property = width_spec;
 	  it->min_width_start = it->current_x;
@@ -5795,10 +5797,10 @@ handle_display_prop (struct it *it)
   if (!STRINGP (it->string))
     object = it->w->contents;
 
-  /* Handle min-width ends. */
+  /* Handle min-width ends, except when processing an overlay.  */
   if (!NILP (it->min_width_property)
       && NILP (find_display_property (propval, Qmin_width)))
-    display_min_width (it, bufpos, object, Qnil);
+    display_min_width (it, CHARPOS (*position), object, Qnil);
 
   if (NILP (propval))
     return HANDLED_NORMALLY;
-- 
2.25.1


[-- Attachment #3: test-cases.el --]
[-- Type: text/plain, Size: 1100 bytes --]

(progn
  ;; Should look like:
  ;; 12345
  ;; 678     90
  (switch-to-buffer "simple")
  (erase-buffer)
  (insert "12345\n67890")
  (redisplay)
  (put-text-property
   7 10 'display '((min-width (8)))))


(progn
  ;; Should look like:
  ;; 12345
  ;; 67  89  0
  (switch-to-buffer "consecutive")
  (erase-buffer)
  (insert "12345\n67890")
  (redisplay)
  (put-text-property
   7 9 'display '((min-width (4))))
  (put-text-property
   9 11 'display '((min-width (4)))))


(progn
  ;; Should look like:
  ;; 12345
  ;; hi  890
  (switch-to-buffer "nested")
  (erase-buffer)
  (insert "12345\n67890")
  (redisplay)
  (put-text-property
   7 9 'display
   (propertize "hi" 'display '((min-width (4))))))


(progn
  ;; Should look like:
  ;; 12345         |
  ;; 678     90    |X
  (switch-to-buffer "overlay")
  (erase-buffer)
  (insert "12345\n67890")
  (set-window-margins (selected-window) 0 1)
  (redisplay)
  (setq o (make-overlay 8 9))
  (overlay-put o 'before-string
               (propertize "o" 'display '((margin right-margin) "X")))
  (put-text-property
   7 10 'display '((min-width (8)))))