bug#73600: 31.0.50; Visual wrap prefix mode and image display

[Eli Zaretskii <eliz@gnu.org>]

> Date: Sun, 13 Oct 2024 13:50:25 -0700
> Cc: 73600@debbugs.gnu.org, gautier@gautierponsinet.xyz
> From: Jim Porter <jporterbugs@gmail.com>
> 
> 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 xdisp.c does to detect the beginning of a property.  But
is that what it does to find the end of the property which is being
processed?

> 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.

I think at this stage it is basically up to you which version to
install.  Adding tests is always welcome, of course, regardless of
which version is installed.

Thanks.