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

From: Jim Porter <[email-masked]>
Date: Fri, 4 Oct 2024 12:45:10 -0700

On 10/3/2024 11:28 PM, Eli Zaretskii wrote:
> A.k.a. (bobp), right?  Since point is already at position.

Fixed. Since point is at position, I've modified everything in the 
function to use (point). That way, there's no risk of confusion if 
someone looks at this years later and wonders why some parts use "point" 
while others use "position".

> I think this is too radical.  There are 'display' properties that do
> not replace the text: (space PROPS), (height HEIGHT), (raise FACT),
> and a few others.  These should not prevent adding the wrapping
> properties, I think.  The challenge is to detect the presence of
> "allowed" properties and the absence of "forbidden" ones, since a
> 'display' property's spec could be a list or a vector of several
> separate values.  So the code will need to look inside the list/vector
> and examine each element.

I think in addition to that, it's allowed if the display property has 
changed at the beginning (or end) of the line. For example, if I were to 
use the 'image' spec to render an image in-line with some other text, 
then 'visual-wrap-prefix-mode' can set the wrap-prefix as usual with no 
issues. The simpler case is when the image is at the end:

   * Here is some text. [IMG]

That should get the wrap prefix. The reverse is also true, though it's 
more complex:

   [IMG] Here is some text.

A mode that does something like that should set 'adaptive-fill-function' 
to handle this case and return an appropriate fill-prefix. Then 
'visual-wrap-prefix-mode' would do the right thing. For example, maybe 
EWW could do this to support images for bullet points (I'm not sure this 
is something we *actually* want to do in EWW, but hopefully it serves as 
an example.)

To be on the safe side, I'm only marking 'height' and 'raise' as safe 
for now, but adding new safe specs should just be a matter of putting 
them in the list.

From d4d97008452893c0043d585a2730300142f0b32d Mon Sep 17 00:00:00 2001
From: Jim Porter <[email-masked]>
Date: Thu, 3 Oct 2024 17:24:18 -0700
Subject: [PATCH] Don't add visual-wrap-prefix properties to multi-line display
 specs

This makes sure we don't interfere with other display specs, e.g. for
images displayed in 'image-mode' (bug#73600).

* lisp/visual-wrap.el (visual-wrap--safe-display-specs): New variable.
(visual-wrap--display-property-safe-p): New function.
(visual-wrap--apply-to-line): Bail out if we're in the middle of an
unsafe multi-line display spec.
---
 lisp/visual-wrap.el | 45 ++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 5 deletions(-)

diff --git a/lisp/visual-wrap.el b/lisp/visual-wrap.el
index 76276c0f474..63d6510f2e7 100644
--- a/lisp/visual-wrap.el
+++ b/lisp/visual-wrap.el
@@ -73,6 +73,34 @@ visual-wrap--face-extend-p
         (face-extend-p face nil t)
       (face-background face nil t)))))
 
+(defvar visual-wrap--safe-display-specs
+  '(height raise)
+  "A list of display specs that don't interfere with wrap prefixes.")
+
+(defun visual-wrap--display-property-safe-p (position offset)
+  "Return non-nil if the display property at POSITION is \"safe\".
+A \"safe\" display property is one where all the display specs are
+members of `visual-wrap--safe-display-specs', and won't interfere with
+the additional text properties that `visual-wrap-prefix-mode' uses.
+
+OFFSET should be 1 or -1 and tells which direction to look to see if the
+display property has changed.  If it has changed, then the display
+property is always considered safe: since POSITION is at the
+beginning (or end) of that property, our additional text properties
+don't cause problems."
+  (let ((pos-limit (if (= offset 1) (point-max) (point-min)))
+        (display (get-char-property position 'display)))
+    (or (= position pos-limit)
+        (not display)
+        (not (eq (get-char-property (+ position offset) 'display) display))
+        (when (or (vectorp display) (listp display))
+          (unless (listp (car display)) (setq display (list display)))
+          (not (catch 'unsafe
+                 (mapc (lambda (spec)
+                         (unless (memq spec visual-wrap--safe-display-specs)
+                           (throw 'unsafe t)))
+                       display)))))))
+
 (defun visual-wrap--prefix-face (fcp _beg end)
   ;; If the fill-context-prefix already specifies a face, just use that.
   (cond ((get-text-property 0 'face fcp))
@@ -117,9 +145,16 @@ visual-wrap--apply-to-line
   "Apply visual-wrapping properties to the logical line starting at POSITION."
   (save-excursion
     (goto-char position)
-    (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))
                (next-line-prefix (visual-wrap--content-prefix
-                                  first-line-prefix position)))
+                                  first-line-prefix (point))))
       (when (numberp next-line-prefix)
         ;; Set a minimum width for the prefix so it lines up correctly
         ;; with subsequent lines.  Make sure not to do this past the end
@@ -127,12 +162,12 @@ visual-wrap--apply-to-line
         ;; potentially return a prefix longer than the current line in
         ;; the buffer.)
         (add-display-text-property
-         position (min (+ position (length first-line-prefix))
-                       (line-end-position))
+         (point) (min (+ (point) (length first-line-prefix))
+                      eol)
          'min-width `((,next-line-prefix . width))))
       (setq next-line-prefix (visual-wrap--adjust-prefix next-line-prefix))
       (put-text-property
-       position (line-end-position) 'wrap-prefix
+       (point) eol 'wrap-prefix
        (if (numberp next-line-prefix)
            `(space :align-to (,next-line-prefix . width))
          next-line-prefix)))))
-- 
2.25.1