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

Previous Next

Full log

View this message in rfc822 format

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

On 5/27/2025 6:51 AM, Stefan Monnier via Bug reports for GNU Emacs, the 
Swiss army knife of text editors wrote:
> AKA `font-lock-append-text-property`.
> We also have `add-face-text-property` for text-properties containing faces.

Yeah, this is a generalization of those, though it doesn't convert 
scalars to lists. In this case, I needed something like those, but for 
the stickiness text properties.

> 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.

In that commit, the 'remove-text-properties' call actually got moved 
from 'visual-wrap-prefix-function' to 'visual-wrap--content-prefix', 
where it does the same thing as before, or at least attempts to. See here:

>     ;; If we want to repeat the first-line prefix on subsequent lines,
>     ;; return its string value.  However, we 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 prefix) '(wrap-prefix) prefix)

I haven't checked to be sure that the original code handles this 
differently, but since it originally only removed the 'wrap-prefix' from 
the *prefix* (see the '(length pfx)' bit in the first code snippet), I 
don't think it should do anything different from the current 
implementation...

> 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`.

Good point (no pun intended).

>> +(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).

For the removal function, are you imagining that we just delete all the 
relevant properties from a specified region of text? Currently, I've 
attempted to just run the "apply-to-line' code in reverse, but maybe 
that would result in problems depending on when/if JIT lock runs.

I'm a little worried that we'd clobber other modes' properties if we're 
not careful, but maybe it's better to err on the side of deletion so we 
don't have stray properties. If only we could convert all the properties 
to use overlays (I tried)...

Previous Next

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