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

Previous Next

Package: emacs;

Reported by: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>

Date: Sun, 2 Feb 2025 17:51:02 UTC

Severity: normal

Found in version 31.0.50

Done: Jim Porter <jporterbugs <at> gmail.com>

To reply to this bug, email your comments to 76018 AT debbugs.gnu.org.
There is no need to reopen the bug first.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to jporterbugs <at> gmail.com, bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Sun, 02 Feb 2025 17:51:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Kévin Le Gouguec <kevin.legouguec <at> gmail.com>:
New bug report received and forwarded. Copy sent to jporterbugs <at> gmail.com, bug-gnu-emacs <at> gnu.org. (Sun, 02 Feb 2025 17:51:02 GMT) Full text and rfc822 format available.

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

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 31.0.50; wrap-prefix properties from visual-wrap-prefix-mode
 proliferate
Date: Sun, 02 Feb 2025 18:50:40 +0100
Heya!

To reproduce from emacs -Q:

    C-x b repro
    M-x visual-wrap-prefix-mode
    - first line of [words…]
    RET
    RET
    second line of [words…]
    C-o

Expectation: continuation lines for the "second line of [words…]" should
not have any indentation.
Observation: text on that second line has these properties:

    There are text properties here:
      fontified            t
      wrap-prefix          (space :align-to (2 . width))

(NB: "C-o" in the recipe ensures there is a character at point,
otherwise "C-u C-x =" shows nothing on account of being at EOB)

Denoting "hard spaces" (buffer text) with "·" and "visual spaces"
(visual-wrap-prefix-mode decoration) with " ", text thus wraps like
this:

    -·first·line·of·
      words·words

    second·line·of
      words·words

Not sure what the right fix is; having just learned about text property
"stickiness", I can at least work around the issue with…

    (push '(wrap-prefix . t) text-property-default-nonsticky)

… but I have not dogfooded this much yet.

Thanks for your time!


In GNU Emacs 31.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.43, cairo version 1.18.2) of 2025-01-18 built on amdahl30
Repository revision: 840057bb1bfc05a52519793c620d729688ea1d8f
Repository branch: master
Windowing system distributor 'SUSE LINUX', version 11.0.12401004
System Description: openSUSE Tumbleweed

Configured using:
 'configure --prefix=/home/peniblec/apps/.emacs.2025-01-18 --with-cairo
 --with-native-compilation=no --with-sqlite3 --with-xinput2'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS TREE_SITTER WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
  value of $LC_TIME: en_GB.UTF-8
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Mon, 03 Feb 2025 22:43:02 GMT) Full text and rfc822 format available.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>,
 76018 <at> debbugs.gnu.org
Subject: Re: bug#76018: 31.0.50; wrap-prefix properties from
 visual-wrap-prefix-mode proliferate
Date: Mon, 3 Feb 2025 14:41:58 -0800
On 2/2/2025 9:50 AM, Kévin Le Gouguec wrote:
> Expectation: continuation lines for the "second line of [words…]" should
> not have any indentation.
> Observation: text on that second line has these properties:
> 
>      There are text properties here:
>        fontified            t
>        wrap-prefix          (space :align-to (2 . width))

Do you know if this is a recent regression, or if it's been this way 
since visual-wrap-prefix-mode was merged into Emacs? I've made a bunch 
of changes to the mode relatively recently, but the wrap-prefix part is 
mostly the same as before, aside from some different methods to 
calculate the width.





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Tue, 04 Feb 2025 06:58:01 GMT) Full text and rfc822 format available.

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

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 76018 <at> debbugs.gnu.org
Subject: Re: bug#76018: 31.0.50; wrap-prefix properties from
 visual-wrap-prefix-mode proliferate
Date: Tue, 04 Feb 2025 07:57:48 +0100
Jim Porter <jporterbugs <at> gmail.com> writes:

> On 2/2/2025 9:50 AM, Kévin Le Gouguec wrote:
>> Expectation: continuation lines for the "second line of [words…]" should
>> not have any indentation.
>> Observation: text on that second line has these properties:
>>      There are text properties here:
>>        fontified            t
>>        wrap-prefix          (space :align-to (2 . width))
>
> Do you know if this is a recent regression, or if it's been this way since visual-wrap-prefix-mode was merged into Emacs? I've made a bunch of changes to the mode relatively recently, but the wrap-prefix part is mostly the same as before, aside from some different methods to calculate the width.

Barring methodological errors, a smoketest using adaptive-wrap (0.8)
suggests that it did not have this bug; the text properties on the
second line are:

    There are text properties here:
      fontified            t
      wrap-prefix          ""

That's all I have for now; can try to bisect later!




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Tue, 04 Feb 2025 19:38:01 GMT) Full text and rfc822 format available.

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

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 76018 <at> debbugs.gnu.org
Subject: Re: bug#76018: 31.0.50; wrap-prefix properties from
 visual-wrap-prefix-mode proliferate
Date: Tue, 04 Feb 2025 20:37:17 +0100
Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:

> Jim Porter <jporterbugs <at> gmail.com> writes:
>
>> On 2/2/2025 9:50 AM, Kévin Le Gouguec wrote:
>>> Expectation: continuation lines for the "second line of [words…]" should
>>> not have any indentation.
>>> Observation: text on that second line has these properties:
>>>      There are text properties here:
>>>        fontified            t
>>>        wrap-prefix          (space :align-to (2 . width))
>>
>> Do you know if this is a recent regression, or if it's been this way since visual-wrap-prefix-mode was merged into Emacs? I've made a bunch of changes to the mode relatively recently, but the wrap-prefix part is mostly the same as before, aside from some different methods to calculate the width.
>
> Barring methodological errors, a smoketest using adaptive-wrap (0.8)
> suggests that it did not have this bug; the text properties on the
> second line are:
>
>     There are text properties here:
>       fontified            t
>       wrap-prefix          ""
>
> That's all I have for now; can try to bisect later!

