GNU bug report logs - #76018
31.0.50; wrap-prefix properties from visual-wrap-prefix-mode proliferate

Previous Next

Full log

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76018 <at> debbugs.gnu.org, Jim Porter <jporterbugs <at> gmail.com>,
 Po Lu <luangruo <at> yahoo.com>, kevin.legouguec <at> gmail.com
Subject: Re: bug#76018: 31.0.50; wrap-prefix properties from
 visual-wrap-prefix-mode proliferate
Date: Tue, 27 May 2025 09:51:36 -0400

> OK, let's see if Stefan (CC'ed) has comments.

First, I'll note that `visual-wrap.el` is forked from GNU ELPA's
`adaptive-wrap.el`.  Why was it renamed?
Why has there been no effort to keep the two in sync or to mark the other
one as dead?

> This makes visual-wrap-mode incompatible with any other feature that
> uses wrap-prefix, because all of those properties will be removed when
> you turn off the mode, right?  If so, this subtlety should be at least
> documented.

AFAIK this has always been the case.

> +** New function 'append-text-property'.

AKA `font-lock-append-text-property`.
We also have `add-face-text-property` for text-properties containing faces.

> @@ -165,7 +167,12 @@ visual-wrap--apply-to-line
>         position (pos-eol) 'wrap-prefix
>         (if (numberp next-line-prefix)
>             `(space :align-to (,next-line-prefix . width))
> -         next-line-prefix)))))
> +         next-line-prefix))
> +      ;; Make sure that when typing at the end of a line with
> +      ;; `wrap-prefix' set, we don't continue that property over
> +      ;; subsequent lines.  See bug#76018.
> +      (append-text-property position (pos-eol)
> +                            'rear-nonsticky '(wrap-prefix)))))

Why is this needed?  Normally, when you type at the end of the line, the
new text gets jit-locked, so it should get its `wrap-prefix` removed
before we consider whether to (re)add a `wrap-prefix`.

Oh, I think I see: the problem seems to be that Jim's commit
f70a6ea0ea86ef461e40d20664a75a92d02679ea removed the

	 ;; Remove any `wrap-prefix' property that might have been
	 ;; added earlier.  Otherwise, we end up with a string
	 ;; containing a `wrap-prefix' string containing a
	 ;; `wrap-prefix' string ...
	 (remove-text-properties
	  0 (length pfx) '(wrap-prefix) pfx)

which didn't fix just the part in the comment but also the "bleeding
property" problem.

That commit's message doesn't describe its change in much detail (it
spends more time talking about the cosmetic refactoring than about the
actual code changes), so it's hard to know for sure if it was an
accident or if there was a reason for that removal.

BTW, why does `visual-wrap--apply-to-line` take a POSITION argument
which is always (point)?  AFAICT we can drop this argument along with
the corresponding `save-excursion+goto-point`.

> +(defun visual-wrap--apply-to-line (position &optional remove)

That is probably not a good idea: we should always start by removing the
old property.

> @@ -290,7 +321,7 @@ visual-wrap-prefix-mode
>      (with-silent-modifications
>        (save-restriction
>          (widen)
> -        (remove-text-properties (point-min) (point-max) '(wrap-prefix nil))))))
> +        (visual-wrap-prefix-function (point-min) (point-max) t)))))

Better move the removal code to its own function which both this code
and `visual-wrap-prefix-function` can call (both times, unconditionally).


        Stefan

Previous Next

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