mouse.el patch for region tracking

mouse.el patch for region tracking

[JD Smith]

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


Recently I found that Emacs 22 breaks a simple usage of mouse-drag- 
region for defining regions by mouse drag.  When transient-mark-mode  
is set, mouse-drag-region under Emacs 21 would let you define a  
region by dragging, and when the drag is released, it completes (at  
which point you can operate on the region).  This is somewhat similar  
to the XEmacs function 'mouse-track'.

With Emacs 22, several changes were introduced to mouse-drag-region  
(unrelated improvements) which broke this apparently unsupported  
usage.  This breaks a very useful drag-to-inspect functionality in my  
IDLWAVE programming mode.  The call to mouse-drag-region would not  
complete without an additional event after the drag completes.

Since I want all the rich behavior of mouse-drag-region (double- and  
triple-click select by word and line, etc.), a very simple fix is to  
re-christen 'mouse-drag-region-1' as 'mouse-drag-track', and provide  
it an optional argument to enable calling the final event handling  
code which was tripping me up.  Then I can use this functio

I have to admit having understood little of the code I've wrapped in  
'handle-terminating-event', only that it indirectly calls read-event  
via 'mouse-show-mark' and thus blocks the return of the function,  
thus breaking of my old use of mouse-drag-region.  I would appreciate  
it if you could look this over and see if it's doing what I want it  
to (inasmuch as I've communicated that).  In particular the doc  
string is a bit weak if this function were to be advertised for more  
general usage by programmers.

JD

  

[-- Attachment #2: mouse_patch_jds.diff --]
[-- Type: application/octet-stream, Size: 2708 bytes --]

*** mouse.el	27 Dec 2005 20:19:29 -0700	1.290
--- mouse.el	30 Dec 2005 21:20:28 -0700	
***************
*** 758,764 ****
  	  (display-buffer (current-buffer)))
        ;; Give temporary modes such as isearch a chance to turn off.
        (run-hooks 'mouse-leave-buffer-hook)
!       (mouse-drag-region-1 start-event))))
  
  
  (defun mouse-on-link-p (pos)
--- 758,764 ----
  	  (display-buffer (current-buffer)))
        ;; Give temporary modes such as isearch a chance to turn off.
        (run-hooks 'mouse-leave-buffer-hook)
!       (mouse-drag-track start-event t))))
  
  
  (defun mouse-on-link-p (pos)
***************
*** 858,864 ****
    (let ((range (mouse-start-end start end mode)))
      (move-overlay ol (car range) (nth 1 range))))
  