After checking that 2024-01-27 "Import ELPA package adaptive-wrap as
visual-wrap" (6667d6c19c3) is "good" too, I got a bisection going -
testing each step after 'make bootstrap'.  Results:

    $ git bisect start HEAD 6667d6c19c3934871ed54d89dc153efc72f947de -- lisp/visual-wrap.el
    Bisecting: 7 revisions left to test after this (roughly 3 steps)
    [b86bc02096c65517b9a29c20635ece100864fc62] Introduce a global variant of visual-wrap-prefix-mode

    $ git bisect good
    Bisecting: 3 revisions left to test after this (roughly 2 steps)
    [55aad592e177dc2c503ebe9ad2a46e227683315e] Improve computation of indent depth in SHR and 'visual-wrap-prefix-mode'

    $ git bisect bad
    Bisecting: 1 revision left to test after this (roughly 1 step)
    [135da3556bb34bb20a01e02b30bc949c1a45b6cd] Be more careful about aligning prefix lines in 'visual-wrap-prefix-mode'

    $ git bisect bad
    Bisecting: 0 revisions left to test after this (roughly 0 steps)
    [f70a6ea0ea86ef461e40d20664a75a92d02679ea] Add support for variable-pitch fonts in 'visual-wrap-prefix-mode'

    $ git bisect bad
    f70a6ea0ea86ef461e40d20664a75a92d02679ea is the first bad commit
    commit f70a6ea0ea86ef461e40d20664a75a92d02679ea (HEAD)
    Author: Jim Porter <jporterbugs <at> gmail.com>
    Date:   Sat Jul 27 20:48:38 2024 -0700

        Add support for variable-pitch fonts in 'visual-wrap-prefix-mode'

        * lisp/emacs-lisp/subr-x.el (string-pixel-width): Allow passing BUFFER
        to use the face remappings from that buffer when calculating the width.

        * lisp/visual-wrap.el (visual-wrap--prefix): Rename to...
        (visual-wrap--adjust-prefix): ... this, and support PREFIX as a number.
        (visual-wrap-fill-context-prefix): Make obsolete in favor of...
        (visual-wrap--content-prefix): ... this.
        (visual-wrap-prefix-function): Extract inside of loop into...
        (visual-wrap--apply-to-line): ... this.

        * doc/lispref/display.texi (Size of Displayed Text): Update
        documentation for 'string-pixel-width'.

        * etc/NEWS: Announce this change.

     doc/lispref/display.texi  |   6 +++--
     etc/NEWS                  |  12 +++++++++
     lisp/emacs-lisp/subr-x.el |  11 +++++++--
     lisp/visual-wrap.el       | 113 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------
     4 files changed, 102 insertions(+), 40 deletions(-)

Have not delved into the diff yet.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Tue, 27 May 2025 00:12:01 GMT) Full text and rfc822 format available.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>,
 76018 <at> debbugs.gnu.org, eliz <at> gnu.org
Subject: Re: bug#76018: 31.0.50; wrap-prefix properties from
 visual-wrap-prefix-mode proliferate
