bug#41002: Undo breaks inhibit-read-only text property

bug#41002: Undo breaks inhibit-read-only text property

[Serhii Mozghovyi]

The inhibit-read-only text property in a read-only buffer does not work
with Undo:

C-x b <unique buffer name>, then insert some text, such as:

---

First paragraph.

Second paragraph.

---

Then M-: (put-text-property (point-min) (point-max) 'inhibit-read-only
t), and C-x C-q to make the buffer read-only. Now select the "Se" and
kill it with C-w: it succeeds. Then hit C-/ to undo, select "Sec" and
try to kill again. Now it fails. For any other region that does not
intersect with "Se", killing works.

bug#41002: Undo breaks inhibit-read-only text property

[Filipp Gunbin]

On 01/05/2020 13:38 +0300, Serhii Mozghovyi wrote:

> The inhibit-read-only text property in a read-only buffer does not work
> with Undo:
>

[...]

Cannot reproduce on current master.  Can you reproduce it with emacs -Q?
Do you have any custom undo functionality? (like a package)

You should file a proper bug report with M-x report-emacs-bug, it will
automatically include your Emacs version.

Filipp

bug#41002: Undo breaks inhibit-read-only text property

[Serhii Mozghovyi]

Yes, I can reproduce with emacs -Q. My Emacs version is 26.3. Here are
the details:

In GNU Emacs 26.3 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.30)
 of 2020-05-01 built on serhii-dell
Windowing system distributor 'The X.Org Foundation', version 11.0.12004000
System Description:    CentOS Linux release 7.7.1908 (Core)

Recent messages:
Quit
nil
Read-Only mode enabled in current buffer
Mark set
19 (#o23, #x13, ?\C-s)
20 (#o24, #x14, ?\C-t)
Undo!
Mark activated
kill-region: Text is read-only: #<buffer new>
Making completion list...

Configured using:
 'configure --prefix=/home/serhii/local'

Configured features:
XPM JPEG TIFF GIF PNG RSVG SOUND DBUS GSETTINGS GLIB NOTIFY LIBSELINUX
GNUTLS LIBXML2 FREETYPE XFT ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM
THREADS

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Fundamental

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny seq byte-opt gv
bytecomp byte-compile cconv cl-loaddefs cl-lib dired dired-loaddefs
format-spec rfc822 mml easymenu mml-sec password-cache epa derived epg
epg-config gnus-util rmail rmail-loaddefs mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils elec-pair time-date
mule-util tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar
dnd fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode elisp-mode lisp-mode prog-mode register page menu-bar
rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core term/tty-colors frame cl-generic cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray minibuffer
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads dbusbind
inotify dynamic-setting system-font-setting font-render-setting
move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 95086 6451)
 (symbols 48 20250 1)
 (miscs 40 47 168)
 (strings 32 28285 1382)
 (string-bytes 1 745581)
 (vectors 16 13895)
 (vector-slots 8 500670 14660)
 (floats 8 50 198)
 (intervals 56 235 9)
 (buffers 992 13)
 (heap 1024 24712 1221))

On 5/2/20 8:33 PM, Filipp Gunbin wrote:
> On 01/05/2020 13:38 +0300, Serhii Mozghovyi wrote:
>
>> The inhibit-read-only text property in a read-only buffer does not work
>> with Undo:
>>
> [...]
>
> Cannot reproduce on current master.  Can you reproduce it with emacs -Q?
> Do you have any custom undo functionality? (like a package)
>
> You should file a proper bug report with M-x report-emacs-bug, it will
> automatically include your Emacs version.
>
> Filipp

bug#41002: Undo breaks inhibit-read-only text property

[Filipp Gunbin]

> On 5/2/20 8:33 PM, Filipp Gunbin wrote:
[...]
>> Cannot reproduce on current master.  Can you reproduce it with emacs -Q?
>> Do you have any custom undo functionality? (like a package)

I see I wasn't trying right, it's actually reproducible on master too.

I think this comes from verify_interval_modification (textprop.c).  For
each interval, we check INTERVAL_WRITABLE_P, and then:

commit d1b04a9e7ada7070dbd84bb450411c1f169b3739
Author: Lars Magne Ingebrigtsen <larsi@gnus.org>
Date:   Sun Nov 16 23:36:58 2014 +0100
[...]
diff --git a/src/textprop.c b/src/textprop.c
--- a/src/textprop.c
+++ b/src/textprop.c
@@ -2301,0 +2301,4 @@
+	  if (i->position + LENGTH (i) < end
+	      && (!NILP (BVAR (current_buffer, read_only))
+		  && NILP (Vinhibit_read_only)))
+	    xsignal1 (Qbuffer_read_only, Fcurrent_buffer ());

