Updated proposal for DEL to delete active region

Updated proposal for DEL to delete active region

[Chong Yidong]

Here is an updated proposal, which binds C-d and DEL to new Lisp
commands `delete-forward' and `delete-backward', which delete the active
region if `delete-deletes-active-region' is non-nil.

It removes the variable `mouse-region-delete-keys', removing the special
handling of mouse selections.  It also simplifies delete-backward-char,
moving the special overwrite-mode untabification into delete-backward.

Comments?


=== modified file 'etc/NEWS'
*** etc/NEWS	2010-05-20 22:16:19 +0000
--- etc/NEWS	2010-05-22 17:39:16 +0000
***************
*** 119,124 ****
--- 119,135 ----
  ** If delete-file is called with a prefix argument, it really deletes,
  regardless of the value of `delete-by-moving-to-trash'.
  
+ ** New commands delete-forward and delete-backward.
+ C-d and DEL are now bound to these commands, instead of delete-char
+ and delete-backward-char.
+ 
+ *** New option `delete-deletes-active-region', if non-nil, means C-d
+ and DEL delete the region if `delete-deletes-active-region' is
+ non-nil.  If set to `kill', these commands kill instead.
+ 
+ The option replaces `mouse-region-delete-keys', which has been
+ removed.
+ 
  \f
  * Changes in Specialized Modes and Packages in Emacs 24.1
  
***************
*** 226,231 ****
--- 237,248 ----
  \f
  * Incompatible Lisp Changes in Emacs 24.1
  
+ ** mouse-region-delete-keys has been removed.
+ 
+ ** delete-backward-char no longer untabifies in overwrite mode.
+ The new command `delete-backward', which is intended for interactive
+ use, does so.
+ 
  ** Test for special mode-class was moved from view-file to view-buffer.
  
  ** Passing a nil argument to a minor mode function now turns the mode

=== modified file 'lisp/bindings.el'
*** lisp/bindings.el	2010-05-15 13:23:48 +0000
--- lisp/bindings.el	2010-05-22 17:17:43 +0000
***************
*** 765,770 ****
--- 765,773 ----
      (setq i (1+ i))))
  (define-key global-map [?\C-\M--] 'negative-argument)
  
+ (define-key global-map "\177" 'delete-backward)
+ (define-key global-map "\C-d" 'delete-forward)
+ 
  (define-key global-map "\C-k" 'kill-line)
  (define-key global-map "\C-w" 'kill-region)
  (define-key esc-map "w" 'kill-ring-save)

=== modified file 'lisp/mouse.el'
*** lisp/mouse.el	2010-01-13 08:35:10 +0000
--- lisp/mouse.el	2010-05-22 17:17:43 +0000
***************
*** 1263,1273 ****
  
  ;; Momentarily show where the mark is, if highlighting doesn't show it.
  
- (defcustom mouse-region-delete-keys '([delete] [deletechar] [backspace])
-   "List of keys that should cause the mouse region to be deleted."
-   :group 'mouse
-   :type '(repeat key-sequence))
- 
  (defun mouse-show-mark ()
    (let ((inhibit-quit t)
  	(echo-keystrokes 0)
--- 1263,1268 ----
***************
*** 1297,1304 ****
  				 'vertical-scroll-bar))
  			(and (memq 'down (event-modifiers event))
  			     (not (key-binding key))
! 			     (not (mouse-undouble-last-event events))
! 			     (not (member key mouse-region-delete-keys)))))
  	(and (consp event)
  	     (or (eq (car event) 'switch-frame)
  		 (eq (posn-point (event-end event))
--- 1292,1298 ----
  				 'vertical-scroll-bar))
  			(and (memq 'down (event-modifiers event))
  			     (not (key-binding key))
! 			     (not (mouse-undouble-last-event events)))))
  	(and (consp event)
  	     (or (eq (car event) 'switch-frame)
  		 (eq (posn-point (event-end event))
***************
*** 1311,1332 ****
  		      (setq events nil)))))))
      ;; If we lost the selection, just turn off the highlighting.
      (unless ignore
!       ;; For certain special keys, delete the region.
!       (if (member key mouse-region-delete-keys)
! 	  (progn
! 	    ;; Since notionally this is a separate command,
! 	    ;; run all the hooks that would be run if it were
! 	    ;; executed separately.
! 	    (run-hooks 'post-command-hook)
! 	    (setq last-command this-command)
! 	    (setq this-original-command 'delete-region)
! 	    (setq this-command (or (command-remapping this-original-command)
! 				   this-original-command))
! 	    (run-hooks 'pre-command-hook)
! 	    (call-interactively this-command))
! 	;; Otherwise, unread the key so it gets executed normally.
! 	(setq unread-command-events
! 	      (nconc events unread-command-events))))
      (setq quit-flag nil)
      (unless transient-mark-mode
        (delete-overlay mouse-drag-overlay))))
--- 1305,1313 ----
  		      (setq events nil)))))))
      ;; If we lost the selection, just turn off the highlighting.
      (unless ignore
!       ;; Unread the key so it gets executed normally.
!       (setq unread-command-events
! 	    (nconc events unread-command-events)))
      (setq quit-flag nil)
      (unless transient-mark-mode
        (delete-overlay mouse-drag-overlay))))

=== modified file 'lisp/simple.el'
*** lisp/simple.el	2010-05-19 17:16:07 +0000
--- lisp/simple.el	2010-05-22 17:23:41 +0000
***************
*** 844,849 ****
--- 844,917 ----
  	 (overlay-recenter (point))
  	 (recenter -3))))
  