Date: Mon, 26 May 2025 17:11:15 -0700
[Message part 1 (text/plain, inline)]
On 2/2/2025 9:50 AM, Kévin Le Gouguec wrote:
> Heya!
> 
> To reproduce from emacs -Q:
> 
>      C-x b repro
>      M-x visual-wrap-prefix-mode
>      - first line of [words…]
>      RET
>      RET
>      second line of [words…]
>      C-o
> 
> Expectation: continuation lines for the "second line of [words…]" should
> not have any indentation.
> Observation: text on that second line has these properties:
> 
>      There are text properties here:
>        fontified            t
>        wrap-prefix          (space :align-to (2 . width))
> 
> (NB: "C-o" in the recipe ensures there is a character at point,
> otherwise "C-u C-x =" shows nothing on account of being at EOB)
> 
> Denoting "hard spaces" (buffer text) with "·" and "visual spaces"
> (visual-wrap-prefix-mode decoration) with " ", text thus wraps like
> this:
> 
>      -·first·line·of·
>        words·words
> 
>      second·line·of
>        words·words
> 
> Not sure what the right fix is; having just learned about text property
> "stickiness", I can at least work around the issue with…
> 
>      (push '(wrap-prefix . t) text-property-default-nonsticky)
> 
> … but I have not dogfooded this much yet.

Attached is a patch with a test for this, relying on the 
'rear-nonsticky' text property.

Eli, what do you think? I've also moved an internal Eshell utility 
function ('eshell--append-text-property') to subr-x.el, since it would 
be useful for the visual-wrap code. I've updated the manual and NEWS to 
mention this function as well so that others can use it if they like.
[0001-Make-wrap-prefix-nonsticky-when-using-visual-wrap-pr.patch (text/plain, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Tue, 27 May 2025 01:31:01 GMT) Full text and rfc822 format available.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>,
 76018 <at> debbugs.gnu.org, eliz <at> gnu.org
Subject: Re: bug#76018: 31.0.50; wrap-prefix properties from
 visual-wrap-prefix-mode proliferate
Date: Mon, 26 May 2025 18:30:34 -0700
[Message part 1 (text/plain, inline)]
On 5/26/2025 5:11 PM, Jim Porter wrote:
> Eli, what do you think? I've also moved an internal Eshell utility 
> function ('eshell--append-text-property') to subr-x.el, since it would 
> be useful for the visual-wrap code. I've updated the manual and NEWS to 
> mention this function as well so that others can use it if they like.

After some further thought, I noticed that 'visual-wrap-prefix-mode' 
doesn't clean up after itself properly. Attached is a fix, taking 
advantage of a new function called 'remove-display-text-properties', 
which does what you might expect.
[0001-Make-wrap-prefix-nonsticky-when-using-visual-wrap-pr.patch (text/plain, attachment)]
[0002-Properly-clean-up-after-deactivating-visual-wrap-pre.patch (text/plain, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Tue, 27 May 2025 06:56:01 GMT) Full text and rfc822 format available.

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

From: Juri Linkov <juri <at> linkov.net>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 76018 <at> debbugs.gnu.org, eliz <at> gnu.org,
 Kévin Le Gouguec <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:49:34 +0300
> +@defun append-text-property start end prop values &optional object
> +This function appends a list of @var{values} to the @var{prop} property
> +for the text between @var{start} and @var{end} in the string or buffer
> +@var{object}.  If @var{object} is @code{nil}, it defaults to the current
> +buffer.
> +@end defun

Shouldn't this explain a difference from 'add-text-properties'?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Tue, 27 May 2025 11:34:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 76018 <at> debbugs.gnu.org, 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 14:32:55 +0300
> Date: Mon, 26 May 2025 18:30:34 -0700
> From: Jim Porter <jporterbugs <at> gmail.com>
> 
> On 5/26/2025 5:11 PM, Jim Porter wrote:
> > Eli, what do you think? I've also moved an internal Eshell utility 
> > function ('eshell--append-text-property') to subr-x.el, since it would 
> > be useful for the visual-wrap code. I've updated the manual and NEWS to 
> > mention this function as well so that others can use it if they like.
> 
> After some further thought, I noticed that 'visual-wrap-prefix-mode' 
> doesn't clean up after itself properly. Attached is a fix, taking 
> advantage of a new function called 'remove-display-text-properties', 
> which does what you might expect.

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

> --- a/lisp/visual-wrap.el
> +++ b/lisp/visual-wrap.el
> @@ -33,6 +33,8 @@
>  
>  ;;; Code:
>  
> +(require 'text-property-search)

Why did you need this 'require'?

> +@lisp
> +(add-display-text-property 1 8 'raise 0.5)
> +(add-display-text-property 4 8 'height 2.0)
> +(remove-display-text-property 2 6 'raise)
> +@end lisp

The lines inside @lisp..@end lisp should be in @group..@end group, to
prevent them from being split between pages in the printed version of
the manual.

> +      (if (not remove)
> +          ;; Apply `wrap-prefix' properties.
> +          (progn
> +            (put-text-property
> +             position (pos-eol) 'wrap-prefix
> +             (if (numberp next-line-prefix)
> +                 `(space :align-to (,next-line-prefix . width))
> +               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)))
> +        ;; Remove `wrap-prefix' properties.
> +        (remove-text-properties position (pos-eol) '(wrap-prefix nil))
> +        (visual-wrap--remq-text-property position (pos-eol)
> +                                         'rear-nonsticky 'wrap-prefix)))))

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.

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Tue, 27 May 2025 13:52:02 GMT) Full text and rfc822 format available.

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





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Tue, 27 May 2025 17:04:02 GMT) Full text and rfc822 format available.

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

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: Re: 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)...




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Tue, 27 May 2025 20:24:02 GMT) Full text and rfc822 format available.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: Po Lu <luangruo <at> yahoo.com>, 76018 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, 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 16:23:48 -0400
>> 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.

FWIW, I'd welcome a more coherent set of primitives.

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

Ah, I see.

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

Oh, indeed that `remove-text-properties` was applied to the string, not
to the buffer.  Duh!

> For the removal function, are you imagining that we just delete all the
> relevant properties from a specified region of text?

Traditionally jit-lock functions start by removing the property they
manage over the whole region that they're about to process.
E.g. in `adaptive-wrap.el` I think the patch below is missing.
For `visual-wrap.el` we additionally need to remove the `display`
properties we set up, which can be a fair bit more difficult (unless we
decide to just remove all `display` properties).

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

So far, we generally don't try to interact properly with other packages
using the same property.  It's usually good enough.
In the present case, our minor mode tends to place a `wrap-prefix`
property over pretty much the hole buffer anyway, so there's not much
room for another package to use that property without conflict.


        Stefan


diff --git a/adaptive-wrap.el b/adaptive-wrap.el
index e5b906f892..140e50293e 100644
--- a/adaptive-wrap.el
+++ b/adaptive-wrap.el
@@ -1,6 +1,6 @@
 ;;; adaptive-wrap.el --- Smart line-wrapping with wrap-prefix  -*- lexical-binding: t; -*-
 
-;; Copyright (C) 2011-2021  Free Software Foundation, Inc.
+;; Copyright (C) 2011-2025  Free Software Foundation, Inc.
 
 ;; Author: Stephen Berman <stephen.berman <at> gmx.net>
 ;;         Stefan Monnier <monnier <at> iro.umontreal.ca>
@@ -136,6 +136,7 @@ How much to adjust it is decided by `adaptive-wrap-extra-indent'."
   (goto-char beg)
   (forward-line 0)
   (setq beg (point))
+  (remove-text-properties beg end '(wrap-prefix))
   (while (< (point) end)
     (let ((lbp (point)))
       (put-text-property





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Wed, 28 May 2025 01:06:02 GMT) Full text and rfc822 format available.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Po Lu <luangruo <at> yahoo.com>, 76018 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, 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 18:05:14 -0700
[Message part 1 (text/plain, inline)]
On 5/27/2025 1:23 PM, Stefan Monnier via Bug reports for GNU Emacs, the 
Swiss army knife of text editors wrote:
> So far, we generally don't try to interact properly with other packages
> using the same property.  It's usually good enough.
> In the present case, our minor mode tends to place a `wrap-prefix`
> property over pretty much the hole buffer anyway, so there's not much
> room for another package to use that property without conflict.

How about something like this? This should also address Juri and Eli's 
comments elsewhere in the bug.

One remaining bit that might be valuable is adding a few regression 
tests for 'append-text-property' and 'remove-display-text-property'.
[0001-Make-wrap-prefix-nonsticky-when-using-visual-wrap-pr.patch (text/plain, attachment)]
[0002-Properly-clean-up-after-deactivating-visual-wrap-pre.patch (text/plain, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Wed, 28 May 2025 14:47:02 GMT) Full text and rfc822 format available.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: Po Lu <luangruo <at> yahoo.com>, 76018 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, kevin.legouguec <at> gmail.com
Subject: Re: bug#76018: 31.0.50; wrap-prefix properties from
 visual-wrap-prefix-mode proliferate
Date: Wed, 28 May 2025 10:46:03 -0400
> --- a/lisp/visual-wrap.el
> +++ b/lisp/visual-wrap.el
> @@ -165,7 +165,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)))))

Using the `rear-nonsticky` or `front-sticky` properties is a PITA with
lots of odd corner cases, so I'd much rather we try to use the
standard approach of removing all our properties at start.

We have to write the "remove" code for the case where we turn the mode
off anyway.


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Wed, 28 May 2025 17:20:02 GMT) Full text and rfc822 format available.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Po Lu <luangruo <at> yahoo.com>, 76018 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, kevin.legouguec <at> gmail.com
Subject: Re: bug#76018: 31.0.50; wrap-prefix properties from
 visual-wrap-prefix-mode proliferate
Date: Wed, 28 May 2025 10:19:15 -0700
[Message part 1 (text/plain, inline)]
On 5/28/2025 7:46 AM, Stefan Monnier via Bug reports for GNU Emacs, the 
Swiss army knife of text editors wrote:
> Using the `rear-nonsticky` or `front-sticky` properties is a PITA with
> lots of odd corner cases, so I'd much rather we try to use the
> standard approach of removing all our properties at start.

Ok, I removed that. That means we don't need to add 
'append-text-property' just yet, though it would probably be a useful 
function to add one day.

I also wrote some tests for 'remove-display-text-property'. In the 
process of doing so, I found a nasty bug in the existing implementation 
(fixed in the first patch).
[0001-Don-t-delete-in-place-when-replacing-a-display-prope.patch (text/plain, attachment)]
[0002-Clean-up-text-properties-in-visual-wrap-prefix-mode.patch (text/plain, attachment)]
[0003-Remove-superfluous-POSITION-argument-from-visual-wra.patch (text/plain, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Thu, 29 May 2025 07:39:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: luangruo <at> yahoo.com, 76018 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 kevin.legouguec <at> gmail.com
Subject: Re: bug#76018: 31.0.50; wrap-prefix properties from
 visual-wrap-prefix-mode proliferate
Date: Thu, 29 May 2025 10:37:51 +0300
> Date: Wed, 28 May 2025 10:19:15 -0700
> Cc: Po Lu <luangruo <at> yahoo.com>, 76018 <at> debbugs.gnu.org,
>  Eli Zaretskii <eliz <at> gnu.org>, kevin.legouguec <at> gmail.com
> From: Jim Porter <jporterbugs <at> gmail.com>
> 
> +@lisp
> +(add-display-text-property 1 8 'raise 0.5)
> +(add-display-text-property 4 8 'height 2.0)
> +(remove-display-text-property 2 6 'raise)
> +@end lisp

Please use @group unless you don't care if these lines are split
between pages in the printed version of the manuals.

> +@defun remove-display-text-property start end prop &optional object
> +Remove @code{display} property @var{prop} from the text from @var{start}
> +to @var{end}.
> +
> +If any text in the region has any other @code{display} properties, those
> +properties are retained.  For instance:

I think this (and the doc string of the function) is confusing,
because it is not clear what you mean by "'display' property PROP" vs
"other 'display' properties".  The example seems to suggest that PROP
is the car of the value of the 'display' property.  Tha tis, if the
property's value is '(space . PROPS)', then one needs to call this
function with 'space' as PROP, and the same for 'image' etc.  This
should be spelled out.  And even after this is spelled out, there are
questions left that beg their answers:

 . what about 'display' property whose value is a string?
 . what about 'display' property whose value is '((margin nil) STRING)'?

Please add enough explanations to answer these questions at least in
the manual.

> ++++
> +** New function 'remove-display-text-property'.
> +This function removes a display property from the specified region of
> +text, preserving any other display properties already set for that
> +region.

This should say something like

  This function removes a specific kind of 'display' property from the
  specified region of text, [...]

because "a display property" is too vague.

> +(defun add-display-text-property (start end prop value
> +                                        &optional object)
> +  "Add display property PROP with VALUE to the text from START to END.
> +If any text in the region has a non-nil `display' property, those
> +properties are retained.

But they could be split into two separate stretches, right?

Also, I think the same potential confusion with PROP can happen here,
so the doc string should be made clearer in that regard.

> +If OBJECT is non-nil, it should be a string or a buffer.  If nil,
> +this defaults to the current buffer."

This is better written as

  OBJECT is either a string or a buffer whose text should have the
  property added, and defaults to the current buffer.

> +(defun remove-display-text-property (start end prop &optional object)
> +  "Remove display property PROP from the text from START to END.
> +If any text in the region has other `display' specs, those specs are
> +retained.

See the comments above about this "removal" and the meaning of PROP.

> +If OBJECT is non-nil, it should be a string or a buffer.  If nil,
> +this defaults to the current buffer."

See the comment about similar doc-string text above.

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Sat, 31 May 2025 05:00:04 GMT) Full text and rfc822 format available.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: luangruo <at> yahoo.com, 76018 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 kevin.legouguec <at> gmail.com
Subject: Re: bug#76018: 31.0.50; wrap-prefix properties from
 visual-wrap-prefix-mode proliferate
Date: Fri, 30 May 2025 21:59:40 -0700
On 5/29/2025 12:37 AM, Eli Zaretskii wrote:
>> Date: Wed, 28 May 2025 10:19:15 -0700
>> Cc: Po Lu <luangruo <at> yahoo.com>, 76018 <at> debbugs.gnu.org,
>>   Eli Zaretskii <eliz <at> gnu.org>, kevin.legouguec <at> gmail.com
>> From: Jim Porter <jporterbugs <at> gmail.com>
>>
>> +@lisp
>> +(add-display-text-property 1 8 'raise 0.5)
>> +(add-display-text-property 4 8 'height 2.0)
>> +(remove-display-text-property 2 6 'raise)
>> +@end lisp
> 
> Please use @group unless you don't care if these lines are split
> between pages in the printed version of the manuals.

Whoops, sorry about missing that. The @group got lost when I rewrote my 
patches.

>> +If any text in the region has any other @code{display} properties, those
>> +properties are retained.  For instance:
> 
> I think this (and the doc string of the function) is confusing,
> because it is not clear what you mean by "'display' property PROP" vs
> "other 'display' properties".  The example seems to suggest that PROP
> is the car of the value of the 'display' property.  Tha tis, if the
> property's value is '(space . PROPS)', then one needs to call this
> function with 'space' as PROP, and the same for 'image' etc.  This
> should be spelled out.

This is something that confuses me too. Is there a term we should use 
for 'space' or 'image'? I know that '(space . PROPS)' is a "display 
specification", and the 'display' text property can be a display 
specification (or a list or vector of display specs). But it seems 
there's no term for the car of the display spec. Maybe "the car of a 
display spec" would work? Currently, the manual entry for 
'get-display-property' calls it a "specific display property" or just 
"'display' property". I'm open to suggestions here.

> And even after this is spelled out, there are
> questions left that beg their answers:
> 
>   . what about 'display' property whose value is a string?

Currently (including before my patch), 'add-display-text-property' 
doesn't support a string value here. I'll add documentation to that effect.

>   . what about 'display' property whose value is '((margin nil) STRING)'?

This should work fine, since we compare the car of the display spec 
using 'equal'.

>> +(defun add-display-text-property (start end prop value
>> +                                        &optional object)
>> +  "Add display property PROP with VALUE to the text from START to END.
>> +If any text in the region has a non-nil `display' property, those
>> +properties are retained.
> 
> But they could be split into two separate stretches, right?

Not for this function ('add-display-text-property'), but for 
'remove-display-text-property', this could split the property in two: 
one before and one after the specified region. That's similar to 
'remove-text-properties'. I tried to show the details of all that in the 
manual entry.

>> +If OBJECT is non-nil, it should be a string or a buffer.  If nil,
>> +this defaults to the current buffer."
> 
> This is better written as
> 
>    OBJECT is either a string or a buffer whose text should have the
>    property added, and defaults to the current buffer.

Sounds good to me.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Sat, 31 May 2025 07:21:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: luangruo <at> yahoo.com, 76018 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 kevin.legouguec <at> gmail.com
Subject: Re: bug#76018: 31.0.50; wrap-prefix properties from
 visual-wrap-prefix-mode proliferate
Date: Sat, 31 May 2025 10:20:09 +0300
> Date: Fri, 30 May 2025 21:59:40 -0700
> Cc: luangruo <at> yahoo.com, 76018 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
>  kevin.legouguec <at> gmail.com
> From: Jim Porter <jporterbugs <at> gmail.com>
> 
> >> +If any text in the region has any other @code{display} properties, those
> >> +properties are retained.  For instance:
> > 
> > I think this (and the doc string of the function) is confusing,
> > because it is not clear what you mean by "'display' property PROP" vs
> > "other 'display' properties".  The example seems to suggest that PROP
> > is the car of the value of the 'display' property.  Tha tis, if the
> > property's value is '(space . PROPS)', then one needs to call this
> > function with 'space' as PROP, and the same for 'image' etc.  This
> > should be spelled out.
> 
> This is something that confuses me too. Is there a term we should use 
> for 'space' or 'image'? I know that '(space . PROPS)' is a "display 
> specification", and the 'display' text property can be a display 
> specification (or a list or vector of display specs). But it seems 
> there's no term for the car of the display spec. Maybe "the car of a 
> display spec" would work? Currently, the manual entry for 
> 'get-display-property' calls it a "specific display property" or just 
> "'display' property". I'm open to suggestions here.

Yes, "the car of the display spec" will work, I think.

> >   . what about 'display' property whose value is '((margin nil) STRING)'?
> 
> This should work fine, since we compare the car of the display spec 
> using 'equal'.

So that kind of property is also not supported?

> >> +(defun add-display-text-property (start end prop value
> >> +                                        &optional object)
> >> +  "Add display property PROP with VALUE to the text from START to END.
> >> +If any text in the region has a non-nil `display' property, those
> >> +properties are retained.
> > 
> > But they could be split into two separate stretches, right?
> 
> Not for this function ('add-display-text-property'), but for 
> 'remove-display-text-property', this could split the property in two: 
> one before and one after the specified region. That's similar to 
> 'remove-text-properties'. I tried to show the details of all that in the 
> manual entry.

I think the add function could also split.  Consider text which has
some display property between positions A and B, and then you call
add-display-text-property to add a display property with another spec
between START > A and END < B.  The result will be text with the
original display property between A and START, the new property plus
the old one between A and END, and a copy of the original property
between END and B.  Right?

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Sat, 31 May 2025 17:49:03 GMT) Full text and rfc822 format available.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: luangruo <at> yahoo.com, 76018 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 kevin.legouguec <at> gmail.com
Subject: Re: bug#76018: 31.0.50; wrap-prefix properties from
 visual-wrap-prefix-mode proliferate
Date: Sat, 31 May 2025 10:48:45 -0700
On 5/31/2025 12:20 AM, Eli Zaretskii wrote:
>> Date: Fri, 30 May 2025 21:59:40 -0700
>> Cc: luangruo <at> yahoo.com, 76018 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
>>   kevin.legouguec <at> gmail.com
>> From: Jim Porter <jporterbugs <at> gmail.com>
>>
>>>    . what about 'display' property whose value is '((margin nil) STRING)'?
>>
>> This should work fine, since we compare the car of the display spec
>> using 'equal'.
> 
> So that kind of property is also not supported?

It *should* be supported. However, it turns out that 
'add-display-text-property' doesn't work right with '(margin nil)'. I 
think the first step here should be to fix all the bugs I've found with 
'add-display-text-property', and then I'll work on finishing up the rest.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Sat, 31 May 2025 18:07:03 GMT) Full text and rfc822 format available.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: luangruo <at> yahoo.com, 76018 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 kevin.legouguec <at> gmail.com
Subject: Re: bug#76018: 31.0.50; wrap-prefix properties from
 visual-wrap-prefix-mode proliferate
Date: Sat, 31 May 2025 11:06:24 -0700
[Message part 1 (text/plain, inline)]
On 5/31/2025 10:48 AM, Jim Porter wrote:
> It *should* be supported. However, it turns out that 
> 'add-display-text-property' doesn't work right with '(margin nil)'. I 
> think the first step here should be to fix all the bugs I've found with 
> 'add-display-text-property', and then I'll work on finishing up the rest.

Ok, that was simpler than I thought. Eli, what do you think about adding 
this patch to the Emacs 30 branch? It just fixes some bugs in 
'add-display-text-property', as described in the commit message.
[0001-Don-t-delete-in-place-when-replacing-a-display-prope.patch (text/plain, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Sun, 01 Jun 2025 04:52:07 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: luangruo <at> yahoo.com, 76018 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 kevin.legouguec <at> gmail.com
Subject: Re: bug#76018: 31.0.50; wrap-prefix properties from
 visual-wrap-prefix-mode proliferate
Date: Sun, 01 Jun 2025 07:50:42 +0300
> Date: Sat, 31 May 2025 10:48:45 -0700
> Cc: luangruo <at> yahoo.com, 76018 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
>  kevin.legouguec <at> gmail.com
> From: Jim Porter <jporterbugs <at> gmail.com>
> 
> On 5/31/2025 12:20 AM, Eli Zaretskii wrote:
> >> Date: Fri, 30 May 2025 21:59:40 -0700
> >> Cc: luangruo <at> yahoo.com, 76018 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
> >>   kevin.legouguec <at> gmail.com
> >> From: Jim Porter <jporterbugs <at> gmail.com>
> >>
> >>>    . what about 'display' property whose value is '((margin nil) STRING)'?
> >>
> >> This should work fine, since we compare the car of the display spec
> >> using 'equal'.
> > 
> > So that kind of property is also not supported?
> 
> It *should* be supported.

Then I don't understand how your comment about using 'equal' answers
my question, which was what should be passed to
remove-display-property as its PROP argument to remove properties of
the form ((margin nil) STRING)'?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Sun, 01 Jun 2025 04:59:07 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: luangruo <at> yahoo.com, 76018 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 kevin.legouguec <at> gmail.com
Subject: Re: bug#76018: 31.0.50; wrap-prefix properties from
 visual-wrap-prefix-mode proliferate
Date: Sun, 01 Jun 2025 07:58:30 +0300
> Date: Sat, 31 May 2025 11:06:24 -0700
> From: Jim Porter <jporterbugs <at> gmail.com>
> Cc: luangruo <at> yahoo.com, 76018 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
>  kevin.legouguec <at> gmail.com
> 
> On 5/31/2025 10:48 AM, Jim Porter wrote:
> > It *should* be supported. However, it turns out that 
> > 'add-display-text-property' doesn't work right with '(margin nil)'. I 
> > think the first step here should be to fix all the bugs I've found with 
> > 'add-display-text-property', and then I'll work on finishing up the rest.
> 
> Ok, that was simpler than I thought. Eli, what do you think about adding 
> this patch to the Emacs 30 branch? It just fixes some bugs in 
> 'add-display-text-property', as described in the commit message.

Not sure we should make such changes on the release branch.
Especially since I don't understand the issue very well: what do you
mean by "list came directly from the property value" and by "used by
other regions of text"?

> When calling 'add-display-property' on a region of text that already
                ^^^^^^^^^^^^^^^^^^^^
Typo: that's not the correct name of the function.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Sun, 01 Jun 2025 19:25:02 GMT) Full text and rfc822 format available.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: luangruo <at> yahoo.com, 76018 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 kevin.legouguec <at> gmail.com
Subject: Re: bug#76018: 31.0.50; wrap-prefix properties from
 visual-wrap-prefix-mode proliferate
Date: Sun, 1 Jun 2025 12:24:36 -0700
On 5/31/2025 9:58 PM, Eli Zaretskii wrote:
>> Ok, that was simpler than I thought. Eli, what do you think about adding
>> this patch to the Emacs 30 branch? It just fixes some bugs in
>> 'add-display-text-property', as described in the commit message.
> 
> Not sure we should make such changes on the release branch.

That's alright. I don't have a preference either way.

> Especially since I don't understand the issue very well: what do you
> mean by "list came directly from the property value" and by "used by
> other regions of text"?

I've reworded the commit message to explain things better (I hope):

  When calling 'add-display-text-property' on a region of text that
  already contains PROP, we delete the old property from the list of
  values.  If the region's 'display' property is a list of display
  specifications, we need to avoid destructively modifying the list; other
  regions of text could be using the same list object.  Instead, if
  deleting a property from a list, use 'remove'.  (For a 'display'
  properties that's a vector or a single display spec, this doesn't matter
  since we first make a new list in the code.)

You could see this issue in the old code by doing this:

  (with-temp-buffer
    (erase-buffer)
    (insert "abcd")
    (put-text-property 1 4 'display '((space-width 2) (height 2.0)))
    (add-display-text-property 2 3 'height 1.0)
    (message "%S" (buffer-string)))

This *should* produce the following:

  #("abcd"
    0 1 (display ((space-width 2) (height 2.0)))
    1 2 (display ((height 1.0) (space-width 2)))
    2 3 (display ((space-width 2) (height 2.0))))

but it actually produces this:

  #("abcd"
    0 1 (display ((space-width 2)))
    1 2 (display ((height 1.0) (space-width 2)))
    2 3 (display ((space-width 2))))

That's because the call to 'delete' in 'add-display-text-property' 
destructively modifies the original list object, so the buffer regions 
from 1 to 2 and 3 to 4 end up without the '(height 2.0)' display spec.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Sun, 01 Jun 2025 19:38:02 GMT) Full text and rfc822 format available.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: luangruo <at> yahoo.com, 76018 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 kevin.legouguec <at> gmail.com
Subject: Re: bug#76018: 31.0.50; wrap-prefix properties from
 visual-wrap-prefix-mode proliferate
Date: Sun, 1 Jun 2025 12:37:42 -0700
On 5/31/2025 9:50 PM, Eli Zaretskii wrote:
> Then I don't understand how your comment about using 'equal' answers
> my question, which was what should be passed to
> remove-display-property as its PROP argument to remove properties of
> the form ((margin nil) STRING)'?

There was a bug in this code, but with that fixed, you would call it 
like so:

  (remove-display-text-property start end '(margin nil))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Mon, 02 Jun 2025 06:00:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: luangruo <at> yahoo.com, 76018 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 kevin.legouguec <at> gmail.com
Subject: Re: bug#76018: 31.0.50; wrap-prefix properties from
 visual-wrap-prefix-mode proliferate
Date: Mon, 02 Jun 2025 08:58:27 +0300
> Date: Sun, 1 Jun 2025 12:24:36 -0700
> Cc: luangruo <at> yahoo.com, 76018 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
>  kevin.legouguec <at> gmail.com
> From: Jim Porter <jporterbugs <at> gmail.com>
> 
> I've reworded the commit message to explain things better (I hope):
> 
>    When calling 'add-display-text-property' on a region of text that
>    already contains PROP, we delete the old property from the list of
>    values.  If the region's 'display' property is a list of display
>    specifications, we need to avoid destructively modifying the list; other
>    regions of text could be using the same list object.  Instead, if
>    deleting a property from a list, use 'remove'.  (For a 'display'
>    properties that's a vector or a single display spec, this doesn't matter
>    since we first make a new list in the code.)

This is more clear, but please use consistent terminology.  In the
above, you use "list of values" and "list of display specifications"
to mean the same thing (AFAIU), and "property" to mean "display
specification".

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Mon, 02 Jun 2025 06:01:01 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: luangruo <at> yahoo.com, 76018 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 kevin.legouguec <at> gmail.com
Subject: Re: bug#76018: 31.0.50; wrap-prefix properties from
 visual-wrap-prefix-mode proliferate
Date: Mon, 02 Jun 2025 08:59:35 +0300
> Date: Sun, 1 Jun 2025 12:37:42 -0700
> Cc: luangruo <at> yahoo.com, 76018 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
>  kevin.legouguec <at> gmail.com
> From: Jim Porter <jporterbugs <at> gmail.com>
> 
> On 5/31/2025 9:50 PM, Eli Zaretskii wrote:
> > Then I don't understand how your comment about using 'equal' answers
> > my question, which was what should be passed to
> > remove-display-property as its PROP argument to remove properties of
> > the form ((margin nil) STRING)'?
> 
> There was a bug in this code, but with that fixed, you would call it 
> like so:
> 
>    (remove-display-text-property start end '(margin nil))

Right, and this should be documented.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Tue, 03 Jun 2025 00:50:02 GMT) Full text and rfc822 format available.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: luangruo <at> yahoo.com, 76018 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 kevin.legouguec <at> gmail.com
Subject: Re: bug#76018: 31.0.50; wrap-prefix properties from
 visual-wrap-prefix-mode proliferate
Date: Mon, 2 Jun 2025 17:49:07 -0700
[Message part 1 (text/plain, inline)]
On 6/1/2025 10:58 PM, Eli Zaretskii wrote:
> This is more clear, but please use consistent terminology.  In the
> above, you use "list of values" and "list of display specifications"
> to mean the same thing (AFAIU), and "property" to mean "display
> specification".

Good catch. Now done in this patch series. I've also added a patch 
(0002) to improve the wording of the existing documentation, and 
hopefully finished up everything for fixing the original bug.

For patch 0002, I debated changing the various function names to 
'get-display-specification' and 'add-display-specification', but I left 
them as-is for now. If you think we should change them so the wording is 
more consistent, let me know (of course, I'd add aliases from the old 
names to the new ones too).
[0001-Don-t-delete-in-place-when-replacing-a-display-prope.patch (text/plain, attachment)]
[0002-Improve-documentation-for-display-property-functions.patch (text/plain, attachment)]
[0003-Clean-up-text-properties-in-visual-wrap-prefix-mode.patch (text/plain, attachment)]
[0004-Remove-superfluous-POSITION-argument-from-visual-wra.patch (text/plain, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Sat, 07 Jun 2025 09:35:01 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: luangruo <at> yahoo.com, 76018 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 kevin.legouguec <at> gmail.com
Subject: Re: bug#76018: 31.0.50; wrap-prefix properties from
 visual-wrap-prefix-mode proliferate
Date: Sat, 07 Jun 2025 12:34:49 +0300
> Date: Mon, 2 Jun 2025 17:49:07 -0700
> Cc: luangruo <at> yahoo.com, 76018 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
>  kevin.legouguec <at> gmail.com
> From: Jim Porter <jporterbugs <at> gmail.com>
> 
> On 6/1/2025 10:58 PM, Eli Zaretskii wrote:
> > This is more clear, but please use consistent terminology.  In the
> > above, you use "list of values" and "list of display specifications"
> > to mean the same thing (AFAIU), and "property" to mean "display
> > specification".
> 
> Good catch. Now done in this patch series. I've also added a patch 
> (0002) to improve the wording of the existing documentation, and 
> hopefully finished up everything for fixing the original bug.

LGTM, thanks.

> For patch 0002, I debated changing the various function names to 
> 'get-display-specification' and 'add-display-specification', but I left 
> them as-is for now. If you think we should change them so the wording is 
> more consistent, let me know (of course, I'd add aliases from the old 
> names to the new ones too).

No need to rename the functions, I think.




Reply sent to Jim Porter <jporterbugs <at> gmail.com>:
You have taken responsibility. (Wed, 11 Jun 2025 05:21:02 GMT) Full text and rfc822 format available.

Notification sent to Kévin Le Gouguec <kevin.legouguec <at> gmail.com>:
bug acknowledged by developer. (Wed, 11 Jun 2025 05:21:03 GMT) Full text and rfc822 format available.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: luangruo <at> yahoo.com, 76018-done <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 kevin.legouguec <at> gmail.com
Subject: Re: bug#76018: 31.0.50; wrap-prefix properties from
 visual-wrap-prefix-mode proliferate
Date: Tue, 10 Jun 2025 22:19:52 -0700
On 6/7/2025 2:34 AM, Eli Zaretskii wrote:
> LGTM, thanks.

Thanks for taking a look. Since I believe all the open issues are now 
addressed, I've merged my patch series to the master branch as 
38c57855ae2. Closing this bug now.

Of course, if I've missed anything, just let me know and I'll try to 
handle it.

>> For patch 0002, I debated changing the various function names to
>> 'get-display-specification' and 'add-display-specification', but I left
>> them as-is for now. If you think we should change them so the wording is
>> more consistent, let me know (of course, I'd add aliases from the old
>> names to the new ones too).
> 
> No need to rename the functions, I think.

Fine by me either way. I have a (very!) slight preference for renaming 
them to make the terminology clearer, but so long as documentation uses 
consistent naming, I'm happy. We can always rename the functions later 
if we change our minds.





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76018; Package emacs. (Wed, 11 Jun 2025 19:44:01 GMT) Full text and rfc822 format available.

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

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: luangruo <at> yahoo.com, Eli Zaretskii <eliz <at> gnu.org>,
 76018-done <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#76018: 31.0.50; wrap-prefix properties from
 visual-wrap-prefix-mode proliferate
Date: Wed, 11 Jun 2025 21:42:51 +0200
Jim Porter <jporterbugs <at> gmail.com> writes:

> Of course, if I've missed anything, just let me know and I'll try to handle it.

Thanks a lot for the fix Jim, & thanks everyone for the help 🙇




This bug report was last modified 6 days ago.

Previous Next


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