So if there happens to be an (writeable) interval ending before "end"
(the end of the region we're killing with C-w), we will signal that the
buffer is read-only.  This may be read as "if we're going over the
boundary of an interval, then check the _buffer_ read-onliness".  Maybe
Lars, as the author of this code, could comment on this.

Filipp

bug#41002: Undo breaks inhibit-read-only text property

[Eli Zaretskii]

> From: Filipp Gunbin <fgunbin@fastmail.fm>
> Date: Sun, 03 May 2020 06:22:20 +0300
> Cc: 41002@debbugs.gnu.org, larsi@gnus.org
> 
> --- a/src/textprop.c
> +++ b/src/textprop.c
> @@ -2301,0 +2301,4 @@
> +	  if (i->position + LENGTH (i) < end
> +	      && (!NILP (BVAR (current_buffer, read_only))
> +		  && NILP (Vinhibit_read_only)))
> +	    xsignal1 (Qbuffer_read_only, Fcurrent_buffer ());
> 
> So if there happens to be an (writeable) interval ending before "end"
> (the end of the region we're killing with C-w), we will signal that the
> buffer is read-only.  This may be read as "if we're going over the
> boundary of an interval, then check the _buffer_ read-onliness".  Maybe
> Lars, as the author of this code, could comment on this.

I think you are right: we need to augment this with something that
pays attention to the inhibit-read-only property.  Would you like to
suggest a patch?

Btw, how is this related to undo?

Thanks.

bug#41002: Undo breaks inhibit-read-only text property

[Filipp Gunbin]

On 09/05/2020 10:37 +0300, Eli Zaretskii wrote:

>> From: Filipp Gunbin <fgunbin@fastmail.fm>
>> Date: Sun, 03 May 2020 06:22:20 +0300
>> Cc: 41002@debbugs.gnu.org, larsi@gnus.org
>>
>> --- a/src/textprop.c
>> +++ b/src/textprop.c
>> @@ -2301,0 +2301,4 @@
>> +	  if (i->position + LENGTH (i) < end
>> +	      && (!NILP (BVAR (current_buffer, read_only))
>> +		  && NILP (Vinhibit_read_only)))
>> +	    xsignal1 (Qbuffer_read_only, Fcurrent_buffer ());
>>
>> So if there happens to be an (writeable) interval ending before "end"
>> (the end of the region we're killing with C-w), we will signal that the
>> buffer is read-only.  This may be read as "if we're going over the
>> boundary of an interval, then check the _buffer_ read-onliness".  Maybe
>> Lars, as the author of this code, could comment on this.
>
> I think you are right: we need to augment this with something that
> pays attention to the inhibit-read-only property.  Would you like to
> suggest a patch?

That property is checked in INTERVAL_WRITABLE_P just above, it looks
fine..  The problem is I don't know what the "if" I cited is for.

> Btw, how is this related to undo?

Before undo, we seem to have one interval spanning all the text.  After
undo, there appears a short interval on which "i->position + LENGTH (i)
< end" triggers.

Filipp

bug#41002: Undo breaks inhibit-read-only text property

[Eli Zaretskii]

> From: Filipp Gunbin <fgunbin@fastmail.fm>
> Cc: egnartsms@gmail.com,  41002@debbugs.gnu.org,  larsi@gnus.org
> Date: Sat, 09 May 2020 16:34:43 +0300
> 
> > I think you are right: we need to augment this with something that
> > pays attention to the inhibit-read-only property.  Would you like to
> > suggest a patch?
> 
> That property is checked in INTERVAL_WRITABLE_P just above, it looks
> fine.

That tests if the interval is _not_ writable.  But the problem is not
triggered by the interval, it is triggered by characters after the
interval, is it not?

> The problem is I don't know what the "if" I cited is for.

Hopefully, Lars will answer that.

> > Btw, how is this related to undo?
> 
> Before undo, we seem to have one interval spanning all the text.  After
> undo, there appears a short interval on which "i->position + LENGTH (i)
> < end" triggers.

Right, thanks.  So this situation could be recreated without any undo
in the recipe, right?

bug#41002: Undo breaks inhibit-read-only text property

[Filipp Gunbin]

On 09/05/2020 16:49 +0300, Eli Zaretskii wrote:

>> From: Filipp Gunbin <fgunbin@fastmail.fm>
>> Cc: egnartsms@gmail.com,  41002@debbugs.gnu.org,  larsi@gnus.org
>> Date: Sat, 09 May 2020 16:34:43 +0300
>>
>> > I think you are right: we need to augment this with something that
>> > pays attention to the inhibit-read-only property.  Would you like to
>> > suggest a patch?
>>
>> That property is checked in INTERVAL_WRITABLE_P just above, it looks
>> fine.
>
> That tests if the interval is _not_ writable.  But the problem is not
> triggered by the interval, it is triggered by characters after the
> interval, is it not?

The buffer is read-only, and all characters have inhibit-read-only
property set (it's in the recipe).  Undo properly restores characters as
well as their properties.  But now, after undo, we don't have one single
interval any more, we have 3 intervals instead: before killed region,
killed (and restored) region, and the rest.  And this is where our
problematic (?) "if" triggers.  So I wouldn't talk about characters
after the interval, no.

>> > Btw, how is this related to undo?
>>
>> Before undo, we seem to have one interval spanning all the text.  After
>> undo, there appears a short interval on which "i->position + LENGTH (i)
>> < end" triggers.
>
> Right, thanks.  So this situation could be recreated without any undo
> in the recipe, right?

Yes, just put-text-property on separate (and adjacent) intervals instead
of the full buffer at once, and try to kill a region which crosses 
any interval boundary:

(with-temp-buffer
  (insert "ab")
  (put-text-property 1 2 'inhibit-read-only t)
  (put-text-property 2 3 'inhibit-read-only t)
  (setq buffer-read-only t)
  (kill-region 1 3))                    ;(kill-region 1 2) works

Thanks,
Filipp

bug#41002: Undo breaks inhibit-read-only text property

[Lars Ingebrigtsen]

Filipp Gunbin <fgunbin@fastmail.fm> writes:

> @@ -2301,0 +2301,4 @@
> +	  if (i->position + LENGTH (i) < end
> +	      && (!NILP (BVAR (current_buffer, read_only))
> +		  && NILP (Vinhibit_read_only)))
> +	    xsignal1 (Qbuffer_read_only, Fcurrent_buffer ());
>
> So if there happens to be an (writeable) interval ending before "end"
> (the end of the region we're killing with C-w), we will signal that the
> buffer is read-only.  This may be read as "if we're going over the
> boundary of an interval, then check the _buffer_ read-onliness".  Maybe
> Lars, as the author of this code, could comment on this.

Hm...  I've re-read the original patch, and I don't really have any
recollection of what my original thought here was, I'm afraid.  

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no