GNU bug report logs -
#72721
31.0.50; Visual-wrap-prefix-mode breaks Magit log buffers
Previous Next
Full log
View this message in rfc822 format
[Message part 1 (text/plain, inline)]
On 8/20/2024 4:53 AM, Eli Zaretskii wrote:
>> Date: Mon, 19 Aug 2024 17:46:18 -0700
>> Cc: eliz <at> gnu.org
>> From: Jim Porter <jporterbugs <at> gmail.com>
>>
>> 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.
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.
>> 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?
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.
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).
I've also attached a backtrace, though I'm not sure how informative it
is on its own.
[0001-Fix-bad-interaction-between-min-width-display-spec-a.patch (text/plain, attachment)]
[backtrace.txt (text/plain, attachment)]
This bug report was last modified 264 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.