unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#13312: 24.3.50; delete selection mode not killing on overwrite
@ 2012-12-30 23:58 Tony Day
  2012-12-31  1:29 ` Glenn Morris
  2013-10-19 14:44 ` Drew Adams
  0 siblings, 2 replies; 12+ messages in thread
From: Tony Day @ 2012-12-30 23:58 UTC (permalink / raw)
  To: 13312

recipe:

Emacs -Q

(delete-selection-mode)

Selected text gets deleted on overwriting, rather than killed.


In GNU Emacs 24.3.50.1 (x86_64-apple-darwin, NS apple-appkit-1038.36)
 of 2012-12-30 on bob.porkrind.org
Bzr revision: 111377 yamaoka@jpl.org-20121229231805-uxaoiydm94ttem4b
Windowing system distributor `Apple', version 10.3.1187
Configured using:
 `configure --host=x86_64-apple-darwin --build=i686-apple-darwin
 --with-ns'

Important settings:
  value of $LC_ALL: en_US.UTF-8
  value of $LANG: en_US
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

Major mode: Fundamental

Minor modes in effect:
  delete-selection-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
C-h C-a <down> <up> M-x d e l e t <tab> s e l <tab> 
<return> C-x b f o o <return> K i l l SPC m e <left> 
<left> <left> <left> <left> <left> <left> <down> <return> 
<return> <return> <return> C-y M-x k i l l <tab> r 
i n <tab> <backspace> <backspace> <backspace> <backspace> 
c l e a <tab> <backspace> <backspace> <backspace> <backspace> 
<backspace> <backspace> <backspace> <backspace> <backspace> 
<backspace> <backspace> <backspace> <backspace> <backspace> 
C-g <down> <return> <return> <return> <up> <up> <up> 
<up> <up> <up> <up> <up> <S-down> <S-left> o v e r 
w r i t e <down> <down> <down> <down> <down> <down> 
C-y M-y M-y <help-echo> <down-mouse-1> <mouse-1> <menu-bar> 
<help-menu> <send-emacs-bug-report>

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Delete-Selection mode enabled

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message format-spec rfc822 mml
easymenu mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils cus-start cus-load delsel time-date
tooltip ediff-hook vc-hooks lisp-float-type mwheel ns-win tool-bar dnd
fontset image regexp-opt fringe tabulated-list newcomment lisp-mode
register page menu-bar rfn-eshadow timer select scroll-bar mouse
jit-lock font-lock syntax facemenu font-core frame cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev
minibuffer loaddefs button faces cus-face macroexp files text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote make-network-process ns multi-tty
emacs)





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#13312: 24.3.50; delete selection mode not killing on overwrite
  2012-12-30 23:58 bug#13312: 24.3.50; delete selection mode not killing on overwrite Tony Day
@ 2012-12-31  1:29 ` Glenn Morris
  2013-01-01  0:06   ` Tony Day
  2013-01-03 17:10   ` Stefan Monnier
  2013-10-19 14:44 ` Drew Adams
  1 sibling, 2 replies; 12+ messages in thread
From: Glenn Morris @ 2012-12-31  1:29 UTC (permalink / raw)
  To: Tony Day; +Cc: 13312

Tony Day wrote:

> Emacs -Q
>
> (delete-selection-mode)
>
> Selected text gets deleted on overwriting, rather than killed.

Why did you expect `delete-selection-mode' to kill rather than delete?





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#13312: 24.3.50; delete selection mode not killing on overwrite
  2012-12-31  1:29 ` Glenn Morris
@ 2013-01-01  0:06   ` Tony Day
  2013-01-03 17:10   ` Stefan Monnier
  1 sibling, 0 replies; 12+ messages in thread
From: Tony Day @ 2013-01-01  0:06 UTC (permalink / raw)
  To: Glenn Morris; +Cc: 13312

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

On Mon, Dec 31, 2012 at 12:29 PM, Glenn Morris <rgm@gnu.org> wrote:

> Why did you expect `delete-selection-mode' to kill rather than delete?


I asked on stack exchange for how to change the behaviour and someone
suggested it was a bug.  I figured delete-selection-mode was less of a
mouthful than maybe-delete-or-kill-selection-mode ;)

The critical code lines were was also ambiguous and beyond my powers of
comprehension:

(put 'self-insert-command 'delete-selection
     (lambda ()
       (not (run-hook-with-args-until-success
             'self-insert-uses-region-functions))))

Thanks for confirming the default behaviour.  On changing to kill rather
than delete with:

(put 'self-insert-command 'delete-selection 'kill)

I get no self-inserted key, and an extra blank item in the kill ring.

Regardless, why don't we call this a hack and not a bug.

Tony

[-- Attachment #2: Type: text/html, Size: 1423 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#13312: 24.3.50; delete selection mode not killing on overwrite
  2012-12-31  1:29 ` Glenn Morris
  2013-01-01  0:06   ` Tony Day
@ 2013-01-03 17:10   ` Stefan Monnier
  1 sibling, 0 replies; 12+ messages in thread
From: Stefan Monnier @ 2013-01-03 17:10 UTC (permalink / raw)
  To: Glenn Morris; +Cc: 13312

severity 13312 wishlist
thanks

>> Emacs -Q
>> (delete-selection-mode)
>> Selected text gets deleted on overwriting, rather than killed.

It's the expected behavior, but we could accept patches to optionally
provide the other behavior (tho usually the deleted text is still available
from the "PRIMARY" selection, so the need to put it on the kill-ring is
not that high and could even be reduced further if we provided more
commands to access the last "PRIMARY" selection).


        Stefan





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#13312: 24.3.50; delete selection mode not killing on overwrite
  2012-12-30 23:58 bug#13312: 24.3.50; delete selection mode not killing on overwrite Tony Day
  2012-12-31  1:29 ` Glenn Morris
@ 2013-10-19 14:44 ` Drew Adams
  2013-10-19 15:28   ` Juri Linkov
  1 sibling, 1 reply; 12+ messages in thread
From: Drew Adams @ 2013-10-19 14:44 UTC (permalink / raw)
  To: 13312

I don't think this should be on the wishlist.  It's a clear bug.

(put 'ANY-COMMAND 'delete-selection 'kill) should kill the
text in the active region.  It does not do so in the case
where ANY-COMMAND is `self-insert-command'.  A bug.

The fact that "usually the delete text is still available from the
"PRIMARY" selection" is irrelevant.  (Not to mention that there is
no "PRIMARY" selection on some platforms.)

Putting `kill' as the property value should kill the text, pure and
simple.
ss





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#13312: 24.3.50; delete selection mode not killing on overwrite
  2013-10-19 14:44 ` Drew Adams
@ 2013-10-19 15:28   ` Juri Linkov
  2013-10-19 15:37     ` Drew Adams
  0 siblings, 1 reply; 12+ messages in thread
From: Juri Linkov @ 2013-10-19 15:28 UTC (permalink / raw)
  To: Drew Adams; +Cc: 13312