+ (defcustom delete-deletes-active-region t
+   "Whether `delete-forward' and `delete-backward' delete active regions.
+ This has an effect only when Transient Mark mode is enabled.
+ If the value is the symbol `kill', `delete-backward-char' kills
+ the active region instead of deleting it."
+   :type '(choice (const :tag "Delete region" t)
+                  (const :tag "Kill region" kill)
+                  (const :tag "Do not delete region" nil))
+   :group 'editing
+   :version "24.1")
+ 
+ (defun delete-backward (n &optional killflag)
+   "Delete the previous N characters (following if N is negative).
+ If Transient Mark mode is enabled, the mark is active, and N is 1,
+ delete the text in the region and deactivate the mark instead.
+ To disable this, set `delete-deletes-region' to nil.
+ 
+ Optional second arg KILLFLAG, if non-nil, means to kill (save in
+ kill ring) instead of delete.  Interactively, N is the prefix
+ arg, and KILLFLAG is set if N is explicitly specified."
+   (interactive "p\nP")
+   (unless (integerp n)
+     (signal 'wrong-type-argument (list 'integerp n)))
+   (cond ((and (use-region-p)
+ 	      delete-deletes-region
+ 	      (= n 1))
+ 	 ;; If a region is active, kill or delete it.
+ 	 (if (eq delete-deletes-region 'kill)
+ 	     (kill-region (region-beginning) (region-end))
+ 	   (delete-region (region-beginning) (region-end))))
+ 	;; In overwrite mode, back over columns while clearing them
+ 	;; out, unless at end of line.
+ 	((and overwrite-mode
+ 	      (> n 0)
+ 	      (null (memq (char-before) '(?\t ?\n)))
+ 	      (null (eobp))
+ 	      (null (eq (char-after) ?\n)))
+ 	 (let* ((ocol (current-column))
+ 		(val (delete-char (- n) killflag)))
+ 	   (save-excursion
+ 	     (insert-char ?\s (- ocol (current-column)) nil))))
+ 	;; Otherwise, just call `delete-backward-char'.
+ 	(t
+ 	 (delete-backward-char n killflag))))
+ 
+ (defun delete-forward (n &optional killflag)
+   "Delete the previous N characters (following if N is negative).
+ If Transient Mark mode is enabled, the mark is active, and N is 1,
+ delete the text in the region and deactivate the mark instead.
+ To disable this, set `delete-deletes-active-region' to nil.
+ 
+ Optional second arg KILLFLAG non-nil means to kill (save in kill
+ ring) instead of delete.  Interactively, N is the prefix arg, and
+ KILLFLAG is set if N was explicitly specified."
+   (interactive "p\nP")
+   (unless (integerp n)
+     (signal 'wrong-type-argument (list 'integerp n)))
+   (cond ((not (and (use-region-p)
+ 		   delete-deletes-active-region
+ 		   (= n 1)))
+ 	 ;; With no region active, call `delete-char'.
+ 	 (delete-char n killflag))
+ 	;; Otherwise, kill or delete the region.
+ 	((eq delete-deletes-active-region 'kill)
+ 	 (kill-region (region-beginning) (region-end)))
+ 	(t
+ 	 (delete-region (region-beginning) (region-end)))))
+ 
  (defun mark-whole-buffer ()
    "Put point at beginning and mark at end of buffer.
  You probably should not use this function in Lisp programs;
***************
*** 3276,3282 ****
  		(delete-char 1)))
  	  (forward-char -1)
  	  (setq count (1- count))))))
