23.0.50; delete-seletion-mode and read-only text

23.0.50; delete-seletion-mode and read-only text

[Johan Bockgård]

In delete-seletion-mode, trying to self insert in read-only text when
there is a selection signals an error that clears pre-command-hook.

emacs -Q

    (delete-selection-mode 1)
    (insert (propertize "12345" 'read-only t))

Select read only text with C-SPC + moving. Press a self-inserting key ("x").

=> pre-command-hook is cleared.

delete-selection-mode does check buffer-read-only, but it does not
protect against read-only text.

-- 
Johan Bockgård

Re: 23.0.50; delete-seletion-mode and read-only text

[martin rudalics]

[-- Attachment #1: Type: text/plain, Size: 499 bytes --]

> In delete-seletion-mode, trying to self insert in read-only text when
> there is a selection signals an error that clears pre-command-hook.
> 
> emacs -Q
> 
>     (delete-selection-mode 1)
>     (insert (propertize "12345" 'read-only t))
> 
> Select read only text with C-SPC + moving. Press a self-inserting key ("x").
> 
> => pre-command-hook is cleared.
> 
> delete-selection-mode does check buffer-read-only, but it does not
> protect against read-only text.

Does the attached patch fix it?


[-- Attachment #2: delsel.patch --]
[-- Type: text/plain, Size: 886 bytes --]

*** delsel.el.~1.43.~	Wed Oct 31 01:30:54 2007
--- delsel.el	Mon Jan 21 23:01:08 2008
***************
*** 80,86 ****
  
  (defun delete-selection-pre-hook ()
    (when (and delete-selection-mode transient-mark-mode mark-active
! 	     (not buffer-read-only))
      (let ((type (and (symbolp this-command)
  		     (get this-command 'delete-selection))))
        (condition-case data
--- 80,90 ----
  
  (defun delete-selection-pre-hook ()
    (when (and delete-selection-mode transient-mark-mode mark-active
! 	     (not buffer-read-only)
! 	     (let ((from (min (point) (mark)))
! 		   (to (max (point) (mark))))
! 	       (not (or (get-text-property from 'read-only)
! 			(< (next-single-property-change from 'read-only nil to) to)))))
      (let ((type (and (symbolp this-command)
  		     (get this-command 'delete-selection))))
        (condition-case data

[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: 23.0.50; delete-seletion-mode and read-only text

[Richard Stallman]

Does this change fix it?

*** delsel.el	11 Jan 2008 09:38:45 -0500	1.38.2.3
--- delsel.el	22 Jan 2008 05:57:28 -0500	
***************
*** 73,81 ****
      (transient-mark-mode t)))
  
  (defun delete-active-region (&optional killp)
!   (if killp
!       (kill-region (point) (mark))
!     (delete-region (point) (mark)))
    t)
  
  (defun delete-selection-pre-hook ()
--- 73,83 ----
      (transient-mark-mode t)))
  
  (defun delete-active-region (&optional killp)
!   (condition-case ()
!       (if killp
! 	  (kill-region (point) (mark))
! 	(delete-region (point) (mark)))
!     (error nil))
    t)
  
  (defun delete-selection-pre-hook ()

Re: 23.0.50; delete-seletion-mode and read-only text

[Johan Bockgård]

Richard Stallman <rms@gnu.org> writes:

> Does this change fix it?

Yes.

> *** delsel.el	11 Jan 2008 09:38:45 -0500	1.38.2.3
> --- delsel.el	22 Jan 2008 05:57:28 -0500	
> ***************
> *** 73,81 ****
>       (transient-mark-mode t)))
>   
>   (defun delete-active-region (&optional killp)
> !   (if killp
> !       (kill-region (point) (mark))
> !     (delete-region (point) (mark)))
>     t)
>   
>   (defun delete-selection-pre-hook ()
> --- 73,83 ----
>       (transient-mark-mode t)))
>   
>   (defun delete-active-region (&optional killp)
> !   (condition-case ()
> !       (if killp
> ! 	  (kill-region (point) (mark))
> ! 	(delete-region (point) (mark)))
> !     (error nil))
>     t)
>   
>   (defun delete-selection-pre-hook ()

-- 
Johan Bockgård

before-change-functions (was: 23.0.50; delete-seletion-mode and read-only text)

[Johan Bockgård]

martin rudalics <rudalics@gmx.at> writes:

>> In delete-seletion-mode, trying to self insert in read-only text when
>> there is a selection signals an error that clears pre-command-hook.
>
> Does the attached patch fix it?

It should fix this case, but delete-region can fail for other reasons,
so rms's fix might be better.

E.g. Customize (Widget) uses before-change-functions to prevent edits.
What happens when you select a region in a customize buffer and press
C-y (in delete-seletion-mode) is that the yank succeeds, but deleting
the selection fails

    Error in pre-command-hook: (text-read-only Attempt to change text
    outside editable field)

before-change-functions signals an error inside pre-command-hook,
something which causes both hooks to be cleared.  The yank then
proceeds uninhibited. (This is related to
http://lists.gnu.org/archive/html/emacs-devel/2007-09/msg01002.html "Is
there some way for a before-change-function to prevent a change [without
signaling an error]?")

-- 
Johan Bockgård

read-only overlays (was: 23.0.50; delete-seletion-mode and read-only text)

[Johan Bockgård]

martin rudalics <rudalics@gmx.at> writes:

>  (not (or (get-text-property from 'read-only)
>           (< (next-single-property-change from 'read-only nil to) to)))))

At first I was going to suggest that this should use get-char-property
and next-single-char-property-change, to include read-only overlays.
But on closer look, it seems that those beasts don't exist.

The manual's description of `inhibit-read-only'[1] does say that

    "Read-only characters in a buffer are those that have non-`nil'
     `read-only' properties (either text properties or overlay
     properties)."

But (overlay-put o 'read-only t) has no effect.

(`read-only' is indeed not listed in "Overlay Properties"[2].  OTOH, of
the properties in "Special Properties"[3] that are not listed in
"Overlay Properties", `pointer', `field', `line-spacing' and
`line-height' all have effect as property of an overlay, but maybe that
is just luck.  (`syntax-table' and `read-only' don't.))


[1] (info "(elisp) Read Only Buffers")
[2] (info "(elisp) Overlay Properties")
[3] (info "(elisp) Special Properties")

-- 
Johan Bockgård

Re: read-only overlays

[martin rudalics]

> The manual's description of `inhibit-read-only'[1] does say that
> 
>     "Read-only characters in a buffer are those that have non-`nil'
>      `read-only' properties (either text properties or overlay
>      properties)."
> 
> But (overlay-put o 'read-only t) has no effect.

It's a plain mystery, compare

http://lists.gnu.org/archive/html/emacs-devel/2005-12/msg00041.html

Re: 23.0.50; delete-seletion-mode and read-only text

[Richard Stallman]

I think it is clearer to catch the error than to check the property by
hand.  But the question is where to do this?  Where you did it, in
delete-selection-pre-hook, or where I did it, in delete-active-region?

The question is, if you type a self-inserting character, is it right
to skip just the deletion done by delete-active-region and do the
insertion, or should it skip both?  My patch does the former, yours
does the latter.

Re: 23.0.50; delete-seletion-mode and read-only text

[martin rudalics]

[-- Attachment #1: Type: text/plain, Size: 1035 bytes --]

 > I think it is clearer to catch the error than to check the property by
 > hand.  But the question is where to do this?  Where you did it, in
 > delete-selection-pre-hook, or where I did it, in delete-active-region?

Honestly, mine was a simple-minded attempt to handle read-only buffers
and read-only text analogously.

 > The question is, if you type a self-inserting character, is it right
 > to skip just the deletion done by delete-active-region and do the
 > insertion, or should it skip both?  My patch does the former, yours
 > does the latter.

I suppose your patch has the flaw that when `overwrite-mode' is on, the
following part

		 (if (and overwrite-mode (eq this-command 'self-insert-command))
		   (let ((overwrite-mode nil))
		     (self-insert-command (prefix-numeric-value current-prefix-arg))
		     (setq this-command 'ignore)))))

will still cause an error and clear `pre-command-hook'.

Maybe just add buffer-read-only and text-read-only to the handler in
`delete-selection-pre-hook' as in the attached patch?

[-- Attachment #2: delsel.patch --]
[-- Type: text/plain, Size: 1384 bytes --]

*** delsel.el.~1.43.~	Wed Oct 31 01:30:54 2007
--- delsel.el	Wed Jan 23 08:42:28 2008
***************
*** 79,86 ****
    t)
  
  (defun delete-selection-pre-hook ()
!   (when (and delete-selection-mode transient-mark-mode mark-active
! 	     (not buffer-read-only))
      (let ((type (and (symbolp this-command)
  		     (get this-command 'delete-selection))))
        (condition-case data
--- 79,85 ----
    t)
  
  (defun delete-selection-pre-hook ()
!   (when (and delete-selection-mode transient-mark-mode mark-active)
      (let ((type (and (symbolp this-command)
  		     (get this-command 'delete-selection))))
        (condition-case data
***************
*** 113,119 ****
  	 ;; stop safe_run_hooks from clearing out pre-command-hook.
  	 (and (eq inhibit-quit 'pre-command-hook)
  	      (setq inhibit-quit 'delete-selection-dummy))
! 	 (signal 'file-supersession (cdr data)))))))
  
  (put 'self-insert-command 'delete-selection t)
  (put 'self-insert-iso 'delete-selection t)
--- 112,120 ----
  	 ;; stop safe_run_hooks from clearing out pre-command-hook.
  	 (and (eq inhibit-quit 'pre-command-hook)
  	      (setq inhibit-quit 'delete-selection-dummy))
! 	 (signal 'file-supersession (cdr data)))
! 	((buffer-read-only text-read-only)
! 	 (message "Text is read-only"))))))
  
  (put 'self-insert-command 'delete-selection t)
  (put 'self-insert-iso 'delete-selection t)

[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: before-change-functions

[martin rudalics]

 >     Error in pre-command-hook: (text-read-only Attempt to change text
 >     outside editable field)

Signalling an error in a `pre-command-hook' is a bit like selling a user
the rope with which he will hang himself.

Re: 23.0.50; delete-seletion-mode and read-only text

[Richard Stallman]

If your patch seems to give correct results, please install it.

Re: read-only overlays (was: 23.0.50; delete-seletion-mode and read-only text)

[Richard Stallman]

It is no accident that read-only works only as a text property.
It is tested for in textprop.c only as a text property.

I am not sure that a read-only property on an overlay makes much sense.
It is a strange thing to do.  So I will fix the documentation.

Re: read-only overlays

[Stefan Monnier]

> It is no accident that read-only works only as a text property.
> It is tested for in textprop.c only as a text property.

> I am not sure that a read-only property on an overlay makes much sense.
> It is a strange thing to do.  So I will fix the documentation.

I don't understand why the `read-only' property would be more strange on
an overlay that on a text-property.


        Stefan

Re: read-only overlays

[Richard Stallman]

    I don't understand why the `read-only' property would be more strange on
    an overlay that on a text-property.

It is hard for me to put into words; it is just a feeling, not a
logical argument.