From: Jim Porter <jporterbugs@gmail.com>
To: Eli Zaretskii <eliz@gnu.org>
Cc: 72721@debbugs.gnu.org, gautier@gautierponsinet.xyz
Subject: bug#72721: 31.0.50; Visual-wrap-prefix-mode breaks Magit log buffers
Date: Tue, 20 Aug 2024 20:15:48 -0700 [thread overview]
Message-ID: <c3a6a9e9-0cae-f7bb-a52e-29c30a3a2e46@gmail.com> (raw)
In-Reply-To: <86r0ajuigi.fsf@gnu.org>
[-- 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)))))
next prev parent reply other threads:[~2024-08-21 3:15 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-19 21:39 bug#72721: 31.0.50; Visual-wrap-prefix-mode breaks Magit log buffers Gautier Ponsinet
2024-08-20 0:46 ` Jim Porter
2024-08-20 11:53 ` Eli Zaretskii
2024-08-20 17:33 ` Jim Porter
2024-08-20 19:01 ` Eli Zaretskii
2024-08-21 3:15 ` Jim Porter [this message]
2024-08-21 5:18 ` Jim Porter
2024-08-21 19:12 ` Jim Porter
2024-08-22 12:54 ` Eli Zaretskii
2024-08-22 9:59 ` Eli Zaretskii
2024-08-22 9:53 ` Eli Zaretskii
2024-08-22 16:19 ` Jim Porter
2024-08-25 7:29 ` Eli Zaretskii
2024-08-25 16:26 ` Jim Porter
2024-08-25 17:39 ` Eli Zaretskii
2024-08-25 18:41 ` Jim Porter
2024-08-29 11:47 ` Eli Zaretskii
2024-08-30 5:01 ` Jim Porter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c3a6a9e9-0cae-f7bb-a52e-29c30a3a2e46@gmail.com \
--to=jporterbugs@gmail.com \
--cc=72721@debbugs.gnu.org \
--cc=eliz@gnu.org \
--cc=gautier@gautierponsinet.xyz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.