GNU bug report logs - #73600
31.0.50; Visual wrap prefix mode and image display

Previous Next

Full log

Message #56 received at 73600 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 73600 <at> debbugs.gnu.org, gautier <at> gautierponsinet.xyz
Subject: Re: bug#73600: 31.0.50; Visual wrap prefix mode and image display
Date: Sat, 19 Oct 2024 11:23:30 +0300

> Date: Sun, 13 Oct 2024 13:50:25 -0700
> Cc: 73600 <at> debbugs.gnu.org, gautier <at> gautierponsinet.xyz
> From: Jim Porter <jporterbugs <at> 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.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.