On 10/13/2024 11:51 AM, Eli Zaretskii wrote:
> That is probably correct, at least in some cases, but why should we
> rely on that for this purpose?  Isn't it more reliable and
> future-proof to skip the entire span of the outermost "replacing"
> display property, and never look at properties inside that text?  Does
> it really matter for visual-wrap-prefix-mode to look inside?

As I understand this bug, we just need to make sure that visual-wrap.el 
doesn't change the display property of any single/atomic span of text 
that xdisp.c will replace. Otherwise, xdisp.c will stop at every change 
to 'display' in turn, leading to duplicated images (or whatever we're 
displaying).

I think the most reliable/future-proof way to fix this would be work as 
much like xdisp.c as we can. From what I can tell, xdisp.c is using 
'next-single-char-property-change' for this: for 
replacing-display-props, 'handle_single_display_spec' calls 
'display_prop_end', which calls 'Fnext_single_char_property_change'.

That's what my patch is doing, though it's in Lisp, and it's erring on 
the side of caution with what display specs could be "unsafe".

> IOW, I prefer simplicity of the logic to sophisticated design based on
> what we actually see Emacs do in tricky cases, because time and again
> I learned the hard way that my mental models of what actually happens
> can be erroneous in the fine details.

I've attached a patch that isn't *exactly* what you suggested, but 
should add an extra layer of safety at the cost of possibly failing to 
add wrapping properties in when we should/could. In this version 
("be-extra-careful.patch"), whenever we find an unsafe display property 
at the end of a line, we just skip ahead until we find the first 
known-safe display property.

This is the best I could come up with without adding a lot of extra 
complexity to visual-wrap.el, which I think would have just made it more 
prone to mistakes. If you have any other ideas though, let me know.

I also attached the patch I prefer ("match-xdisp.patch"), which is just 
what I posted previously with some additional details in the comments. 
Would you feel better about this version if I could add regression tests 
that ensure xdisp.c does what we expect? In previous work on 
visual-wrap, I looked a bit at doing this, and I think it would probably 
work if we ran the tests in a GUI frame. That probably requires some 
additional scaffolding in the Makefiles, but if it helps improve our 
confidence in the display engine, I think it's worth my time to attempt it.