> I don't think this should be on the wishlist.  It's a clear bug.
>
> (put 'ANY-COMMAND 'delete-selection 'kill) should kill the
> text in the active region.  It does not do so in the case
> where ANY-COMMAND is `self-insert-command'.  A bug.
>
> The fact that "usually the delete text is still available from the
> "PRIMARY" selection" is irrelevant.  (Not to mention that there is
> no "PRIMARY" selection on some platforms.)
>
> Putting `kill' as the property value should kill the text, pure and
> simple.

There is no bug.  In delete-selection-mode with overwrite-mode
selecting a region and typing a self-inserting key deletes the
selected region and replaces it with the typed key.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#13312: 24.3.50; delete selection mode not killing on overwrite
  2013-10-19 15:28   ` Juri Linkov
@ 2013-10-19 15:37     ` Drew Adams
  2013-10-19 16:28       ` Juri Linkov
  0 siblings, 1 reply; 12+ messages in thread
From: Drew Adams @ 2013-10-19 15:37 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 13312

> > I don't think this should be on the wishlist.  It's a clear bug.
> >
> > (put 'ANY-COMMAND 'delete-selection 'kill) should kill the
> > text in the active region.  It does not do so in the case
> > where ANY-COMMAND is `self-insert-command'.  A bug.
> >
> > The fact that "usually the delete text is still available from the
> > "PRIMARY" selection" is irrelevant.  (Not to mention that there is
> > no "PRIMARY" selection on some platforms.)
> >
> > Putting `kill' as the property value should kill the text, pure
> > and simple.
> 
> There is no bug.  In delete-selection-mode with overwrite-mode
> selecting a region and typing a self-inserting key deletes the
> selected region and replaces it with the typed key.

Did you read the bug description and the associate StackOverflow
question?  The bug is that when you put `kill' as the
`delete-selection' property value on `self-insert-command' the
region is not killed - it is not added to the `kill-ring'.  And
the first char to be self-inserted is not inserted.

This is not about the out-of-the-box behavior of self-inserting
in delete-selection mode, i.e., with the property value having
its default value.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#13312: 24.3.50; delete selection mode not killing on overwrite
  2013-10-19 15:37     ` Drew Adams
@ 2013-10-19 16:28       ` Juri Linkov
  2013-10-19 17:42         ` Drew Adams
  2013-12-11  1:11         ` Juri Linkov
  0 siblings, 2 replies; 12+ messages in thread
From: Juri Linkov @ 2013-10-19 16:28 UTC (permalink / raw)
  To: Drew Adams; +Cc: 13312

> Did you read the bug description and the associate StackOverflow
> question?  The bug is that when you put `kill' as the
> `delete-selection' property value on `self-insert-command' the
> region is not killed - it is not added to the `kill-ring'.

Ah, I thought this is a new bug report from you.  But I still
can't find a link to the associate StackOverflow question.

What I wanted to suggest is to add more self-inserting commands
to delsel like `insert-char' and `quoted-insert'.  I guess
they don't need the same `self-insert-uses-region-functions'
like the currently existing `self-insert-iso' doesn't use that.

But when you want put `kill' as the `delete-selection' property
then it's better to use this code:

