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

From: Eli Zaretskii <[email-masked]>
Date: Sat, 12 Oct 2024 15:00:16 +0300

> Date: Sat, 5 Oct 2024 11:07:39 -0700
> Cc: [email-masked], [email-masked]
> From: Jim Porter <[email-masked]>
> 
> >> -    (when-let ((first-line-prefix (fill-match-adaptive-prefix))
> >> +    (when-let ((eol (pos-eol))
> >> +               ;; Don't add wrapping properties if we're in the middle
> >> +               ;; of an unsafe multi-line display spec.  (For example,
> >> +               ;; this could be an image containing a newline-byte being
> >> +               ;; displayed in `image-mode'; see bug#73600.)
> >> +               ((visual-wrap--display-property-safe-p (point) -1))
> >> +               ((visual-wrap--display-property-safe-p eol 1))
> >> +               (first-line-prefix (fill-match-adaptive-prefix))
> > 
> > I'm not sure I can reason about this logic.  I'd prefer the code to
> > completely skip text covered by "unsafe" display properties, because
> > reasoning about that text makes no sense: it will not show on display.
> > Specifically, for the "text" that is the bytes of the image file, I
> > don't know how to begin reasoning about newlines embedded in those
> > bytes.  Thus, it is hard for me to tell if I agree with the above
> > logic or not.
> 
> The idea here is this:
> 
> 1. If we have some display property that spans multiple lines (either
>     from the previous line onto our current line, or from the current
>     line onto the next), then we need to be careful and check the specs
>     of this multi-line display property.
> 
>    a. If the multi-line display prop is something safe (e.g. 'raise'), we
>       can proceed with adding our visual-wrap props.
> 
>    b. Otherwise, we can't safely add visual-wrap props, so we bail out.
> 
> 2. If no display property spans multiple lines, then any display props
>     on our line are *only* on our line, so we can add the visual-wrap
>     props without breaking the 'eq'-ness of any existing display props.
>     That's there to allow the behavior as shown in "image.html".
> 
> For the image-mode case, if we have one newline-byte, then we hit 
> condition (1): we have the same display property on both the first and 
> second lines. Then we examine each display spec, and find that there's 
> one that's not on the safe list (b), so we bail out.
> 
> I agree that this logic is fairly complex, and I'm happy to rearrange it 
> or add comments as needed once we're on the same page. I'm also ok with 
> just saying "any multi-line display properties are assumed to be unsafe" 
> for now. That's simpler, and we could add this extra logic later if 
> someone notices a specific issue.

I'd prefer if we had code that, when it detects an unsafe display
property, would completely skip all of it.  With the above code, I
fond it difficult to reason whether it sometimes considers newlines
inside unsafe properties.