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

From: Eli Zaretskii <[email-masked]>
Date: Tue, 20 Aug 2024 22:01:33 +0300

> Date: Tue, 20 Aug 2024 10:33:06 -0700
> Cc: [email-masked], [email-masked]
> From: Jim Porter <[email-masked]>
> 
> > 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.

> > It looks like you are breaking min-width support for display strings?
> > They are used on the mode line and also in other places, and in
> > general, min-width should treat buffers and strings alike.  Can you
> > explain the motivation for the proposed changes, and describe what you
> > saw with the current code in this case?  Where's the call to
> > get-text-property and why did it use a buffer position instead of a
> > string position?
> 
> You're probably right. I think my patch was a little over-aggressive 
> (see attached for a more-surgical one). This patch may still be wrong, 
> but hopefully it gets a bit closer to what we want.

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.

> 
> I think this is what's happening, in a bit more detail: 
> magit-log-current uses overlays (I think to set up the right margin 
> text?). When visual-wrap-prefix-mode ("vwpm") is enabled, the display 
> engine goes through the buffer, finds the 'min-width' display property 
> from vwpm and holds onto it. Next, it starts processing an overlay.
> 
> Eventually, that calls 'handle_display_prop' for the overlay which calls 
> 'display_min_width'. At this point, we have an object stored in
> 'it->min_width_property' (thanks to vwpm), the local variable 'object' 
> is the overlay string, and 'bufpos' is the actual buffer position.
> 
> Finally we call 'get_display_property' with the bufpos and object (which 
> calls 'Fget_text_property'), and kaboom: 'object' is a string of length 
> 1, but bufpos is much larger (~400 in my test).

This sounds like we shouldn't be calling get_display_property here, or
use a different OBJECT when we call it.  IOW, the object and the
position must match, and since here they don't, there's a disconnect
somewhere.  We need to find that disconnect and understand why it
happens.