! (defun mouse-drag-region-1 (start-event)
    (mouse-minibuffer-check start-event)
    (setq mouse-selection-click-count-buffer (current-buffer))
    (let* ((original-window (selected-window))
--- 858,865 ----
    (let ((range (mouse-start-end start end mode)))
      (move-overlay ol (car range) (nth 1 range))))
  
! (defun mouse-drag-track (start-event  &optional handle-terminating-event)
!     "Track mouse drags by highlighting area between point and cursor."
    (mouse-minibuffer-check start-event)
    (setq mouse-selection-click-count-buffer (current-buffer))
    (let* ((original-window (selected-window))
***************
*** 942,948 ****
  		 (integer-or-marker-p end-point))
          (mouse-move-drag-overlay mouse-drag-overlay start-point end-point click-count))
  
!       (if (consp event)
  	  (let* ((fun (key-binding (vector (car event))))
  		 (do-multi-click   (and (> (event-click-count event) 0)
  					(functionp fun)
--- 943,949 ----
  		 (integer-or-marker-p end-point))
          (mouse-move-drag-overlay mouse-drag-overlay start-point end-point click-count))
  
!       (if (and (consp event) handle-terminating-event)
  	  (let* ((fun (key-binding (vector (car event))))
  		 (do-multi-click   (and (> (event-click-count event) 0)
  					(functionp fun)
***************
*** 1030,1036 ****
  
          ;; Case where the end-event is not a cons cell (it's just a boring
          ;; char-key-press).
! 	(delete-overlay mouse-drag-overlay)))))
  \f
  ;; Commands to handle xterm-style multiple clicks.
  (defun mouse-skip-word (dir)
--- 1031,1039 ----
  
          ;; Case where the end-event is not a cons cell (it's just a boring
          ;; char-key-press).
! 	(unless (and transient-mark-mode 
! 		     (consp event))
! 	  (delete-overlay mouse-drag-overlay))))))
  \f
  ;; Commands to handle xterm-style multiple clicks.
  (defun mouse-skip-word (dir)

[-- 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: mouse.el patch for region tracking

[Chong Yidong]

JD Smith <jdsmith@as.arizona.edu> writes:

> Recently I found that Emacs 22 breaks a simple usage of mouse-drag- 
> region for defining regions by mouse drag.  When transient-mark-mode
> is set, mouse-drag-region under Emacs 21 would let you define a
> region by dragging, and when the drag is released, it completes (at
> which point you can operate on the region).  This is somewhat similar
> to the XEmacs function 'mouse-track'.

You're not very clear about what exactly is broken.  Could you give a
recipe to reproduce the bug, whatever the bug is?

Re: mouse.el patch for region tracking

[Andreas Schwab]

JD Smith <jdsmith@as.arizona.edu> writes:

> Recently I found that Emacs 22 breaks a simple usage of mouse-drag- 
> region for defining regions by mouse drag.  When transient-mark-mode  
> is set, mouse-drag-region under Emacs 21 would let you define a  
> region by dragging, and when the drag is released, it completes (at  
> which point you can operate on the region).

I can still do that with an Emacs built from CVS yesterday.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: mouse.el patch for region tracking

[Stefan Monnier]

>> Recently I found that Emacs 22 breaks a simple usage of mouse-drag- 
>> region for defining regions by mouse drag.  When transient-mark-mode  
>> is set, mouse-drag-region under Emacs 21 would let you define a  
>> region by dragging, and when the drag is released, it completes (at  
>> which point you can operate on the region).

> I can still do that with an Emacs built from CVS yesterday.

How?  The mouse-drag-region function doesn't end when you release the mouse
but only after the next event (this is because it's waiting to see if the
next event is in mouse-region-delete-keys).  That makes it unusable in other
contexts (when you try to do something similar to what XEmacs's mouse-track
function does).

In Emacs-21, there was a bug which was that when transient-mark-mode was
enabled, mouse-region-delete-keys wasn't obeyed (and the function returned
immediately upon the release of the mouse button).  JD unknowingly relied on
this bug to get the behavior he needs.

My vote would be to kill mouse-region-delete-keys, but it seems that Richard
insists on this functionality so it looks like JD's patch is a good and safe
way to recover the old feature which is used by several third party
elisp packages.


        Stefan

Re: mouse.el patch for region tracking

[JD Smith]

On Dec 31, 2005, at 8:13 AM, Chong Yidong wrote:

> JD Smith <jdsmith@as.arizona.edu> writes:
>
>> Recently I found that Emacs 22 breaks a simple usage of mouse-drag-
>> region for defining regions by mouse drag.  When transient-mark-mode
>> is set, mouse-drag-region under Emacs 21 would let you define a
>> region by dragging, and when the drag is released, it completes (at
>> which point you can operate on the region).  This is somewhat similar
>> to the XEmacs function 'mouse-track'.
>
> You're not very clear about what exactly is broken.  Could you give a
> recipe to reproduce the bug, whatever the bug is?

Sure, sorry.  I'm not sure it can be classified as a bug, since the  
behavior I was using wasn't really documented, and the changes which  
"broke" it were actually improvements to mouse.el.  Nonetheless, it's  
a useful capability which XEmacs has and which Emacs 22 doesn't  
(without the patch).

Try:

(defun do-drag (event)
   (interactive "e")
   (let ((transient-mark-mode t)) ; transient mark
     (message "DRAG STARTED")
     (mouse-drag-region event)
     (message "DRAG FINISHED")))

(global-set-key [(control shift down-mouse-2)] 'do-drag)

On shift-mouse-2-drag, under Emacs 21, this code would complete after  
the drag.  With Emacs 22, this is no longer the case; you need  
another event, like a key press, to return.  See this thread for more  
detail:

http://lists.gnu.org/archive/html/emacs-devel/2005-06/msg00214.html

With the patch I submitted, I can do:

(defun do-drag (event)
   "Print value of variable at the mouse position, with `help'"
   (interactive "e")
   (message "DRAG STARTED")
   (mouse-drag-track event)
   (message "DRAG FINISHED"))

and it works as desired under Emacs 22 (I'd use this new function if  
bound, and fall back on the transient-mark-mode trick otherwise).

JD

Re: mouse.el patch for region tracking

[Richard M. Stallman]

I have not checked it carefully, but the general idea seems ok.
If someone verifies this is correct, then please install it.

Re: mouse.el patch for region tracking

[Kim F. Storm]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> My vote would be to kill mouse-region-delete-keys, but it seems that Richard
> insists on this functionality so it looks like JD's patch is a good and safe
> way to recover the old feature which is used by several third party
> elisp packages.

We can (easily?) do both:

IMO, setting/let-binding mouse-region-delete-keys to nil should inhibit reading
the next event in mouse-drag-region.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: mouse.el patch for region tracking

[Stefan Monnier]

>> My vote would be to kill mouse-region-delete-keys, but it seems that Richard
>> insists on this functionality so it looks like JD's patch is a good and safe
>> way to recover the old feature which is used by several third party
>> elisp packages.

> We can (easily?) do both:

> IMO, setting/let-binding mouse-region-delete-keys to nil should inhibit
> reading the next event in mouse-drag-region.

There is another reason to wait for the next event: the selected region is
highlighted until the next event.  Now this is not necessary if
transient-mark-mode is activated, so your suggestion is that code that wants
to do something like XEmacs's mouse-track should do

   (let ((transient-mark-mode t)
         (mouse-region-delete-keys nil))
     (mouse-drag-region event))

Maybe that's OK, but I really find it butt-ugly.


        Stefan

Re: mouse.el patch for region tracking

[Kim F. Storm]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> There is another reason to wait for the next event: the selected region is
> highlighted until the next event.  Now this is not necessary if
> transient-mark-mode is activated, so your suggestion is that code that wants
> to do something like XEmacs's mouse-track should do

In any case, JD's patch is good, but there should be some more
commentary to explain the meaning of the HANDLE-TERMINATING-EVENT arg
to mouse-drag-track.  Also, we may want to announce this function
somewhere.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: mouse.el patch for region tracking

[JD Smith]

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

On Mon, 2006-01-02 at 10:38 +0100, Kim F. Storm wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> 
> > There is another reason to wait for the next event: the selected region is
> > highlighted until the next event.  Now this is not necessary if
> > transient-mark-mode is activated, so your suggestion is that code that wants
> > to do something like XEmacs's mouse-track should do
> 
> In any case, JD's patch is good, but there should be some more
> commentary to explain the meaning of the HANDLE-TERMINATING-EVENT arg
> to mouse-drag-track.  Also, we may want to announce this function
> somewhere.

I've reworked the patch a bit, because I found that the mark was not
being set in my previous version.  Now I skip only the copy-region and
mouse-show-mark stuff, unless the optional argument is passed.  This
leaves the mark set (which of course is preferred for operating on the
region).  I also expanded the documentation to mouse-drag-track.

The one question remaining is whether it is appropriate to delete the
overlay (as I currently do) when transient-mark-mode is nil, and
terminating events aren't being processed.  This means the highlighted
region vanishes just after the drag completes.

I checked XEmacs mouse-track, and it works differently: with zmacs-
regions on, the overlay disappears only when a new region is started,
just like transient-mark-mode.  However, with it nil, the overlay still
remains until some other event (any event) arrives, yet somehow it's
able to allow the drag to complete anyway.  I.e., it doesn't block
operation on the region, as mouse-drag-region does.

I'm not sure how this is achieved, but I think it's the best behavior
(most consistent with regular mouse selection).  Unfortunately, the way
"regular mouse selection" seems to work in Emacs is by using that
blocking call to 'mouse-show-mark' to delete the overlay when the next
event arrives, which is the behavior this patch is attempting to
address.  

Unless someone can think of a clever way to implement this, I think I
can live with the overlay disappearing (or can just (let ((transient-
mark-mode t)) and delete it myself as appropriate).

Let me know if this looks acceptable and I'll check it in.

JD


[-- Attachment #2: mouse_patch_jds2.diff --]
[-- Type: text/x-patch, Size: 4425 bytes --]

Index: mouse.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/mouse.el,v
retrieving revision 1.292
diff -b -c -c -r1.292 mouse.el
*** mouse.el	1 Jan 2006 07:55:12 -0000	1.292
--- mouse.el	2 Jan 2006 21:06:17 -0000
***************
*** 765,771 ****
  	  (display-buffer (current-buffer)))
        ;; Give temporary modes such as isearch a chance to turn off.
        (run-hooks 'mouse-leave-buffer-hook)
!       (mouse-drag-region-1 start-event))))
  
  
  (defun mouse-on-link-p (pos)
--- 765,771 ----
  	  (display-buffer (current-buffer)))
        ;; Give temporary modes such as isearch a chance to turn off.
        (run-hooks 'mouse-leave-buffer-hook)
!       (mouse-drag-track start-event t))))
  
  
  (defun mouse-on-link-p (pos)
***************
*** 865,871 ****
    (let ((range (mouse-start-end start end mode)))
      (move-overlay ol (car range) (nth 1 range))))
  
! (defun mouse-drag-region-1 (start-event)
    (mouse-minibuffer-check start-event)
    (setq mouse-selection-click-count-buffer (current-buffer))
    (let* ((original-window (selected-window))
--- 865,879 ----
    (let ((range (mouse-start-end start end mode)))
      (move-overlay ol (car range) (nth 1 range))))
  
! (defun mouse-drag-track (start-event  &optional handle-terminating-event)
!     "Track mouse drags by highlighting area between point and cursor.
! The region will be defined with mark and point.  If
! HANDLE-TERMINATING-EVENT is set, after the drag completes, wait
! for and handle an additional terminating event (keypress, etc.),
! delete or copy the selected region to the kill buffer, follow
! links, etc.  When HANDLE-TERMINATING-EVENT is nil, returns
! immediately after the drag completes, removing the
! overlay (unless transient mark mode is active)."
    (mouse-minibuffer-check start-event)
    (setq mouse-selection-click-count-buffer (current-buffer))
    (let* ((original-window (selected-window))
***************
*** 949,959 ****
  		 (integer-or-marker-p end-point))
          (mouse-move-drag-overlay mouse-drag-overlay start-point end-point click-count))
  
        (if (consp event)
  	  (let* ((fun (key-binding (vector (car event))))
  		 (do-multi-click   (and (> (event-click-count event) 0)
  					(functionp fun)
! 					(not (memq fun '(mouse-set-point mouse-set-region))))))
              ;; Run the binding of the terminating up-event, if possible.
  	    (if (and (not (= (overlay-start mouse-drag-overlay)
  			     (overlay-end mouse-drag-overlay)))
--- 957,970 ----
  		 (integer-or-marker-p end-point))
          (mouse-move-drag-overlay mouse-drag-overlay start-point end-point click-count))
  
+       ;; Handle the terminating event
        (if (consp event)
  	  (let* ((fun (key-binding (vector (car event))))
  		 (do-multi-click   (and (> (event-click-count event) 0)
  					(functionp fun)
! 					(not (memq fun 
! 						   '(mouse-set-point 
! 						     mouse-set-region))))))
  	    ;; Run the binding of the terminating up-event, if possible.
  	    (if (and (not (= (overlay-start mouse-drag-overlay)
  			     (overlay-end mouse-drag-overlay)))
***************
*** 977,982 ****
--- 988,997 ----
  		       last-command this-command)
  		  (push-mark region-commencement t t)
  		  (goto-char region-termination)
+ 		  (if (not handle-terminating-event)
+ 		      ;; Skip all post-event handling, return immediately.
+ 		      (unless transient-mark-mode
+ 			(delete-overlay mouse-drag-overlay))
  		    ;; Don't let copy-region-as-kill set deactivate-mark.
  		    (when mouse-drag-copy-region
  		      (let (deactivate-mark)
***************
*** 989,995 ****
                      ;; avoid trying to use the region.
                      (and (mark t) mark-active
                           (eq buffer (current-buffer))
!                          (mouse-set-region-1))))
                ;; Run the binding of the terminating up-event.
  	      ;; If a multiple click is not bound to mouse-set-point,
  	      ;; cancel the effects of mouse-move-drag-overlay to
--- 1004,1010 ----
  		      ;; avoid trying to use the region.
  		      (and (mark t) mark-active
  			   (eq buffer (current-buffer))
! 			   (mouse-set-region-1)))))
                ;; Run the binding of the terminating up-event.
  	      ;; If a multiple click is not bound to mouse-set-point,
  	      ;; cancel the effects of mouse-move-drag-overlay to

[-- 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: mouse.el patch for region tracking

[Stefan Monnier]

> The one question remaining is whether it is appropriate to delete the
> overlay (as I currently do) when transient-mark-mode is nil, and
> terminating events aren't being processed.  This means the highlighted
> region vanishes just after the drag completes.

I have a question about this part: why do you not delete the overlay when
transient-mark-mode is active?

As for the extra argument handle-terminating-event, I'd call it
"do-mouse-drag-region-post-process" and I wouldn't document what it
does, but just say that it should only be used by mouse-drag-region.


        Stefan

Re: mouse.el patch for region tracking

[JD Smith]

On Jan 2, 2006, at 6:45 PM, Stefan Monnier wrote:

>> The one question remaining is whether it is appropriate to delete the
>> overlay (as I currently do) when transient-mark-mode is nil, and
>> terminating events aren't being processed.  This means the  
>> highlighted
>> region vanishes just after the drag completes.
>
> I have a question about this part: why do you not delete the  
> overlay when
> transient-mark-mode is active?

I guess because the region is still active, so, as normally in  
transient-mark-mode, the active region should remain highlighted  
until mark is deactivated, just as in normal selection.  Maybe this  
isn't necessary.  I'd just as soon have it always delete.  In my  
mode, I place another overlay there more temporarily, so it doesn't  
affect me.

> As for the extra argument handle-terminating-event, I'd call it
> "do-mouse-drag-region-post-process" and I wouldn't document what it
> does, but just say that it should only be used by mouse-drag-region.

Done.

One thing bothered me while playing with transient mark mode: 'mouse- 
region-delete-keys', while nice, is very inconsistent in transient  
mark mode.    If you define a region with the mouse, it works.  If  
you define a region with C-space and then move point, it does not  
work.  I know the name implies this, but the semantics of "mark  
something yellow, hit the delete key" seem fairly intuitive.

JD

Re: mouse.el patch for region tracking

[Stefan Monnier]

>> I have a question about this part: why do you not delete the overlay when
>> transient-mark-mode is active?

> I guess because the region is still active, so, as normally in
> transient-mark-mode, the active region should remain highlighted until mark
> is deactivated, just as in normal selection.

The region highlighting (when transient-mark-mode is ON) is not done by
mouse-region-overlay, so deleting this overlay shouldn't make
any difference.

> Maybe this isn't necessary.

Indeed AFAICT it shouldn't be necessary, which is why I ask.
If the explicit test is placed there only based on a fear of what might
happen, rather than on actual evidence, then I suggest to remove it.

> One thing bothered me while playing with transient mark mode: 'mouse- 
> region-delete-keys', while nice, is very inconsistent in transient mark
> mode.  If you define a region with the mouse, it works.  If you define
> a region with C-space and then move point, it does not work.  I know the
> name implies this, but the semantics of "mark something yellow, hit the
> delete key" seem fairly intuitive.

Yes, it has many such inconsistencies.  If you want it to be consistent, try
delete-selection-mode.  Otherwise, just kill this ugly beast.


        Stefan

Re: mouse.el patch for region tracking

[JD Smith]

On Jan 3, 2006, at 8:37 AM, Stefan Monnier wrote:

>>> I have a question about this part: why do you not delete the  
>>> overlay when
>>> transient-mark-mode is active?
>
>> I guess because the region is still active, so, as normally in
>> transient-mark-mode, the active region should remain highlighted  
>> until mark
>> is deactivated, just as in normal selection.
>
> The region highlighting (when transient-mark-mode is ON) is not  
> done by
> mouse-region-overlay, so deleting this overlay shouldn't make
> any difference.
>
>> Maybe this isn't necessary.
>
> Indeed AFAICT it shouldn't be necessary, which is why I ask.
> If the explicit test is placed there only based on a fear of what  
> might
> happen, rather than on actual evidence, then I suggest to remove it.

The evidence is that, when transient-mark-mode is bound, as:

(defun do-drag (event)
   (interactive "e")
   (let ((transient-mark-mode t))
     (message "DRAG STARTED")
     (mouse-drag-track event)
     (message "DRAG FINISHED")))

the overlay is deleted anyway.  If transient mark mode is on outside  
of the let, everything behaves normally.  This is likely the correct  
behavior.  I've made this and the other changes you suggested, and  
checked them in.

Thanks,

JD