!   (delete-backward-char
     (let ((skip (cond ((eq backward-delete-char-untabify-method 'hungry) " \t")
                       ((eq backward-delete-char-untabify-method 'all)
                        " \t\n\r"))))
--- 3344,3350 ----
  		(delete-char 1)))
  	  (forward-char -1)
  	  (setq count (1- count))))))
!   (delete-backward
     (let ((skip (cond ((eq backward-delete-char-untabify-method 'hungry) " \t")
                       ((eq backward-delete-char-untabify-method 'all)
                        " \t\n\r"))))

=== modified file 'src/cmds.c'
*** src/cmds.c	2010-05-15 13:23:48 +0000
--- src/cmds.c	2010-05-22 17:28:52 +0000
***************
*** 240,246 ****
         doc: /* Delete the following N characters (previous if N is negative).
  Optional second arg KILLFLAG non-nil means kill instead (save in kill ring).
  Interactively, N is the prefix arg, and KILLFLAG is set if
! N was explicitly specified.  */)
       (n, killflag)
       Lisp_Object n, killflag;
  {
--- 240,248 ----
         doc: /* Delete the following N characters (previous if N is negative).
  Optional second arg KILLFLAG non-nil means kill instead (save in kill ring).
  Interactively, N is the prefix arg, and KILLFLAG is set if
! N was explicitly specified.
! 
! The command `delete-forward' is more suitable for interactive use.  */)
       (n, killflag)
       Lisp_Object n, killflag;
  {
***************
*** 278,328 ****
         doc: /* Delete the previous N characters (following if N is negative).
  Optional second arg KILLFLAG non-nil means kill instead (save in kill ring).
  Interactively, N is the prefix arg, and KILLFLAG is set if
! N was explicitly specified.  */)
       (n, killflag)
       Lisp_Object n, killflag;
  {
-   Lisp_Object value;
-   int deleted_special = 0;
-   int pos, pos_byte, i;
- 
    CHECK_NUMBER (n);
! 
!   /* See if we are about to delete a tab or newline backwards.  */
!   pos = PT;
!   pos_byte = PT_BYTE;
!   for (i = 0; i < XINT (n) && pos_byte > BEGV_BYTE; i++)
!     {
!       int c;
! 
!       DEC_BOTH (pos, pos_byte);
!       c = FETCH_BYTE (pos_byte);
!       if (c == '\t' || c == '\n')
! 	{
! 	  deleted_special = 1;
! 	  break;
! 	}
!     }
! 
!   /* In overwrite mode, back over columns while clearing them out,
!      unless at end of line.  */
!   if (XINT (n) > 0
!       && ! NILP (current_buffer->overwrite_mode)
!       && ! deleted_special
!       && ! (PT == ZV || FETCH_BYTE (PT_BYTE) == '\n'))
!     {
!       int column = (int) current_column (); /* iftc */
! 
!       value = Fdelete_char (make_number (-XINT (n)), killflag);
!       i = column - (int) current_column (); /* iftc */
!       Finsert_char (make_number (' '), make_number (i), Qnil);
!       /* Whitespace chars are ASCII chars, so we can simply subtract.  */
!       SET_PT_BOTH (PT - i, PT_BYTE - i);
!     }
!   else
!     value = Fdelete_char (make_number (-XINT (n)), killflag);
! 
!   return value;
  }
  
  static int nonundocount;
--- 280,293 ----
         doc: /* Delete the previous N characters (following if N is negative).
  Optional second arg KILLFLAG non-nil means kill instead (save in kill ring).
  Interactively, N is the prefix arg, and KILLFLAG is set if
! N was explicitly specified.
! 
! The command `delete-backward' is more suitable for interactive use.  */)
       (n, killflag)
       Lisp_Object n, killflag;
  {
    CHECK_NUMBER (n);
!   return Fdelete_char (make_number (-XINT (n)), killflag);
  }
  
  static int nonundocount;
***************
*** 656,665 ****
  
    initial_define_key (global_map, Ctl ('A'), "beginning-of-line");
    initial_define_key (global_map, Ctl ('B'), "backward-char");
-   initial_define_key (global_map, Ctl ('D'), "delete-char");
    initial_define_key (global_map, Ctl ('E'), "end-of-line");
    initial_define_key (global_map, Ctl ('F'), "forward-char");
-   initial_define_key (global_map, 0177, "delete-backward-char");
  }
  
  /* arch-tag: 022ba3cd-67f9-4978-9c5d-7d2b18d8644e
--- 621,628 ----

RE: Updated proposal for DEL to delete active region

[Drew Adams]

> + ** New commands delete-forward and delete-backward.
> + C-d and DEL are now bound to these commands, instead of delete-char
> + and delete-backward-char.

So we now break any existing code that counts on `delete-char' or
`delete-backward-char' being key-bound.  In particular, customizations (e.g. key
remappings) involving those commands no longer work.

And what about `backward-delete-char-untabify'?
To cite Stefan (who got the untabify name slightly wrong):

>> But this doesn't seem right: we're not going to have one
>> such variable for delete-backward-char, delete-char,
>> delete-backward-char-untabify, and the handful of other commands
>> that grep "'delete-selection 'supersede" lisp/**/*.el
>> indicate will want a similar treatment.

So instead of having one such var per command (bad), or one var for all such
commands (bad), or using a property on a command symbol such as delsel does
(good), we now have two new commands whose implementations hard-code this
feature (bad).  The new commands and the new option hard-code each other.

Flexibility and legacy out the window.  Might as well stuff users in a sack, tie
it tight, and dump it overboard.


> * Incompatible Lisp Changes in Emacs 24.1
> 
> + ** mouse-region-delete-keys has been removed.
> + 
> + ** delete-backward-char no longer untabifies in overwrite mode.
> + The new command `delete-backward', which is intended for interactive
> + use, does so.

Those might be the only incompatible vanilla-Emacs code changes for this, but
they are not the only incompatible changes that can affect Lisp code (including
user code).  Basic, traditional key bindings are changed.  Any Lisp code that
expects those is now broken.


Why do this by defining new commands and changing existing key bindings?

Why not just attach a property to any existing command symbols that you want to
become sensitive to this new feature?  That's the way `delete-selection-mode'
does this sort of thing.

Oh yeah, I remember: you guys don't like attaching properties to command
symbols...  The delsel and thingatpt treatment is a saner approach, IMO.  Yes,
it's true that symbols are not functions and not all functions are associated
with symbols.  It's still saner.

Re: Updated proposal for DEL to delete active region

[Eli Zaretskii]

> From: "Drew Adams" <drew.adams@oracle.com>
> Date: Sat, 22 May 2010 11:37:18 -0700
> Cc: 
> 
> > + ** New commands delete-forward and delete-backward.
> > + C-d and DEL are now bound to these commands, instead of delete-char
> > + and delete-backward-char.
> 
> So we now break any existing code [...]

"We now" do nothing of the kind.  That was a PROPOSAL.  If you have
objections, please express them in a more constructive, maybe even
kind, manner.

RE: Updated proposal for DEL to delete active region

[Drew Adams]

> > So we now break any existing code [...]
> 
> "We now" do nothing of the kind.  That was a PROPOSAL.

I was responding to the PROPOSAL.

IF the PROPOSAL is followed THEN among the consequences are...


Constructive suggestion:

> Why not just attach a property to any existing command 
> symbols that you want to become sensitive to this new
> feature?  That's the way `delete-selection-mode' does
> this sort of thing.

Re: Updated proposal for DEL to delete active region

[David Kastrup]

"Drew Adams" <drew.adams@oracle.com> writes:

>> > So we now break any existing code [...]
>> 
>> "We now" do nothing of the kind.  That was a PROPOSAL.
>
> I was responding to the PROPOSAL.
>
> IF the PROPOSAL is followed THEN among the consequences are...
>
>
> Constructive suggestion:
>
>> Why not just attach a property to any existing command 
>> symbols that you want to become sensitive to this new
>> feature?  That's the way `delete-selection-mode' does
>> this sort of thing.

You could read up on the corresponding discussion.  Tying special
behavior to symbols rather than functions is a bad idea.

-- 
David Kastrup

RE: Updated proposal for DEL to delete active region

[Drew Adams]

> >> Why not just attach a property to any existing command 
> >> symbols that you want to become sensitive to this new
> >> feature?  That's the way `delete-selection-mode' does
> >> this sort of thing.
> 
> You could read up on the corresponding discussion.  Tying special
> behavior to symbols rather than functions is a bad idea.

I read the discussion at the time. Hard-coding behavior the way that was
proposed is a worse idea.

With the proposal, any command that you might want to use in place of the new
`delete-(back|for)ward' commands will need to hard-code the same behavior they
have in order to take account of the new option `delete-deletes-active-region'. 

IOW, the proposal would purportedly add flexibility by having an option to
define the alternative behaviors, but overall it would take away flexibility by
hard-coding the behavior that makes use of that option. The option is
made-to-order for those two new commands, and vice versa. Nothing flexible about
it.

That option-respecting behavior should be factored out and made easily
applicable (attachable, hookable, whatever) to any command you like.

If you do not like the mechanism for such loose and easy coupling to be simply
putting a property on a command symbol - (put 'delete-char 'delete-selection
'supersede), then provide another simple and elegant mechanism.  What's your
proposal - defadvice?

Hard-coding the behavior and replacing (only two of) the existing char-deletion
commands is not the answer.

(When can we expect the same approach to be used to transform thingatpt.el?)

Re: Updated proposal for DEL to delete active region

[David Kastrup]

"Drew Adams" <drew.adams@oracle.com> writes:

> Flexibility and legacy out the window.  Might as well stuff users in a
> sack, tie it tight, and dump it overboard.

With some users, it might simplify things.

-- 
David Kastrup

Re: Updated proposal for DEL to delete active region

[Chong Yidong]

"Drew Adams" <drew.adams@oracle.com> writes:

> So we now break any existing code that counts on `delete-char' or
> `delete-backward-char' being key-bound.  In particular, customizations
> (e.g. key remappings) involving those commands no longer work.

Yes, this is a clear downside.

In the absence of backward compatibility considerations, it would be
cleaner to separate the function that deletes from the command that
deletes.  This is similar to the distinction we make between, say,
forward-line and next-line.

This is not entirely academic.  For instance, when overwrite-mode is
active, delete-backward-char untabifies while deleting.  This behavior
is useful when called interactively, but not clean when called
programmatically.

It could be argued that the benefits are too minuscule compared to the
disruption.  I would like to hear some arguments either way.

If we decide not to go the delete-backward/delete-forward route, there
is still the earlier approach I've presented, which changes delete-char
and delete-backward-char directly without separating them out.

> So instead of having one such var per command (bad), or one var for
> all such commands (bad), or using a property on a command symbol such
> as delsel does (good), we now have two new commands whose
> implementations hard-code this feature (bad).  The new commands and
> the new option hard-code each other.

First, the goal is not to replace delete-selection mode.  It's to
eliminate the special-case handling of mouse selections and make active
selections behave more consistently.

Second, a huge number of commands already hard-code specific behavior
when Transient Mark mode is active.  That's the point of Transient Mark
mode.  So I do not find this argument persuasive.

RE: Updated proposal for DEL to delete active region

[Drew Adams]

> > So we now break any existing code that counts on `delete-char'
> > or `delete-backward-char' being key-bound.  In particular, 
> > customizations (e.g. key remappings) involving those commands
> > no longer work.
> 
> Yes, this is a clear downside.

I'm glad we agree on that.

> > So instead of having one such var per command (bad), or one var
> > for all such commands (bad), or using a property on a command 
> > symbol such as delsel does (good), we now have two new commands
> > whose implementations hard-code this feature (bad).  The new
> > commands and the new option hard-code each other.
> 
> First, the goal is not to replace delete-selection mode.  It's to
> eliminate the special-case handling of mouse selections

Yes, I know.  And I agree with that initiative.
I was against that special case from the beginning.

I think you are going beyond mouse-selection in the proposed change, however.

> and make active selections behave more consistently.

(I'm not sure what that means, so I can't speak to it.)

> Second, a huge number of commands already hard-code specific
> behavior when Transient Mark mode is active.  That's the point
> of Transient Mark mode.

That might be, but I don't think it's the same kind of behavior.  That
`query-replace' might behave differently depending on whether there is an active
region is one thing.  That doesn't affect basic editing commands and their
traditional keys.  When you replace the bindings of the basic character-deletion
commands it can affect any mode or user code that remaps those commands.

I didn't even mention that the proposal changes the behavior of
`delete-backward-char'.  I didn't notice that part of the proposal at first
reading.  IIUC, That would break any code (or interactive use) that invokes
`delete-backward-char' and expects it to untabify.

Can we not please find some other way to simply "eliminate the special-case
handling of mouse selections"?  What about going back to the code that was there
before that feature was added?

Re: Updated proposal for DEL to delete active region

[Andreas Schwab]

Chong Yidong <cyd@stupidchicken.com> writes:

> This is not entirely academic.  For instance, when overwrite-mode is
> active, delete-backward-char untabifies while deleting.  This behavior
> is useful when called interactively, but not clean when called
> programmatically.

Programmatically, you should call delete-region anyway.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Updated proposal for DEL to delete active region

[Stefan Monnier]

>> This is not entirely academic.  For instance, when overwrite-mode is
>> active, delete-backward-char untabifies while deleting.  This behavior
>> is useful when called interactively, but not clean when called
>> programmatically.
> Programmatically, you should call delete-region anyway.

Indeed, I've just added delete-backward-char to
byte-compile-interactive-only-functions.


        Stefan

Re: Updated proposal for DEL to delete active region

[Stefan Monnier]

> Here is an updated proposal, which binds C-d and DEL to new Lisp
> commands `delete-forward' and `delete-backward', which delete the active
> region if `delete-deletes-active-region' is non-nil.
> It removes the variable `mouse-region-delete-keys', removing the special
> handling of mouse selections.  It also simplifies delete-backward-char,
> moving the special overwrite-mode untabification into delete-backward.

Looks pretty good now.  Comments:
- I don't think we want to change delete-backward-char.  Instead, we
  want it to be an alias for delete-backward (or the reverse, I don't
  have any strong opinion either way), and we want to add it to
  byte-compile-interactive-only-functions (it should have been there,
  and we should put it there on the emacs-23 branch already).
  But yes, we can move it to Elisp at the same occasion.
- the code you sent uses delete-deletes-region instead of
  delete-deletes-active-region at some places.
- removing mouse-region-delete-keys should allow the removal of a lot
  more code (the highlighting code can be removed because the temporary
  transient mark mode takes care of it nowadays).  The important part of
  this removal is that it should remove all the code that calls
  read-event waiting for the next command.


        Stefan