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