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 14:53:30 +0300

> Date: Mon, 19 Aug 2024 17:46:18 -0700
> Cc: [email-masked]
> From: Jim Porter <[email-masked]>
> 
> On 8/19/2024 2:39 PM, Gautier Ponsinet wrote:
> > Hello everyone,
> > 
> > The new visual-wrap-prefix-mode breaks the rendering of the Magit Log
> > buffers.
> > 
> > In emacs -Q:
> > * Install Magit and its dependencies and load Magit.
> > * Go to a local repository (via M-x dired or M-x cd).
> > * M-x global-visual-wrap-prefix-mode
> > * M-x magit-log-current
> > 
> > Could someone please confirm/reproduce?
> 
> I can confirm this. I'm not quite sure of all the details, but it seem 
> that this is due to a bad interaction between overlays and the 
> 'min-width' display spec. The end result was that we were calling 
> 'get-text-property' with a (large-ish) buffer position when the OBJECT 
> arg was a string of length 1. That can happen in magit-log on the 
> mostly-blank line where it's making the ASCII art just below a merge 
> commit. (The leading whitespace makes 'visual-wrap-prefix-mode' do its 
> thing.)
> 
> I'm not super familiar with how the display engine works, but I think we 
> don't want to call 'display_min_width' when we're working with an 
> overlay. See the attached patch.

I'd appreciate a reproducer without Magit, as I don't have it
installed and would prefer not to have to.

> Eli, I'm sure you understand this code much better than me. Does the 
> above make sense? I can also try to improve the commentary in the code, 
> but I'm just making some educated guesses as to what's happening here.

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?