(put 'self-insert-command 'delete-selection
     (lambda ()
       (and (not (run-hook-with-args-until-success
		  'self-insert-uses-region-functions))
	    'kill)))

It currently doesn't work properly because `kill-region' overwrites
the value of `this-command'.  This is a bug that can be fixed
by this patch that also fixes overwrite-mode for `kill' and
puts `delete-selection' on more commands:

=== modified file 'lisp/delsel.el'
--- lisp/delsel.el	2013-01-01 09:11:05 +0000
+++ lisp/delsel.el	2013-10-19 16:24:16 +0000
@@ -78,7 +78,9 @@ (defun delete-active-region (&optional k
   "Delete the active region.
 If KILLP in not-nil, the active region is killed instead of deleted."
   (if killp
-      (kill-region (point) (mark))
+      ;; Prevent `kill-region' from changing the value of `this-command'.
+      (let (this-command)
+	(kill-region (point) (mark)))
     (delete-region (point) (mark)))
   t)
 
@@ -102,7 +104,13 @@ (defun delete-selection-helper (type)
      FUNCTION should take no argument and return one of the above values or nil."
   (condition-case data
       (cond ((eq type 'kill)
-	     (delete-active-region t))
+	     (delete-active-region t)
+	     (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))))
 	    ((eq type 'yank)
 	     ;; Before a yank command, make sure we don't yank the
 	     ;; head of the kill-ring that really comes from the
@@ -166,6 +174,8 @@ (put 'self-insert-command 'delete-select
              'self-insert-uses-region-functions))))
 
 (put 'self-insert-iso 'delete-selection t)
+(put 'insert-char 'delete-selection t)
+(put 'quoted-insert 'delete-selection t)
 
 (put 'yank 'delete-selection 'yank)
 (put 'clipboard-yank 'delete-selection 'yank)
@@ -175,6 +185,7 @@ (put 'delete-backward-char 'delete-selec
 (put 'backward-delete-char-untabify 'delete-selection 'supersede)
 (put 'delete-char 'delete-selection 'supersede)
 
+(put 'reindent-then-newline-and-indent 'delete-selection t)
 (put 'newline-and-indent 'delete-selection t)
 (put 'newline 'delete-selection t)
 (put 'open-line 'delete-selection 'kill)
@@ -203,9 +214,9 @@ (defun delsel-unload-function ()
   (define-key minibuffer-local-completion-map "\C-g" 'abort-recursive-edit)
   (define-key minibuffer-local-must-match-map "\C-g" 'abort-recursive-edit)
   (define-key minibuffer-local-isearch-map "\C-g" 'abort-recursive-edit)
-  (dolist (sym '(self-insert-command self-insert-iso yank clipboard-yank
+  (dolist (sym '(self-insert-command self-insert-iso insert-char quoted-insert yank clipboard-yank
 		 insert-register delete-backward-char backward-delete-char-untabify
-		 delete-char newline-and-indent newline open-line))
+		 delete-char reindent-then-newline-and-indent newline-and-indent newline open-line))
     (put sym 'delete-selection nil))
   ;; continue standard unloading
   nil)






^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#13312: 24.3.50; delete selection mode not killing on overwrite
  2013-10-19 16:28       ` Juri Linkov
@ 2013-10-19 17:42         ` Drew Adams
  2013-10-19 21:08           ` Juri Linkov
  2013-12-11  1:11         ` Juri Linkov
  1 sibling, 1 reply; 12+ messages in thread
From: Drew Adams @ 2013-10-19 17:42 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 13312

> Ah, I thought this is a new bug report from you.

;-)

> But I still can't find a link to the associate StackOverflow question.

http://stackoverflow.com/questions/14076480/how-to-kill-text-when-overwriting-in-delete-selection-mode

> What I wanted to suggest is to add more self-inserting commands
> to delsel like `insert-char' and `quoted-insert'.  I guess
> they don't need the same `self-insert-uses-region-functions'
> like the currently existing `self-insert-iso' doesn't use that.

Sounds like a more general discussion.

Please consider moving that and the rest of the discussion to a
separate bug report or emacs-devel.  It doesn't seem like it is
limited to this bug.

My request was to move this bug back to Normal from Wish List.
If someone would also look into fixing it, that would be great.

> But when you want put `kill' as the `delete-selection' property
> then it's better to use this code:
> (put 'self-insert-command 'delete-selection
>      (lambda ()
>        (and (not (run-hook-with-args-until-success
> 		  'self-insert-uses-region-functions))
> 	    'kill)))

Maybe so, but that is not at all what the `delete-selection-mode'
doc & comments tell users.  If this is a new requirement/guideline,
then it needs to be documented.

But I wonder why this must now be so.  In the past, a user could
just put `kill' as the property.  The code does not seem so clean now.

One of the benefits of the `delsel.el' design (and yes, along with
those benefits come also some disadvantages) is its simplicity for
users.

This seems to go against that.  Is it really necessary?  Isn't
there another way to accomplish the same thing (whatever that is),
so we can keep the simple and clean design for users?

What was the reason for introducing
`self-insert-uses-region-functions'?  It seems it was only for
`electric-pair-mode'.  IIRC, I wasn't too happy with that hack when
it was done.  Now it seems to be dirtying (complicating) `delsel.el'.
Isn't there a better way?

But again, we should probably be discussing this elsewhere, since
it does not seem to be only about this bug.

> It currently doesn't work properly because `kill-region' overwrites
> the value of `this-command'.  This is a bug that can be fixed
> by this patch that also fixes overwrite-mode for `kill' and
> puts `delete-selection' on more commands:

Sorry, I cannot judge now whether this DTRT.  But again, this seems
to reach beyond this bug.  (Let me know if I'm missing something
and this is really specific to this bug.)

I would like to see, if possible, a simpler `delsel.el', rather than
a more complicated one - from the user's perspective, at least, if
not necessarily the implementation.  Users should be able to `put'
a single, understandable symbol as the `delete-selection' property
value.  They should not need to fiddle with obscure lambda forms (or
symbols whose names are not simple to understand).  Symbol `kill' is
simple - it says that you want the region to be killed.

This simplicity was the case before (`delsel.el' is old and simple).
Someone introduced `electric-pair-mode', and then someone else
complained about its interaction with `delete-selection-mode'.  The
fix for that should not have involved screwing `delete-selection-mode',
as seems to be the case so far.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#13312: 24.3.50; delete selection mode not killing on overwrite
  2013-10-19 17:42         ` Drew Adams
@ 2013-10-19 21:08           ` Juri Linkov
  2013-10-19 21:18             ` Drew Adams
  0 siblings, 1 reply; 12+ messages in thread
From: Juri Linkov @ 2013-10-19 21:08 UTC (permalink / raw)
  To: Drew Adams; +Cc: 13312

> What was the reason for introducing
> `self-insert-uses-region-functions'?  It seems it was only for
> `electric-pair-mode'.  IIRC, I wasn't too happy with that hack when
> it was done.  Now it seems to be dirtying (complicating) `delsel.el'.
> Isn't there a better way?

I think (run-hook-with-args-until-success 'self-insert-uses-region-functions)
could be moved to the body of `delete-selection-helper'.
Then the users again will enjoy the simplicity of

  (put 'self-insert-command 'delete-selection t)

and

  (put 'self-insert-command 'delete-selection 'kill)

> Sorry, I cannot judge now whether this DTRT.  But again, this seems
> to reach beyond this bug.  (Let me know if I'm missing something
> and this is really specific to this bug.)

The first hunk of my patch fixes the reported bug.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#13312: 24.3.50; delete selection mode not killing on overwrite
  2013-10-19 21:08           ` Juri Linkov
@ 2013-10-19 21:18             ` Drew Adams
  0 siblings, 0 replies; 12+ messages in thread
From: Drew Adams @ 2013-10-19 21:18 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 13312

> > Isn't there a better way?
> 
> I think (run-hook-with-args-until-success 'self-insert-uses-region-
> functions) could be moved to the body of `delete-selection-helper'.
> Then the users again will enjoy the simplicity of
>     (put 'self-insert-command 'delete-selection t)
> and (put 'self-insert-command 'delete-selection 'kill)

That sounds good.  Thank you, Juri.  I didn't check that things work
well with that change, but the description sounds right.

> The first hunk of my patch fixes the reported bug.

Please install that part, at least.  Not sure what the rest does,
but why not handle it as a separate bug fix?





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#13312: 24.3.50; delete selection mode not killing on overwrite
  2013-10-19 16:28       ` Juri Linkov
  2013-10-19 17:42         ` Drew Adams
@ 2013-12-11  1:11         ` Juri Linkov
  1 sibling, 0 replies; 12+ messages in thread
From: Juri Linkov @ 2013-12-11  1:11 UTC (permalink / raw)
  To: 13312-done

Version: 24.4

> It currently doesn't work properly because `kill-region' overwrites
> the value of `this-command'.  This is a bug that can be fixed
> by this patch that also fixes overwrite-mode for `kill' and
> puts `delete-selection' on more commands:

This is fixed now.





^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2013-12-11  1:11 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-30 23:58 bug#13312: 24.3.50; delete selection mode not killing on overwrite Tony Day
2012-12-31  1:29 ` Glenn Morris
2013-01-01  0:06   ` Tony Day
2013-01-03 17:10   ` Stefan Monnier
2013-10-19 14:44 ` Drew Adams
2013-10-19 15:28   ` Juri Linkov
2013-10-19 15:37     ` Drew Adams
2013-10-19 16:28       ` Juri Linkov
2013-10-19 17:42         ` Drew Adams
2013-10-19 21:08           ` Juri Linkov
2013-10-19 21:18             ` Drew Adams
2013-12-11  1:11         ` Juri Linkov

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).