bug#71605: 30.0.50; [PATCH] Support variable-width text in 'visual-wrap-prefix-mode'

[Eli Zaretskii <eliz@gnu.org>]

> Date: Mon, 17 Jun 2024 10:42:56 -0700
> Cc: 71605@debbugs.gnu.org
> From: Jim Porter <jporterbugs@gmail.com>
> 
> On 6/17/2024 4:37 AM, Eli Zaretskii wrote:
> >> Date: Sun, 16 Jun 2024 19:56:44 -0700
> >> From: Jim Porter <jporterbugs@gmail.com>
> >> The attached patch adds a display spec in this case so that the text
> >> lines up perfectly.
> > 
> > Can you explain the idea of the patch?  I don't think I understand why
> > you use '(space :width)' rather than '(space :align-to)'.
> 
> I tried using :align-to, and it doesn't seem to take effect for the 
> 'wrap-prefix' text property. I haven't looked closely at why that 
> doesn't work, but even if it did, I think it might make things more 
> complex than they already are.

What exactly did you try?  I might be misremembering, but I'm not
aware of any limitations wrt use of :align-to in wrap-prefix.  In
fact, the ELisp reference manual explicitly mentions :align-to in its
description of wrap-prefix.

> If I understand your :align-to suggestion, setting :align-to on 
> everything after the "* " bullet could work in theory, but I don't know 
> what value you could set there to make everything correct.

Some multiple of the width of the default font's representative
character, or of its average-width property?

> The 'face-change' idea could work, or here's another possibility: what 
> about using :relative-width?

:relative-width could work, but you'd need to make sure it takes the
width from a fixed character, otherwise different paragraphs won't
align the same.  The ELisp manual says:

  ‘:relative-width FACTOR’
       Specifies that the width of the stretch should be computed from the
       first character in the group of consecutive characters that have
       the same ‘display’ property.  The space width is the pixel width of
       that character, multiplied by FACTOR.


> If I set that correctly, then the pixel-size should adjust as the
> text scales. It wouldn't handle the case where the actual font
> changes though.

Why not?

> >> -@defun string-pixel-width string
> >> +@defun string-pixel-width string &optional buffer
> >>   This is a convenience function that uses @code{window-text-pixel-size}
> >> -to compute the width of @var{string} (in pixels).
> >> +to compute the width of @var{string} (in pixels).  If @var{buffer} is
> >> +non-@code{nil}, use the face remappings from that buffer when
> >> +determining the width (@pxref{Face Remapping}).
> > 
> > An alternative would be to provide a face to use.
> > 
> > In any case, using BUFFER only for face-remapping-alist is only a
> > small part of what a buffer can do to a string: there's the major mode
> > with its fontifications and whatnot.
> 
> Yeah, I'm not entirely happy with this BUFFER argument either. I don't 
> think we need to worry about fontification in this case though, since 
> you can pass in a fontified string.
> 
> Maybe this should take the face-remapping-alist directly? Or maybe we 
> should pass in a window?

If you can pass a window, you can use window-text-pixel-size instead.

> >> +(defun visual-wrap--adjust-display-width (fcp n)
> >> +  (when-let ((display (get-text-property 0 'display fcp))
> >> +             ((eq (car-safe display) 'space))
> > 
> > Doesn't this only work with very simple 'display' specs?  The 'space'
> > part could be in some place deep in the spec, not just the second
> > symbol.
> 
> Yeah, though the FCP argument is always the prefix we constructed, so we 
> know what the display-spec looks like if it's present.

Sure, but that means the code is fragile, and you need to comment
prominently that if the form of the display spec changes in the
future, this code will need to be adapted.