unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#902: select-active-regions only half-working
@ 2008-09-06  5:53 ` David De La Harpe Golden
       [not found]   ` <handler.902.B.122068042025981.ack@emacsbugs.donarmstrong.com>
                     ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: David De La Harpe Golden @ 2008-09-06  5:53 UTC (permalink / raw)
  To: submit

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

Package: emacs
Version: 23.0.60
Severity: normal
Tags: patch

The select-active-regions in-tree does not work in all cases - it works
when the region is changed by moving the mark but not when it is changed
by certain other means e.g. moving the point with the keyboard.
(Workaround:  C-x C-x C-x C-x ...).

There were a couple of approaches tried a while back (see emacs-devel
"Improved X Selections" thread).  The idle timer seemed to work best in
practice, despite (or because of) "coarse graining" i.e. a bunch of
small region changes in a row don't reupdate the selection for each
change, only kicks in when emacs has idled.

Attached patch is a simple idle-timer based implementation.

Concerns here may also apply to the activate-mark-hook (which states
in its documentation that it is rerun when region changes). Having that
work similarly with the same method would probably mean that
active region timer would have to be enabled whenever the mark is
active, not just when select-active-regions is on.
















[-- Attachment #2: select-active-regions-by-idle-timer.diff --]
[-- Type: text/x-patch, Size: 3131 bytes --]

Index: lisp/simple.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/simple.el,v
retrieving revision 1.945
diff -U 8 -r1.945 simple.el
--- lisp/simple.el	15 Aug 2008 00:30:44 -0000	1.945
+++ lisp/simple.el	6 Sep 2008 05:41:14 -0000
@@ -3439,18 +3439,46 @@
 
 (defun activate-mark ()
   "Activate the mark."
   (when (mark t)
     (setq mark-active t)
     (unless transient-mark-mode
       (setq transient-mark-mode 'lambda))))
 
+(defvar select-active-regions-timer nil)
+
+(defvar select-active-regions-last-region-hash nil)
+
+(defun select-active-regions-maybe-set-selection ()
+  "Implements `select-active-regions', called from idle timer
+`select-active-regions-timer'"
+  (and select-active-regions
+       (region-active-p)
+       (> (region-end) (region-beginning))
+       (if (or (not select-active-regions-last-region-hash)
+	       (x-selection-owner-p nil))
+	   (let ((region-hash
+		  (md5 (current-buffer) (region-beginning)
+		       (region-end) nil t)))
+	     (unless (string-equal region-hash
+				   select-active-regions-last-region-hash)
+	       (x-set-selection
+		nil (buffer-substring (region-beginning) (region-end)))
+	       (setq select-active-regions-last-region-hash
+		     region-hash)))
+	 (setq select-active-regions-last-region-hash nil))))
+
 (defcustom select-active-regions nil
-  "If non-nil, an active region automatically becomes the window selection."
+  "If non-nil, an active region automatically becomes the window selection.
+
+In conjunction with this, you might want to:
+rebind mouse-2 to `mouse-yank-primary', set `x-select-enable-primary' to nil,
+set `x-select-enable-clipboard' to non-nil, set `mouse-drag-copy-region'
+to nil, and turn on `transient-mark-mode'."
   :type 'boolean
   :group 'killing
   :version "23.1")
 
 (defun set-mark (pos)
   "Set this buffer's mark to POS.  Don't use this function!
 That is to say, don't use this function unless you want
 the user to see that the mark has moved, and you want the previous
@@ -3466,19 +3494,26 @@
 store it in a Lisp variable.  Example:
 
    (let ((beg (point))) (forward-line 1) (delete-region beg (point)))."
 
   (if pos
       (progn
 	(setq mark-active t)
 	(run-hooks 'activate-mark-hook)
-	(and select-active-regions
-	     (x-set-selection
-	      nil (buffer-substring (region-beginning) (region-end))))
+	(if select-active-regions
+	    (progn
+	      (unless select-active-regions-timer
+		(setq select-active-regions-timer
+		      (run-with-idle-timer
+		       0.1 t 'select-active-regions-maybe-set-selection)))
+	      (select-active-regions-maybe-set-selection))
+	  (when select-active-regions-timer
+	    (cancel-timer select-active-regions-timer)
+	    (setq select-active-regions-timer nil)))
 	(set-marker (mark-marker) pos (current-buffer)))
     ;; Normally we never clear mark-active except in Transient Mark mode.
     ;; But when we actually clear out the mark value too,
     ;; we must clear mark-active in any mode.
     (setq mark-active nil)
     (run-hooks 'deactivate-mark-hook)
     (set-marker (mark-marker) nil)))
 

[-- Attachment #3: ChangeLog.select-active-regions-by-idle-timer --]
[-- Type: text/plain, Size: 127 bytes --]

2008-09-06 David De La Harpe Golden <david@harpegolden.net>

	* simple.el: idle-timer implementation of select-active-regions


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

* bug#902: Acknowledgement (select-active-regions only half-working)
       [not found]   ` <handler.902.B.122068042025981.ack@emacsbugs.donarmstrong.com>
@ 2008-09-06 19:35     ` David De La Harpe Golden
  0 siblings, 0 replies; 16+ messages in thread
From: David De La Harpe Golden @ 2008-09-06 19:35 UTC (permalink / raw)
  To: 902

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

Uh.  Hashing is probably silly/pointless in this case (wrote that far
too early in the morning...).  Revised patch attached, just saves last
region and uses string-equal.




[-- Attachment #2: select-active-regions-by-idle-timer_rev2.diff --]
[-- Type: text/x-patch, Size: 3055 bytes --]

Index: lisp/simple.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/simple.el,v
retrieving revision 1.945
diff -U 8 -r1.945 simple.el
--- lisp/simple.el	15 Aug 2008 00:30:44 -0000	1.945
+++ lisp/simple.el	6 Sep 2008 19:33:00 -0000
@@ -3439,18 +3439,45 @@
 
 (defun activate-mark ()
   "Activate the mark."
   (when (mark t)
     (setq mark-active t)
     (unless transient-mark-mode
       (setq transient-mark-mode 'lambda))))
 
+(defvar select-active-regions-timer nil)
+
+(defvar select-active-regions-last-region nil)
+
+(defun select-active-regions-maybe-set-selection ()
+  "Implements `select-active-regions', called from idle timer
+`select-active-regions-timer'"
+  (and select-active-regions
+       (region-active-p)
+       (> (region-end) (region-beginning))
+       (if (or (not select-active-regions-last-region)
+	       (x-selection-owner-p nil))
+	   (let ((latest-region
+		  (buffer-substring (region-beginning) (region-end))))
+	     (unless (string-equal latest-region
+				   select-active-regions-last-region)
+	       (x-set-selection
+		nil latest-region)
+	       (setq select-active-regions-last-region
+		     latest-region)))
+	 (setq select-active-regions-last-region nil))))
+
 (defcustom select-active-regions nil
-  "If non-nil, an active region automatically becomes the window selection."
+  "If non-nil, an active region automatically becomes the window selection.
+
+In conjunction with this, you might want to:
+rebind mouse-2 to `mouse-yank-primary', set `x-select-enable-primary' to nil,
+set `x-select-enable-clipboard' to non-nil, set `mouse-drag-copy-region'
+to nil, and turn on `transient-mark-mode'."
   :type 'boolean
   :group 'killing
   :version "23.1")
 
 (defun set-mark (pos)
   "Set this buffer's mark to POS.  Don't use this function!
 That is to say, don't use this function unless you want
 the user to see that the mark has moved, and you want the previous
@@ -3466,19 +3493,26 @@
 store it in a Lisp variable.  Example:
 
    (let ((beg (point))) (forward-line 1) (delete-region beg (point)))."
 
   (if pos
       (progn
 	(setq mark-active t)
 	(run-hooks 'activate-mark-hook)
-	(and select-active-regions
-	     (x-set-selection
-	      nil (buffer-substring (region-beginning) (region-end))))
+	(if select-active-regions
+	    (progn
+	      (unless select-active-regions-timer
+		(setq select-active-regions-timer
+		      (run-with-idle-timer
+		       0.1 t 'select-active-regions-maybe-set-selection)))
+	      (select-active-regions-maybe-set-selection))
+	  (when select-active-regions-timer
+	    (cancel-timer select-active-regions-timer)
+	    (setq select-active-regions-timer nil)))
 	(set-marker (mark-marker) pos (current-buffer)))
     ;; Normally we never clear mark-active except in Transient Mark mode.
     ;; But when we actually clear out the mark value too,
     ;; we must clear mark-active in any mode.
     (setq mark-active nil)
     (run-hooks 'deactivate-mark-hook)
     (set-marker (mark-marker) nil)))
 

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

* bug#902: select-active-regions only half-working
  2008-09-06  5:53 ` bug#902: select-active-regions only half-working David De La Harpe Golden
       [not found]   ` <handler.902.B.122068042025981.ack@emacsbugs.donarmstrong.com>
@ 2008-09-06 19:50   ` Stefan Monnier
  2008-09-06 20:22     ` David De La Harpe Golden
  2009-07-15  1:40   ` bug#902: marked as done (select-active-regions only half-working) Emacs bug Tracking System
  2 siblings, 1 reply; 16+ messages in thread
From: Stefan Monnier @ 2008-09-06 19:50 UTC (permalink / raw)
  To: David De La Harpe Golden; +Cc: submit, 902

> The select-active-regions in-tree does not work in all cases - it works
> when the region is changed by moving the mark but not when it is changed
> by certain other means e.g. moving the point with the keyboard.
> (Workaround:  C-x C-x C-x C-x ...).

Could you be more precise and give an actual recipe?


        Stefan





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

* bug#902: select-active-regions only half-working
  2008-09-06 19:50   ` bug#902: select-active-regions only half-working Stefan Monnier
@ 2008-09-06 20:22     ` David De La Harpe Golden
  2008-09-07  3:53       ` Stefan Monnier
  0 siblings, 1 reply; 16+ messages in thread
From: David De La Harpe Golden @ 2008-09-06 20:22 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 902

Stefan Monnier wrote:
>> The select-active-regions in-tree does not work in all cases - it works
>> when the region is changed by moving the mark but not when it is changed
>> by certain other means e.g. moving the point with the keyboard.
>> (Workaround:  C-x C-x C-x C-x ...).
> 
> Could you be more precise and give an actual recipe?
> 

With transient-mark-mode and select-active-regions on:

Set mark. (C-SPC)

Move point with keyboard, defining an active region.

Try to middle-click-paste into another application (that
accepts middle clicks to paste in primary, of course)

Note that what is pasted in doesn't correspond to the
active region. It should.

Sorry, this was a known issue pre-bug-tracker, should really have
been filed a long time ago:
http://lists.gnu.org/archive/html/emacs-devel/2008-02/msg00327.html
(includes very similar patches that no longer apply cleanly)

There might still be a related ordering issue in mouse-drag-track too,
that caused annoying off-by-a-little-bit glitches when mouse-selecting:

http://lists.gnu.org/archive/html/emacs-devel/2008-02/msg00316.html






















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

* bug#902: select-active-regions only half-working
  2008-09-06 20:22     ` David De La Harpe Golden
@ 2008-09-07  3:53       ` Stefan Monnier
  2008-09-07 20:27         ` David De La Harpe Golden
  0 siblings, 1 reply; 16+ messages in thread
From: Stefan Monnier @ 2008-09-07  3:53 UTC (permalink / raw)
  To: David De La Harpe Golden; +Cc: 902

>> Could you be more precise and give an actual recipe?

> With transient-mark-mode and select-active-regions on:

> Set mark. (C-SPC)

> Move point with keyboard, defining an active region.

> Try to middle-click-paste into another application (that
> accepts middle clicks to paste in primary, of course)

> Note that what is pasted in doesn't correspond to the
> active region.  It should.

Whether it should or not is very debatable.  Historically, Emacs has
always reuqested the user to select with the mouse or use M-w for that,
and I have seen no evidence that people usually expect Emacs to copy the
region to the X11-selection more eagerly.

Your suggestion might please some people, but I do not think it
should be used by default.  So I suggest you turn your code into
a global minor mode.

> There might still be a related ordering issue in mouse-drag-track too,
> that caused annoying off-by-a-little-bit glitches when mouse-selecting:
> http://lists.gnu.org/archive/html/emacs-devel/2008-02/msg00316.html

I'm not sure I understand.  If it relates to the current code (as
opposed to yours), then please post a separate bug report for it (and
include a recipe, of course).


        Stefan






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

* bug#902: select-active-regions only half-working
  2008-09-07  3:53       ` Stefan Monnier
@ 2008-09-07 20:27         ` David De La Harpe Golden
  2008-09-07 21:20           ` Stefan Monnier
  0 siblings, 1 reply; 16+ messages in thread
From: David De La Harpe Golden @ 2008-09-07 20:27 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 902

Stefan Monnier wrote:

>  Whether it should or not is very debatable.

Shrug. That would be why it is optional, then.

> Historically, Emacs has always reuqested the user to select with the
> mouse or use M-w for that,

Indeed. That is pretty much what select-active-regions was introduced to
address though? - so one can e.g. use M-w for CLIPBOARD yet have PRIMARY
set by user-selection (i.e. establishment of a nonzer-sized active
region) like other apps.

> I do not think it should be used by default.

Who's talking about using it by default at this point? If a user
actually turns select-active-regions on though, it should work. It could
easily enough be recast as a global minor mode I guess. But it's just a
boolean setting in-tree at the moment.
















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

* bug#902: select-active-regions only half-working
  2008-09-07 20:27         ` David De La Harpe Golden
@ 2008-09-07 21:20           ` Stefan Monnier
  2008-09-09  0:42             ` David De La Harpe Golden
  0 siblings, 1 reply; 16+ messages in thread
From: Stefan Monnier @ 2008-09-07 21:20 UTC (permalink / raw)
  To: David De La Harpe Golden; +Cc: 902

Sorry, don't mind me, I was completely confused.


        Stefan








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

* bug#902: select-active-regions only half-working
  2008-09-07 21:20           ` Stefan Monnier
@ 2008-09-09  0:42             ` David De La Harpe Golden
  2008-09-09 14:50               ` Stefan Monnier
  0 siblings, 1 reply; 16+ messages in thread
From: David De La Harpe Golden @ 2008-09-09  0:42 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 902

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

Stefan Monnier wrote:
> Sorry, don't mind me, I was completely confused.
> 
> 
No worries.  Anyway, there's probably a much more elegant way:

(Background: I «gasp» read the docstring for x-set-selection, and
_thought_ I'd found a better way - it can take a cons of markers
to _lazily_ find the selection data as whatever's between
the markers when something requests the selection. However, it turns out
that the emacs point is _not_ in fact a marker, so you can't use
mark-marker and point-marker to find the region on-demand (point-marker
just returns a marker to the instantaneous position of the point))

*** Sooo - Here's a solution that seems generally saner, though does
wander deeper into the emacs core - allow x-set-selection to take a
function that will be funcalled on demand to return a string to use as
the selection data, not just a cons of markers.

Avoids performance issues that the moronic string-equal or hash in the
timer would introduce, and the (theoretical, for inhumanly fast users)
potential flakiness of an idle timer.



















[-- Attachment #2: select-active-regions_lazy_r1.diff --]
[-- Type: text/x-patch, Size: 9863 bytes --]

Index: lisp/select.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/select.el,v
retrieving revision 1.44
diff -U 8 -r1.44 select.el
--- lisp/select.el	12 Jun 2008 03:56:16 -0000	1.44
+++ lisp/select.el	9 Sep 2008 00:35:08 -0000
@@ -123,16 +123,20 @@
 integer (or a cons of two integers or list of two integers).
 
 The selection may also be a cons of two markers pointing to the same buffer,
 or an overlay.  In these cases, the selection is considered to be the text
 between the markers *at whatever time the selection is examined*.
 Thus, editing done in the buffer after you specify the selection
 can alter the effective value of the selection.
 
+The selection may also be a function of one argument that returns a string.
+In that case, the selection is considered to be the string
+returned by the function *at whatever time the selection is examined*.
+
 The data may also be a vector of valid non-vector selection values.
 
 The return value is DATA.
 
 Interactively, this command sets the primary selection.  Without
 prefix argument, it reads the selection in the minibuffer.  With
 prefix argument, it uses the text of the region as the selection value ."
   (interactive (if (not current-prefix-arg)
@@ -170,17 +174,22 @@
       (and (consp data)
 	   (markerp (car data))
 	   (markerp (cdr data))
 	   (marker-buffer (car data))
 	   (marker-buffer (cdr data))
 	   (eq (marker-buffer (car data))
 	       (marker-buffer (cdr data)))
 	   (buffer-name (marker-buffer (car data)))
-	   (buffer-name (marker-buffer (cdr data))))))
+	   (buffer-name (marker-buffer (cdr data))))
+      ;; no real guarantee that an impure function that returns
+      ;; a string now will always do so, but might as well
+      ;; try it out, for early failure.
+      (and (functionp data)
+	   (stringp (funcall data)))))
 \f
 ;;; Cut Buffer support
 
 (declare-function x-get-cut-buffer-internal "xselect.c")
 
 (defun x-get-cut-buffer (&optional which-one)
   "Returns the value of one of the 8 X server cut-buffers.
 Optional arg WHICH-ONE should be a number from 0 to 7, defaulting to 0.
@@ -229,17 +238,25 @@
 		(markerp (cdr value)))
 	   (or (eq (marker-buffer (car value)) (marker-buffer (cdr value)))
 	       (signal 'error
 		       (list "markers must be in the same buffer"
 			     (car value) (cdr value))))
 	   (save-excursion
 	     (set-buffer (or (marker-buffer (car value))
 			     (error "selection is in a killed buffer")))
-	     (setq str (buffer-substring (car value) (cdr value))))))
+	     (setq str (buffer-substring (car value) (cdr value)))))
+
+	  ((functionp value)
+	   (let ((ret (funcall value)))
+	     (if (stringp ret)
+		 (setq str ret)
+	       (signal 'error
+		       (list "selection function must return string"
+			     value ret))))))
 
     (when str
       ;; If TYPE is nil, this is a local request, thus return STR as
       ;; is.  Otherwise, encode STR.
       (if (not type)
 	  str
 	(setq coding (or next-selection-coding-system selection-coding-system))
 	(if coding
@@ -304,17 +321,24 @@
 	       ((and (consp value)
 		     (markerp (car value))
 		     (markerp (cdr value)))
 		(or (eq (marker-buffer (car value))
 			(marker-buffer (cdr value)))
 		    (signal 'error
 			    (list "markers must be in the same buffer"
 				  (car value) (cdr value))))
-		(abs (- (car value) (cdr value)))))))
+		(abs (- (car value) (cdr value))))
+	       ((functionp value)
+		(let ((ret (funcall value)))
+		  (if (stringp ret)
+		      (length ret)
+		    (signal 'error
+			    (list "no selection length found"
+				  value ret))))))))
     (if value ; force it to be in 32-bit format.
 	(cons (ash value -16) (logand value 65535))
       nil)))
 
 (defun xselect-convert-to-targets (selection type value)
   ;; return a vector of atoms, but remove duplicates first.
   (let* ((all (cons 'TIMESTAMP (mapcar 'car selection-converter-alist)))
 	 (rest all))
Index: lisp/simple.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/simple.el,v
retrieving revision 1.945
diff -U 8 -r1.945 simple.el
--- lisp/simple.el	15 Aug 2008 00:30:44 -0000	1.945
+++ lisp/simple.el	9 Sep 2008 00:35:12 -0000
@@ -3416,44 +3416,60 @@
 is active, and returns an integer or nil in the usual way.
 
 If you are using this in an editing command, you are most likely making
 a mistake; see the documentation of `set-mark'."
   (if (or force (not transient-mark-mode) mark-active mark-even-if-inactive)
       (marker-position (mark-marker))
     (signal 'mark-inactive nil)))
 
+(defcustom select-active-regions nil
+  "If non-nil, an active region automatically becomes the window selection.
+
+In conjunction with this, to ape some other X11 apps, you might want to:
+rebind mouse-2 to `mouse-yank-primary', set `x-select-enable-primary' to nil,
+set `x-select-enable-clipboard' to non-nil, set `mouse-drag-copy-region'
+to nil, and turn on `transient-mark-mode'."
+  :type 'boolean
+  :group 'killing
+  :version "23.1")
+
 ;; Many places set mark-active directly, and several of them failed to also
 ;; run deactivate-mark-hook.  This shorthand should simplify.
 (defsubst deactivate-mark ()
   "Deactivate the mark by setting `mark-active' to nil.
 \(That makes a difference only in Transient Mark mode.)
 Also runs the hook `deactivate-mark-hook'."
   (when transient-mark-mode
     (if (or (eq transient-mark-mode 'lambda)
 	    (and (eq (car-safe transient-mark-mode) 'only)
 		 (null (cdr transient-mark-mode))))
 	(setq transient-mark-mode nil)
       (if (eq (car-safe transient-mark-mode) 'only)
 	  (setq transient-mark-mode (cdr transient-mark-mode)))
       (setq mark-active nil)
-      (run-hooks 'deactivate-mark-hook))))
+      (run-hooks 'deactivate-mark-hook))
+    (and select-active-regions
+	 (x-selection-owner-p nil)
+ 	 (< (region-beginning) (region-end))
+	 (x-set-selection
+	  nil (buffer-substring (region-beginning) (region-end))))))
 
 (defun activate-mark ()
   "Activate the mark."
   (when (mark t)
     (setq mark-active t)
     (unless transient-mark-mode
-      (setq transient-mark-mode 'lambda))))
-
-(defcustom select-active-regions nil
-  "If non-nil, an active region automatically becomes the window selection."
-  :type 'boolean
-  :group 'killing
-  :version "23.1")
+      (setq transient-mark-mode 'lambda))
+    (and select-active-regions
+	 (x-set-selection
+	  nil (lambda ()
+		(if (< (region-beginning) (region-end))
+		    (buffer-substring (region-beginning) (region-end))
+		  ""))))))
 
 (defun set-mark (pos)
   "Set this buffer's mark to POS.  Don't use this function!
 That is to say, don't use this function unless you want
 the user to see that the mark has moved, and you want the previous
 mark position to be lost.
 
 Normally, when a new mark is set, the old one should go on the stack.
@@ -3466,20 +3482,28 @@
 store it in a Lisp variable.  Example:
 
    (let ((beg (point))) (forward-line 1) (delete-region beg (point)))."
 
   (if pos
       (progn
 	(setq mark-active t)
 	(run-hooks 'activate-mark-hook)
+	(set-marker (mark-marker) pos (current-buffer))
 	(and select-active-regions
 	     (x-set-selection
-	      nil (buffer-substring (region-beginning) (region-end))))
-	(set-marker (mark-marker) pos (current-buffer)))
+	      nil (lambda ()
+		(if (< (region-beginning) (region-end))
+		    (buffer-substring (region-beginning) (region-end))
+		  "")))))
+    (and mark-active select-active-regions
+	 (< (region-beginning) (region-end))
+	 (x-selection-owner-p nil)
+	 (x-set-selection
+	  nil (buffer-substring (region-beginning) (region-end))))
     ;; Normally we never clear mark-active except in Transient Mark mode.
     ;; But when we actually clear out the mark value too,
     ;; we must clear mark-active in any mode.
     (setq mark-active nil)
     (run-hooks 'deactivate-mark-hook)
     (set-marker (mark-marker) nil)))
 
 (defcustom use-empty-active-region nil
Index: lisp/mouse.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/mouse.el,v
retrieving revision 1.347
diff -U 8 -r1.347 mouse.el
--- lisp/mouse.el	11 Aug 2008 01:23:05 -0000	1.347
+++ lisp/mouse.el	9 Sep 2008 00:35:13 -0000
@@ -906,16 +906,17 @@
 (defun mouse-drag-track (start-event  &optional
 				      do-mouse-drag-region-post-process)
     "Track mouse drags by highlighting area between point and cursor.
 The region will be defined with mark and point, and the overlay
 will be deleted after return.  DO-MOUSE-DRAG-REGION-POST-PROCESS
 should only be used by mouse-drag-region."
   (mouse-minibuffer-check start-event)
   (setq mouse-selection-click-count-buffer (current-buffer))
+  (deactivate-mark)
   (let* ((original-window (selected-window))
          ;; We've recorded what we needed from the current buffer and
          ;; window, now let's jump to the place of the event, where things
          ;; are happening.
          (_ (mouse-set-point start-event))
          (echo-keystrokes 0)
 	 (start-posn (event-start start-event))
 	 (start-point (posn-point start-posn))
@@ -950,17 +951,16 @@
     (if (< (point) start-point)
 	(goto-char start-point))
     (setq start-point (point))
     (if remap-double-click ;; Don't expand mouse overlay in links
 	(setq click-count 0))
     (mouse-move-drag-overlay mouse-drag-overlay start-point start-point
                              click-count)
     (overlay-put mouse-drag-overlay 'window start-window)
-    (deactivate-mark)
     (let (event end end-point last-end-point)
       (track-mouse
 	(while (progn
 		 (setq event (read-event))
                  (or (mouse-movement-p event)
                      (memq (car-safe event) '(switch-frame select-window))))
           (if (memq (car-safe event) '(switch-frame select-window))
 	      nil

[-- Attachment #3: ChangeLog.select-active-regions_lazy_r1 --]
[-- Type: text/plain, Size: 332 bytes --]

2008-09-06 David De La Harpe Golden <david@harpegolden.net>

	* select.el: allow x-set-selection to take a function
		     that is called on-demand to obtain selection data.

	* simple.el: lazy implementation of select-active-regions.

	* mouse.el:  fix time-ordering of deactivate-mark operations
	  	     in mouse drag tracking.


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

* bug#902: select-active-regions only half-working
  2008-09-09  0:42             ` David De La Harpe Golden
@ 2008-09-09 14:50               ` Stefan Monnier
  2008-09-09 19:20                 ` David De La Harpe Golden
  0 siblings, 1 reply; 16+ messages in thread
From: Stefan Monnier @ 2008-09-09 14:50 UTC (permalink / raw)
  To: David De La Harpe Golden; +Cc: 902

>> Sorry, don't mind me, I was completely confused.
> No worries.  Anyway, there's probably a much more elegant way:

> (Background: I «gasp» read the docstring for x-set-selection, and
> _thought_ I'd found a better way - it can take a cons of markers
> to _lazily_ find the selection data as whatever's between
> the markers when something requests the selection. However, it turns out
> that the emacs point is _not_ in fact a marker, so you can't use
> mark-marker and point-marker to find the region on-demand (point-marker
> just returns a marker to the instantaneous position of the point))

> *** Sooo - Here's a solution that seems generally saner, though does
> wander deeper into the emacs core - allow x-set-selection to take a
> function that will be funcalled on demand to return a string to use as
> the selection data, not just a cons of markers.

> Avoids performance issues that the moronic string-equal or hash in the
> timer would introduce, and the (theoretical, for inhumanly fast users)
> potential flakiness of an idle timer.

Sounds good on the surface [ I don't know the insides]

> +      ;; no real guarantee that an impure function that returns
> +      ;; a string now will always do so, but might as well
> +      ;; try it out, for early failure.
> +      (and (functionp data)
> +	   (stringp (funcall data)))))

I wouldn't worry/care about checking the return value here.
 
> +	  ((functionp value)
> +	   (let ((ret (funcall value)))
> +	     (if (stringp ret)
> +		 (setq str ret)
> +	       (signal 'error
> +		       (list "selection function must return string"
> +			     value ret))))))

Please move this code to an auxiliary function, since it's
repeated twice.
 
> +    (and select-active-regions
> +	 (x-set-selection
> +	  nil (lambda ()
> +		(if (< (region-beginning) (region-end))
> +		    (buffer-substring (region-beginning) (region-end))
> +		  ""))))))

You should probably save the current buffer in some variable (current at
the time of the x-set-selection) and restore it when the lambda is
called.  Also, give a name to this function, since it's used at
least twice.

An alternative is to use not a function but a buffer (which would mean
"use the region's content, if active").
 

        Stefan






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

* bug#902: select-active-regions only half-working
  2008-09-09 14:50               ` Stefan Monnier
@ 2008-09-09 19:20                 ` David De La Harpe Golden
  2008-09-10 16:37                   ` Stefan Monnier
  0 siblings, 1 reply; 16+ messages in thread
From: David De La Harpe Golden @ 2008-09-09 19:20 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 902

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

Stefan Monnier wrote:

> An alternative is to use not a function but a buffer (which would mean
> "use the region's content, if active").


Yeah, allowing a function might be a giant bit too open-ended.
Attached please find buffer-passing implementation.









[-- Attachment #2: select-active-regions_lazybuf_r1.diff --]
[-- Type: text/x-patch, Size: 12551 bytes --]

Index: lisp/select.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/select.el,v
retrieving revision 1.44
diff -U 8 -r1.44 select.el
--- lisp/select.el	12 Jun 2008 03:56:16 -0000	1.44
+++ lisp/select.el	9 Sep 2008 19:02:15 -0000
@@ -123,16 +123,20 @@
 integer (or a cons of two integers or list of two integers).
 
 The selection may also be a cons of two markers pointing to the same buffer,
 or an overlay.  In these cases, the selection is considered to be the text
 between the markers *at whatever time the selection is examined*.
 Thus, editing done in the buffer after you specify the selection
 can alter the effective value of the selection.
 
+The selection may also be a buffer object. In that case, the selection is
+considered to be the region between the mark and point, if any, in that
+buffer, *at whatever time the selection is examined*.
+
 The data may also be a vector of valid non-vector selection values.
 
 The return value is DATA.
 
 Interactively, this command sets the primary selection.  Without
 prefix argument, it reads the selection in the minibuffer.  With
 prefix argument, it uses the text of the region as the selection value ."
   (interactive (if (not current-prefix-arg)
@@ -170,17 +174,18 @@
       (and (consp data)
 	   (markerp (car data))
 	   (markerp (cdr data))
 	   (marker-buffer (car data))
 	   (marker-buffer (cdr data))
 	   (eq (marker-buffer (car data))
 	       (marker-buffer (cdr data)))
 	   (buffer-name (marker-buffer (car data)))
-	   (buffer-name (marker-buffer (cdr data))))))
+	   (buffer-name (marker-buffer (cdr data))))
+      (bufferp data)))
 \f
 ;;; Cut Buffer support
 
 (declare-function x-get-cut-buffer-internal "xselect.c")
 
 (defun x-get-cut-buffer (&optional which-one)
   "Returns the value of one of the 8 X server cut-buffers.
 Optional arg WHICH-ONE should be a number from 0 to 7, defaulting to 0.
@@ -229,17 +234,24 @@
 		(markerp (cdr value)))
 	   (or (eq (marker-buffer (car value)) (marker-buffer (cdr value)))
 	       (signal 'error
 		       (list "markers must be in the same buffer"
 			     (car value) (cdr value))))
 	   (save-excursion
 	     (set-buffer (or (marker-buffer (car value))
 			     (error "selection is in a killed buffer")))
-	     (setq str (buffer-substring (car value) (cdr value))))))
+	     (setq str (buffer-substring (car value) (cdr value)))))
+
+	  ((bufferp value)
+	   (save-excursion
+	     (set-buffer value)
+	     (if (and (mark t) (point))
+		 (setq str (buffer-substring (mark t) (point)))
+	       (setq str "")))))
 
     (when str
       ;; If TYPE is nil, this is a local request, thus return STR as
       ;; is.  Otherwise, encode STR.
       (if (not type)
 	  str
 	(setq coding (or next-selection-coding-system selection-coding-system))
 	(if coding
@@ -304,17 +316,23 @@
 	       ((and (consp value)
 		     (markerp (car value))
 		     (markerp (cdr value)))
 		(or (eq (marker-buffer (car value))
 			(marker-buffer (cdr value)))
 		    (signal 'error
 			    (list "markers must be in the same buffer"
 				  (car value) (cdr value))))
-		(abs (- (car value) (cdr value)))))))
+		(abs (- (car value) (cdr value))))
+	       ((bufferp value)
+		(save-excursion
+		  (set-buffer value)
+		  (if (and (mark t) (point))
+		      (abs (- (point) (mark t)))
+		    0))))))
     (if value ; force it to be in 32-bit format.
 	(cons (ash value -16) (logand value 65535))
       nil)))
 
 (defun xselect-convert-to-targets (selection type value)
   ;; return a vector of atoms, but remove duplicates first.
   (let* ((all (cons 'TIMESTAMP (mapcar 'car selection-converter-alist)))
 	 (rest all))
@@ -338,28 +356,36 @@
   (cond ((overlayp value)
 	 (buffer-file-name (or (overlay-buffer value)
 			       (error "selection is in a killed buffer"))))
 	((and (consp value)
 	      (markerp (car value))
 	      (markerp (cdr value)))
 	 (buffer-file-name (or (marker-buffer (car value))
 			       (error "selection is in a killed buffer"))))
+	((bufferp value)
+	 (buffer-file-name value))
 	(t nil)))
 
 (defun xselect-convert-to-charpos (selection type value)
   (let (a b tmp)
     (cond ((cond ((overlayp value)
 		  (setq a (overlay-start value)
 			b (overlay-end value)))
 		 ((and (consp value)
 		       (markerp (car value))
 		       (markerp (cdr value)))
 		  (setq a (car value)
-			b (cdr value))))
+			b (cdr value)))
+		 ((bufferp value)
+		  (save-excursion
+		    (set-buffer value)
+		    (and (mark t) (point)
+			 (setq a (mark t)
+			       b (point))))))
 	   (setq a (1- a) b (1- b)) ; zero-based
 	   (if (< b a) (setq tmp a a b b tmp))
 	   (cons 'SPAN
 		 (vector (cons (ash a -16) (logand a 65535))
 			 (cons (ash b -16) (logand b 65535))))))))
 
 (defun xselect-convert-to-lineno (selection type value)
   (let (a b buf tmp)
@@ -368,16 +394,23 @@
 		       (markerp (cdr value)))
 		  (setq a (marker-position (car value))
 			b (marker-position (cdr value))
 			buf (marker-buffer (car value))))
 		 ((overlayp value)
 		  (setq buf (overlay-buffer value)
 			a (overlay-start value)
 			b (overlay-end value)))
+		 ((bufferp value)
+		  (save-excursion
+		    (set-buffer value)
+		    (and (mark t) (point)
+			 (setq buf value
+			       a (mark t)
+			       b (point)))))
 		 )
 	   (save-excursion
 	     (set-buffer buf)
 	     (setq a (count-lines 1 a)
 		   b (count-lines 1 b)))
 	   (if (< b a) (setq tmp a a b b tmp))
 	   (cons 'SPAN
 		 (vector (cons (ash a -16) (logand a 65535))
@@ -390,16 +423,23 @@
 		       (markerp (cdr value)))
 		  (setq a (car value)
 			b (cdr value)
 			buf (marker-buffer a)))
 		 ((overlayp value)
 		  (setq buf (overlay-buffer value)
 			a (overlay-start value)
 			b (overlay-end value)))
+		 ((bufferp value)
+		  (save-excursion
+		    (set-buffer value)
+		    (and (mark t) (point)
+			 (setq buf value
+			       a (mark t)
+			       b (point)))))
 		 )
 	   (save-excursion
 	     (set-buffer buf)
 	     (goto-char a)
 	     (setq a (current-column))
 	     (goto-char b)
 	     (setq b (current-column)))
 	   (if (< b a) (setq tmp a a b b tmp))
Index: lisp/simple.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/simple.el,v
retrieving revision 1.945
diff -U 8 -r1.945 simple.el
--- lisp/simple.el	15 Aug 2008 00:30:44 -0000	1.945
+++ lisp/simple.el	9 Sep 2008 19:02:18 -0000
@@ -3416,44 +3416,55 @@
 is active, and returns an integer or nil in the usual way.
 
 If you are using this in an editing command, you are most likely making
 a mistake; see the documentation of `set-mark'."
   (if (or force (not transient-mark-mode) mark-active mark-even-if-inactive)
       (marker-position (mark-marker))
     (signal 'mark-inactive nil)))
 
+(defcustom select-active-regions nil
+  "If non-nil, an active region automatically becomes the window selection.
+
+In conjunction with this, to ape some other X11 apps, you might want to:
+rebind mouse-2 to `mouse-yank-primary', set `x-select-enable-primary' to nil,
+set `x-select-enable-clipboard' to non-nil, set `mouse-drag-copy-region'
+to nil, and turn on `transient-mark-mode'."
+  :type 'boolean
+  :group 'killing
+  :version "23.1")
+
 ;; Many places set mark-active directly, and several of them failed to also
 ;; run deactivate-mark-hook.  This shorthand should simplify.
 (defsubst deactivate-mark ()
   "Deactivate the mark by setting `mark-active' to nil.
 \(That makes a difference only in Transient Mark mode.)
 Also runs the hook `deactivate-mark-hook'."
   (when transient-mark-mode
+    (and mark-active select-active-regions
+	  (x-selection-owner-p nil)
+	  (x-set-selection nil (x-get-selection nil)))
     (if (or (eq transient-mark-mode 'lambda)
 	    (and (eq (car-safe transient-mark-mode) 'only)
 		 (null (cdr transient-mark-mode))))
 	(setq transient-mark-mode nil)
       (if (eq (car-safe transient-mark-mode) 'only)
 	  (setq transient-mark-mode (cdr transient-mark-mode)))
       (setq mark-active nil)
       (run-hooks 'deactivate-mark-hook))))
 
 (defun activate-mark ()
   "Activate the mark."
   (when (mark t)
     (setq mark-active t)
     (unless transient-mark-mode
-      (setq transient-mark-mode 'lambda))))
+      (setq transient-mark-mode 'lambda))
+    (when select-active-regions
+      (x-set-selection nil (current-buffer)))))
 
-(defcustom select-active-regions nil
-  "If non-nil, an active region automatically becomes the window selection."
-  :type 'boolean
-  :group 'killing
-  :version "23.1")
 
 (defun set-mark (pos)
   "Set this buffer's mark to POS.  Don't use this function!
 That is to say, don't use this function unless you want
 the user to see that the mark has moved, and you want the previous
 mark position to be lost.
 
 Normally, when a new mark is set, the old one should go on the stack.
@@ -3466,20 +3477,22 @@
 store it in a Lisp variable.  Example:
 
    (let ((beg (point))) (forward-line 1) (delete-region beg (point)))."
 
   (if pos
       (progn
 	(setq mark-active t)
 	(run-hooks 'activate-mark-hook)
-	(and select-active-regions
-	     (x-set-selection
-	      nil (buffer-substring (region-beginning) (region-end))))
-	(set-marker (mark-marker) pos (current-buffer)))
+	(set-marker (mark-marker) pos (current-buffer))
+	(when select-active-regions
+	  (x-set-selection nil (current-buffer))))
+    (and mark-active select-active-regions
+	 (x-selection-owner-p nil)
+	 (x-set-selection nil (x-get-selection nil)))
     ;; Normally we never clear mark-active except in Transient Mark mode.
     ;; But when we actually clear out the mark value too,
     ;; we must clear mark-active in any mode.
     (setq mark-active nil)
     (run-hooks 'deactivate-mark-hook)
     (set-marker (mark-marker) nil)))
 
 (defcustom use-empty-active-region nil
Index: lisp/mouse.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/mouse.el,v
retrieving revision 1.347
diff -U 8 -r1.347 mouse.el
--- lisp/mouse.el	11 Aug 2008 01:23:05 -0000	1.347
+++ lisp/mouse.el	9 Sep 2008 19:02:20 -0000
@@ -906,16 +906,17 @@
 (defun mouse-drag-track (start-event  &optional
 				      do-mouse-drag-region-post-process)
     "Track mouse drags by highlighting area between point and cursor.
 The region will be defined with mark and point, and the overlay
 will be deleted after return.  DO-MOUSE-DRAG-REGION-POST-PROCESS
 should only be used by mouse-drag-region."
   (mouse-minibuffer-check start-event)
   (setq mouse-selection-click-count-buffer (current-buffer))
+  (deactivate-mark)
   (let* ((original-window (selected-window))
          ;; We've recorded what we needed from the current buffer and
          ;; window, now let's jump to the place of the event, where things
          ;; are happening.
          (_ (mouse-set-point start-event))
          (echo-keystrokes 0)
 	 (start-posn (event-start start-event))
 	 (start-point (posn-point start-posn))
@@ -950,17 +951,16 @@
     (if (< (point) start-point)
 	(goto-char start-point))
     (setq start-point (point))
     (if remap-double-click ;; Don't expand mouse overlay in links
 	(setq click-count 0))
     (mouse-move-drag-overlay mouse-drag-overlay start-point start-point
                              click-count)
     (overlay-put mouse-drag-overlay 'window start-window)
-    (deactivate-mark)
     (let (event end end-point last-end-point)
       (track-mouse
 	(while (progn
 		 (setq event (read-event))
                  (or (mouse-movement-p event)
                      (memq (car-safe event) '(switch-frame select-window))))
           (if (memq (car-safe event) '(switch-frame select-window))
 	      nil
@@ -1352,16 +1352,19 @@
 (defun mouse-yank-primary (click)
   "Insert the primary selection at the position clicked on.
 Move point to the end of the inserted text.
 If `mouse-yank-at-point' is non-nil, insert at point
 regardless of where you click."
   (interactive "e")
   ;; Give temporary modes such as isearch a chance to turn off.
   (run-hooks 'mouse-leave-buffer-hook)
+  ;; if region is active and _is_ primary (due to select-active-regions)
+  ;; avoid doubling upon repeated consecutive clicks.
+  (and select-active-regions (deactivate-mark))
   (or mouse-yank-at-point (mouse-set-point click))
   (let ((primary (x-get-selection 'PRIMARY)))
     (if primary
         (insert (x-get-selection 'PRIMARY))
       (error "No primary selection"))))
 
 (defun mouse-kill-ring-save (click)
   "Copy the region between point and the mouse click in the kill ring.

[-- Attachment #3: ChangeLog.select-active-regions_lazybuf_r1 --]
[-- Type: text/plain, Size: 457 bytes --]

2008-09-09 David De La Harpe Golden <david@harpegolden.net>

	* select.el: allow x-set-selection to take a buffer object
		     that is inspected on-demand to obtain selection data
		     from its region.

	* simple.el: lazy implementation of select-active-regions.

	* mouse.el:  fix time-ordering of deactivate-mark operations
	  	     in mouse drag tracking.
		     Avoid double insertion in mouse-yank-primary
		     when select-active-regions is on.



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

* bug#902: select-active-regions only half-working
  2008-09-09 19:20                 ` David De La Harpe Golden
@ 2008-09-10 16:37                   ` Stefan Monnier
  2008-09-10 21:45                     ` David De La Harpe Golden
  0 siblings, 1 reply; 16+ messages in thread
From: Stefan Monnier @ 2008-09-10 16:37 UTC (permalink / raw)
  To: David De La Harpe Golden; +Cc: 902

> +	   (save-excursion
> +	     (set-buffer value)

Aka "(with-current-buffer value".
Again, since this code is present at several places, move it into its
own function.  Basically this function could turn the buffer value into
a "cons of markers" value, so you can use it everywhere and reuse the
code that handles a cons of markers.

Also the select-active-regions docstring shouldn't recommend to turn on
transient-mark-mode since it's already ON by default.


        Stefan






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

* bug#902: select-active-regions only half-working
  2008-09-10 16:37                   ` Stefan Monnier
@ 2008-09-10 21:45                     ` David De La Harpe Golden
  2008-09-11  2:01                       ` Stefan Monnier
  0 siblings, 1 reply; 16+ messages in thread
From: David De La Harpe Golden @ 2008-09-10 21:45 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 902

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

Stefan Monnier wrote:

> Again, since this code is present at several places, move it into its
> own function.  Basically this function could turn the buffer value into
> a "cons of markers" value, so you can use it everywhere and reuse the
> code that handles a cons of markers.
>

Okay, sorry, I seem to have an irrational fear of introducing named
functions in elisp (possibly due to lack of CL packages I'm used to for
hiding). Attached.



[-- Attachment #2: select-active-regions_lazybuf_r2.diff --]
[-- Type: text/x-patch, Size: 11810 bytes --]

Index: lisp/select.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/select.el,v
retrieving revision 1.44
diff -U 8 -r1.44 select.el
--- lisp/select.el	12 Jun 2008 03:56:16 -0000	1.44
+++ lisp/select.el	10 Sep 2008 20:37:34 -0000
@@ -123,16 +123,20 @@
 integer (or a cons of two integers or list of two integers).
 
 The selection may also be a cons of two markers pointing to the same buffer,
 or an overlay.  In these cases, the selection is considered to be the text
 between the markers *at whatever time the selection is examined*.
 Thus, editing done in the buffer after you specify the selection
 can alter the effective value of the selection.
 
+The selection may also be a buffer object. In that case, the selection is
+considered to be the region between the mark and point, if any, in that
+buffer, *at whatever time the selection is examined*.
+
 The data may also be a vector of valid non-vector selection values.
 
 The return value is DATA.
 
 Interactively, this command sets the primary selection.  Without
 prefix argument, it reads the selection in the minibuffer.  With
 prefix argument, it uses the text of the region as the selection value ."
   (interactive (if (not current-prefix-arg)
@@ -170,17 +174,18 @@
       (and (consp data)
 	   (markerp (car data))
 	   (markerp (cdr data))
 	   (marker-buffer (car data))
 	   (marker-buffer (cdr data))
 	   (eq (marker-buffer (car data))
 	       (marker-buffer (cdr data)))
 	   (buffer-name (marker-buffer (car data)))
-	   (buffer-name (marker-buffer (cdr data))))))
+	   (buffer-name (marker-buffer (cdr data))))
+      (bufferp data)))
 \f
 ;;; Cut Buffer support
 
 (declare-function x-get-cut-buffer-internal "xselect.c")
 
 (defun x-get-cut-buffer (&optional which-one)
   "Returns the value of one of the 8 X server cut-buffers.
 Optional arg WHICH-ONE should be a number from 0 to 7, defaulting to 0.
@@ -206,18 +211,25 @@
       (x-rotate-cut-buffers-internal 1))
   (x-store-cut-buffer-internal 'CUT_BUFFER0 string))
 
 \f
 ;;; Functions to convert the selection into various other selection types.
 ;;; Every selection type that Emacs handles is implemented this way, except
 ;;; for TIMESTAMP, which is a special case.
 
+(defun xselect-buffer-region-markers-cons (buffer)
+  "Helper function for xselect-convert functions given buffers. (Internal)"
+  (with-current-buffer buffer
+    (cons (mark-marker) (point-marker))))
+
 (defun xselect-convert-to-string (selection type value)
   (let (str coding)
+    (when (bufferp value)
+      (setq value (xselect-buffer-region-markers-cons value)))
     ;; Get the actual string from VALUE.
     (cond ((stringp value)
 	   (setq str value))
 
 	  ((overlayp value)
 	   (save-excursion
 	     (or (buffer-name (overlay-buffer value))
 		 (error "selection is in a killed buffer"))
@@ -291,16 +303,18 @@
 	    (error "Unknow selection type: %S" type))
 	   )))
 
       (setq next-selection-coding-system nil)
       (cons type str))))
 
 
 (defun xselect-convert-to-length (selection type value)
+  (when (bufferp value)
+    (setq value (xselect-buffer-region-markers-cons value)))
   (let ((value
 	 (cond ((stringp value)
 		(length value))
 	       ((overlayp value)
 		(abs (- (overlay-end value) (overlay-start value))))
 	       ((and (consp value)
 		     (markerp (car value))
 		     (markerp (cdr value)))
@@ -338,35 +352,41 @@
   (cond ((overlayp value)
 	 (buffer-file-name (or (overlay-buffer value)
 			       (error "selection is in a killed buffer"))))
 	((and (consp value)
 	      (markerp (car value))
 	      (markerp (cdr value)))
 	 (buffer-file-name (or (marker-buffer (car value))
 			       (error "selection is in a killed buffer"))))
+	((bufferp value)
+	 (buffer-file-name value))
 	(t nil)))
 
 (defun xselect-convert-to-charpos (selection type value)
+  (when (bufferp value)
+    (setq value (xselect-buffer-region-markers-cons value)))
   (let (a b tmp)
     (cond ((cond ((overlayp value)
 		  (setq a (overlay-start value)
 			b (overlay-end value)))
 		 ((and (consp value)
 		       (markerp (car value))
 		       (markerp (cdr value)))
 		  (setq a (car value)
 			b (cdr value))))
 	   (setq a (1- a) b (1- b)) ; zero-based
 	   (if (< b a) (setq tmp a a b b tmp))
 	   (cons 'SPAN
 		 (vector (cons (ash a -16) (logand a 65535))
 			 (cons (ash b -16) (logand b 65535))))))))
 
 (defun xselect-convert-to-lineno (selection type value)
+  (when (bufferp value)
+    (setq value (xselect-buffer-region-markers-cons value)))
   (let (a b buf tmp)
     (cond ((cond ((and (consp value)
 		       (markerp (car value))
 		       (markerp (cdr value)))
 		  (setq a (marker-position (car value))
 			b (marker-position (cdr value))
 			buf (marker-buffer (car value))))
 		 ((overlayp value)
@@ -379,16 +399,18 @@
 	     (setq a (count-lines 1 a)
 		   b (count-lines 1 b)))
 	   (if (< b a) (setq tmp a a b b tmp))
 	   (cons 'SPAN
 		 (vector (cons (ash a -16) (logand a 65535))
 			 (cons (ash b -16) (logand b 65535))))))))
 
 (defun xselect-convert-to-colno (selection type value)
+  (when (bufferp value)
+    (setq value (xselect-buffer-region-markers-cons value)))
   (let (a b buf tmp)
     (cond ((cond ((and (consp value)
 		       (markerp (car value))
 		       (markerp (cdr value)))
 		  (setq a (car value)
 			b (cdr value)
 			buf (marker-buffer a)))
 		 ((overlayp value)
Index: lisp/simple.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/simple.el,v
retrieving revision 1.945
diff -U 8 -r1.945 simple.el
--- lisp/simple.el	15 Aug 2008 00:30:44 -0000	1.945
+++ lisp/simple.el	10 Sep 2008 20:37:38 -0000
@@ -3416,44 +3416,55 @@
 is active, and returns an integer or nil in the usual way.
 
 If you are using this in an editing command, you are most likely making
 a mistake; see the documentation of `set-mark'."
   (if (or force (not transient-mark-mode) mark-active mark-even-if-inactive)
       (marker-position (mark-marker))
     (signal 'mark-inactive nil)))
 
+(defcustom select-active-regions nil
+  "If non-nil, an active region automatically becomes the window selection.
+
+In conjunction with this, to ape some other X11 apps, you might want to:
+rebind mouse-2 to `mouse-yank-primary', set `x-select-enable-primary' to nil,
+set `x-select-enable-clipboard' to non-nil and set `mouse-drag-copy-region'
+to nil."
+  :type 'boolean
+  :group 'killing
+  :version "23.1")
+
 ;; Many places set mark-active directly, and several of them failed to also
 ;; run deactivate-mark-hook.  This shorthand should simplify.
 (defsubst deactivate-mark ()
   "Deactivate the mark by setting `mark-active' to nil.
 \(That makes a difference only in Transient Mark mode.)
 Also runs the hook `deactivate-mark-hook'."
   (when transient-mark-mode
+    (and mark-active select-active-regions
+	  (x-selection-owner-p nil)
+	  (x-set-selection nil (x-get-selection nil)))
     (if (or (eq transient-mark-mode 'lambda)
 	    (and (eq (car-safe transient-mark-mode) 'only)
 		 (null (cdr transient-mark-mode))))
 	(setq transient-mark-mode nil)
       (if (eq (car-safe transient-mark-mode) 'only)
 	  (setq transient-mark-mode (cdr transient-mark-mode)))
       (setq mark-active nil)
       (run-hooks 'deactivate-mark-hook))))
 
 (defun activate-mark ()
   "Activate the mark."
   (when (mark t)
     (setq mark-active t)
     (unless transient-mark-mode
-      (setq transient-mark-mode 'lambda))))
+      (setq transient-mark-mode 'lambda))
+    (when select-active-regions
+      (x-set-selection nil (current-buffer)))))
 
-(defcustom select-active-regions nil
-  "If non-nil, an active region automatically becomes the window selection."
-  :type 'boolean
-  :group 'killing
-  :version "23.1")
 
 (defun set-mark (pos)
   "Set this buffer's mark to POS.  Don't use this function!
 That is to say, don't use this function unless you want
 the user to see that the mark has moved, and you want the previous
 mark position to be lost.
 
 Normally, when a new mark is set, the old one should go on the stack.
@@ -3466,20 +3477,22 @@
 store it in a Lisp variable.  Example:
 
    (let ((beg (point))) (forward-line 1) (delete-region beg (point)))."
 
   (if pos
       (progn
 	(setq mark-active t)
 	(run-hooks 'activate-mark-hook)
-	(and select-active-regions
-	     (x-set-selection
-	      nil (buffer-substring (region-beginning) (region-end))))
-	(set-marker (mark-marker) pos (current-buffer)))
+	(set-marker (mark-marker) pos (current-buffer))
+	(when select-active-regions
+	  (x-set-selection nil (current-buffer))))
+    (and mark-active select-active-regions
+	 (x-selection-owner-p nil)
+	 (x-set-selection nil (x-get-selection nil)))
     ;; Normally we never clear mark-active except in Transient Mark mode.
     ;; But when we actually clear out the mark value too,
     ;; we must clear mark-active in any mode.
     (setq mark-active nil)
     (run-hooks 'deactivate-mark-hook)
     (set-marker (mark-marker) nil)))
 
 (defcustom use-empty-active-region nil
Index: lisp/mouse.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/mouse.el,v
retrieving revision 1.347
diff -U 8 -r1.347 mouse.el
--- lisp/mouse.el	11 Aug 2008 01:23:05 -0000	1.347
+++ lisp/mouse.el	10 Sep 2008 20:37:39 -0000
@@ -906,16 +906,17 @@
 (defun mouse-drag-track (start-event  &optional
 				      do-mouse-drag-region-post-process)
     "Track mouse drags by highlighting area between point and cursor.
 The region will be defined with mark and point, and the overlay
 will be deleted after return.  DO-MOUSE-DRAG-REGION-POST-PROCESS
 should only be used by mouse-drag-region."
   (mouse-minibuffer-check start-event)
   (setq mouse-selection-click-count-buffer (current-buffer))
+  (deactivate-mark)
   (let* ((original-window (selected-window))
          ;; We've recorded what we needed from the current buffer and
          ;; window, now let's jump to the place of the event, where things
          ;; are happening.
          (_ (mouse-set-point start-event))
          (echo-keystrokes 0)
 	 (start-posn (event-start start-event))
 	 (start-point (posn-point start-posn))
@@ -950,17 +951,16 @@
     (if (< (point) start-point)
 	(goto-char start-point))
     (setq start-point (point))
     (if remap-double-click ;; Don't expand mouse overlay in links
 	(setq click-count 0))
     (mouse-move-drag-overlay mouse-drag-overlay start-point start-point
                              click-count)
     (overlay-put mouse-drag-overlay 'window start-window)
-    (deactivate-mark)
     (let (event end end-point last-end-point)
       (track-mouse
 	(while (progn
 		 (setq event (read-event))
                  (or (mouse-movement-p event)
                      (memq (car-safe event) '(switch-frame select-window))))
           (if (memq (car-safe event) '(switch-frame select-window))
 	      nil
@@ -1352,16 +1352,19 @@
 (defun mouse-yank-primary (click)
   "Insert the primary selection at the position clicked on.
 Move point to the end of the inserted text.
 If `mouse-yank-at-point' is non-nil, insert at point
 regardless of where you click."
   (interactive "e")
   ;; Give temporary modes such as isearch a chance to turn off.
   (run-hooks 'mouse-leave-buffer-hook)
+  ;; if region is active and _is_ primary (due to select-active-regions)
+  ;; avoid doubling upon repeated consecutive clicks.
+  (and select-active-regions (deactivate-mark))
   (or mouse-yank-at-point (mouse-set-point click))
   (let ((primary (x-get-selection 'PRIMARY)))
     (if primary
         (insert (x-get-selection 'PRIMARY))
       (error "No primary selection"))))
 
 (defun mouse-kill-ring-save (click)
   "Copy the region between point and the mouse click in the kill ring.

[-- Attachment #3: ChangeLog.select-active-regions_lazybuf_r2 --]
[-- Type: text/plain, Size: 425 bytes --]

2008-09-10  David De La Harpe Golden  <david@harpegolden.net>

	* select.el: allow x-set-selection to take a buffer object that is
	inspected on-demand to obtain selection data from its region.
	* simple.el: lazy implementation of select-active-regions.
	* mouse.el: fix time-ordering of deactivate-mark operations in
	mouse drag tracking. Avoid doubling of selection in
	mouse-yank-primary when select-active-regions is on.

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

* bug#902: select-active-regions only half-working
  2008-09-10 21:45                     ` David De La Harpe Golden
@ 2008-09-11  2:01                       ` Stefan Monnier
  2008-09-11  2:40                         ` David De La Harpe Golden
  2009-01-25 23:44                         ` David De La Harpe Golden
  0 siblings, 2 replies; 16+ messages in thread
From: Stefan Monnier @ 2008-09-11  2:01 UTC (permalink / raw)
  To: David De La Harpe Golden; +Cc: 902

>> Again, since this code is present at several places, move it into its
>> own function.  Basically this function could turn the buffer value into
>> a "cons of markers" value, so you can use it everywhere and reuse the
>> code that handles a cons of markers.

> Okay, sorry, I seem to have an irrational fear of introducing named
> functions in elisp (possibly due to lack of CL packages I'm used to for
> hiding).  Attached.

You can use the double dash convention (i.e. "<prefix>--") to indicate
that this function is "for internal use".


        Stefan


> Index: lisp/select.el
> ===================================================================
> RCS file: /sources/emacs/emacs/lisp/select.el,v
> retrieving revision 1.44
> diff -U 8 -r1.44 select.el
> --- lisp/select.el	12 Jun 2008 03:56:16 -0000	1.44
> +++ lisp/select.el	10 Sep 2008 20:37:34 -0000
> @@ -123,16 +123,20 @@
>  integer (or a cons of two integers or list of two integers).
 
>  The selection may also be a cons of two markers pointing to the same buffer,
>  or an overlay.  In these cases, the selection is considered to be the text
>  between the markers *at whatever time the selection is examined*.
>  Thus, editing done in the buffer after you specify the selection
>  can alter the effective value of the selection.
 
> +The selection may also be a buffer object. In that case, the selection is
> +considered to be the region between the mark and point, if any, in that
> +buffer, *at whatever time the selection is examined*.
> +
>  The data may also be a vector of valid non-vector selection values.
 
>  The return value is DATA.
 
>  Interactively, this command sets the primary selection.  Without
>  prefix argument, it reads the selection in the minibuffer.  With
>  prefix argument, it uses the text of the region as the selection value ."
>    (interactive (if (not current-prefix-arg)
> @@ -170,17 +174,18 @@
>        (and (consp data)
>  	   (markerp (car data))
>  	   (markerp (cdr data))
>  	   (marker-buffer (car data))
>  	   (marker-buffer (cdr data))
>  	   (eq (marker-buffer (car data))
>  	       (marker-buffer (cdr data)))
>  	   (buffer-name (marker-buffer (car data)))
> -	   (buffer-name (marker-buffer (cdr data))))))
> +	   (buffer-name (marker-buffer (cdr data))))
> +      (bufferp data)))
>  \f
>  ;;; Cut Buffer support
 
>  (declare-function x-get-cut-buffer-internal "xselect.c")
 
>  (defun x-get-cut-buffer (&optional which-one)
>    "Returns the value of one of the 8 X server cut-buffers.
>  Optional arg WHICH-ONE should be a number from 0 to 7, defaulting to 0.
> @@ -206,18 +211,25 @@
>        (x-rotate-cut-buffers-internal 1))
>    (x-store-cut-buffer-internal 'CUT_BUFFER0 string))
 
>  \f
>  ;;; Functions to convert the selection into various other selection types.
>  ;;; Every selection type that Emacs handles is implemented this way, except
>  ;;; for TIMESTAMP, which is a special case.
 
> +(defun xselect-buffer-region-markers-cons (buffer)
> +  "Helper function for xselect-convert functions given buffers. (Internal)"
> +  (with-current-buffer buffer
> +    (cons (mark-marker) (point-marker))))
> +
>  (defun xselect-convert-to-string (selection type value)
>    (let (str coding)
> +    (when (bufferp value)
> +      (setq value (xselect-buffer-region-markers-cons value)))
>      ;; Get the actual string from VALUE.
>      (cond ((stringp value)
>  	   (setq str value))
 
>  	  ((overlayp value)
>  	   (save-excursion
>  	     (or (buffer-name (overlay-buffer value))
>  		 (error "selection is in a killed buffer"))
> @@ -291,16 +303,18 @@
>  	    (error "Unknow selection type: %S" type))
>  	   )))
 
>        (setq next-selection-coding-system nil)
>        (cons type str))))
 
 
>  (defun xselect-convert-to-length (selection type value)
> +  (when (bufferp value)
> +    (setq value (xselect-buffer-region-markers-cons value)))
>    (let ((value
>  	 (cond ((stringp value)
>  		(length value))
>  	       ((overlayp value)
>  		(abs (- (overlay-end value) (overlay-start value))))
>  	       ((and (consp value)
>  		     (markerp (car value))
>  		     (markerp (cdr value)))
> @@ -338,35 +352,41 @@
>    (cond ((overlayp value)
>  	 (buffer-file-name (or (overlay-buffer value)
>  			       (error "selection is in a killed buffer"))))
>  	((and (consp value)
>  	      (markerp (car value))
>  	      (markerp (cdr value)))
>  	 (buffer-file-name (or (marker-buffer (car value))
>  			       (error "selection is in a killed buffer"))))
> +	((bufferp value)
> +	 (buffer-file-name value))
>  	(t nil)))
 
>  (defun xselect-convert-to-charpos (selection type value)
> +  (when (bufferp value)
> +    (setq value (xselect-buffer-region-markers-cons value)))
>    (let (a b tmp)
>      (cond ((cond ((overlayp value)
>  		  (setq a (overlay-start value)
>  			b (overlay-end value)))
>  		 ((and (consp value)
>  		       (markerp (car value))
>  		       (markerp (cdr value)))
>  		  (setq a (car value)
>  			b (cdr value))))
>  	   (setq a (1- a) b (1- b)) ; zero-based
>  	   (if (< b a) (setq tmp a a b b tmp))
>  	   (cons 'SPAN
>  		 (vector (cons (ash a -16) (logand a 65535))
>  			 (cons (ash b -16) (logand b 65535))))))))
 
>  (defun xselect-convert-to-lineno (selection type value)
> +  (when (bufferp value)
> +    (setq value (xselect-buffer-region-markers-cons value)))
>    (let (a b buf tmp)
>      (cond ((cond ((and (consp value)
>  		       (markerp (car value))
>  		       (markerp (cdr value)))
>  		  (setq a (marker-position (car value))
>  			b (marker-position (cdr value))
>  			buf (marker-buffer (car value))))
>  		 ((overlayp value)
> @@ -379,16 +399,18 @@
>  	     (setq a (count-lines 1 a)
>  		   b (count-lines 1 b)))
>  	   (if (< b a) (setq tmp a a b b tmp))
>  	   (cons 'SPAN
>  		 (vector (cons (ash a -16) (logand a 65535))
>  			 (cons (ash b -16) (logand b 65535))))))))
 
>  (defun xselect-convert-to-colno (selection type value)
> +  (when (bufferp value)
> +    (setq value (xselect-buffer-region-markers-cons value)))
>    (let (a b buf tmp)
>      (cond ((cond ((and (consp value)
>  		       (markerp (car value))
>  		       (markerp (cdr value)))
>  		  (setq a (car value)
>  			b (cdr value)
>  			buf (marker-buffer a)))
>  		 ((overlayp value)
> Index: lisp/simple.el
> ===================================================================
> RCS file: /sources/emacs/emacs/lisp/simple.el,v
> retrieving revision 1.945
> diff -U 8 -r1.945 simple.el
> --- lisp/simple.el	15 Aug 2008 00:30:44 -0000	1.945
> +++ lisp/simple.el	10 Sep 2008 20:37:38 -0000
> @@ -3416,44 +3416,55 @@
>  is active, and returns an integer or nil in the usual way.
 
>  If you are using this in an editing command, you are most likely making
>  a mistake; see the documentation of `set-mark'."
>    (if (or force (not transient-mark-mode) mark-active mark-even-if-inactive)
>        (marker-position (mark-marker))
>      (signal 'mark-inactive nil)))
 
> +(defcustom select-active-regions nil
> +  "If non-nil, an active region automatically becomes the window selection.
> +
> +In conjunction with this, to ape some other X11 apps, you might want to:
> +rebind mouse-2 to `mouse-yank-primary', set `x-select-enable-primary' to nil,
> +set `x-select-enable-clipboard' to non-nil and set `mouse-drag-copy-region'
> +to nil."
> +  :type 'boolean
> +  :group 'killing
> +  :version "23.1")
> +
>  ;; Many places set mark-active directly, and several of them failed to also
>  ;; run deactivate-mark-hook.  This shorthand should simplify.
>  (defsubst deactivate-mark ()
>    "Deactivate the mark by setting `mark-active' to nil.
>  \(That makes a difference only in Transient Mark mode.)
>  Also runs the hook `deactivate-mark-hook'."
>    (when transient-mark-mode
> +    (and mark-active select-active-regions
> +	  (x-selection-owner-p nil)
> +	  (x-set-selection nil (x-get-selection nil)))
>      (if (or (eq transient-mark-mode 'lambda)
>  	    (and (eq (car-safe transient-mark-mode) 'only)
>  		 (null (cdr transient-mark-mode))))
>  	(setq transient-mark-mode nil)
>        (if (eq (car-safe transient-mark-mode) 'only)
>  	  (setq transient-mark-mode (cdr transient-mark-mode)))
>        (setq mark-active nil)
>        (run-hooks 'deactivate-mark-hook))))
 
>  (defun activate-mark ()
>    "Activate the mark."
>    (when (mark t)
>      (setq mark-active t)
>      (unless transient-mark-mode
> -      (setq transient-mark-mode 'lambda))))
> +      (setq transient-mark-mode 'lambda))
> +    (when select-active-regions
> +      (x-set-selection nil (current-buffer)))))
 
> -(defcustom select-active-regions nil
> -  "If non-nil, an active region automatically becomes the window selection."
> -  :type 'boolean
> -  :group 'killing
> -  :version "23.1")
 
>  (defun set-mark (pos)
>    "Set this buffer's mark to POS.  Don't use this function!
>  That is to say, don't use this function unless you want
>  the user to see that the mark has moved, and you want the previous
>  mark position to be lost.
 
>  Normally, when a new mark is set, the old one should go on the stack.
> @@ -3466,20 +3477,22 @@
>  store it in a Lisp variable.  Example:
 
>     (let ((beg (point))) (forward-line 1) (delete-region beg (point)))."
 
>    (if pos
>        (progn
>  	(setq mark-active t)
>  	(run-hooks 'activate-mark-hook)
> -	(and select-active-regions
> -	     (x-set-selection
> -	      nil (buffer-substring (region-beginning) (region-end))))
> -	(set-marker (mark-marker) pos (current-buffer)))
> +	(set-marker (mark-marker) pos (current-buffer))
> +	(when select-active-regions
> +	  (x-set-selection nil (current-buffer))))
> +    (and mark-active select-active-regions
> +	 (x-selection-owner-p nil)
> +	 (x-set-selection nil (x-get-selection nil)))
>      ;; Normally we never clear mark-active except in Transient Mark mode.
>      ;; But when we actually clear out the mark value too,
>      ;; we must clear mark-active in any mode.
>      (setq mark-active nil)
>      (run-hooks 'deactivate-mark-hook)
>      (set-marker (mark-marker) nil)))
 
>  (defcustom use-empty-active-region nil
> Index: lisp/mouse.el
> ===================================================================
> RCS file: /sources/emacs/emacs/lisp/mouse.el,v
> retrieving revision 1.347
> diff -U 8 -r1.347 mouse.el
> --- lisp/mouse.el	11 Aug 2008 01:23:05 -0000	1.347
> +++ lisp/mouse.el	10 Sep 2008 20:37:39 -0000
> @@ -906,16 +906,17 @@
>  (defun mouse-drag-track (start-event  &optional
>  				      do-mouse-drag-region-post-process)
>      "Track mouse drags by highlighting area between point and cursor.
>  The region will be defined with mark and point, and the overlay
>  will be deleted after return.  DO-MOUSE-DRAG-REGION-POST-PROCESS
>  should only be used by mouse-drag-region."
>    (mouse-minibuffer-check start-event)
>    (setq mouse-selection-click-count-buffer (current-buffer))
> +  (deactivate-mark)
>    (let* ((original-window (selected-window))
>           ;; We've recorded what we needed from the current buffer and
>           ;; window, now let's jump to the place of the event, where things
>           ;; are happening.
>           (_ (mouse-set-point start-event))
>           (echo-keystrokes 0)
>  	 (start-posn (event-start start-event))
>  	 (start-point (posn-point start-posn))
> @@ -950,17 +951,16 @@
>      (if (< (point) start-point)
>  	(goto-char start-point))
>      (setq start-point (point))
>      (if remap-double-click ;; Don't expand mouse overlay in links
>  	(setq click-count 0))
>      (mouse-move-drag-overlay mouse-drag-overlay start-point start-point
>                               click-count)
>      (overlay-put mouse-drag-overlay 'window start-window)
> -    (deactivate-mark)
>      (let (event end end-point last-end-point)
>        (track-mouse
>  	(while (progn
>  		 (setq event (read-event))
>                   (or (mouse-movement-p event)
>                       (memq (car-safe event) '(switch-frame select-window))))
>            (if (memq (car-safe event) '(switch-frame select-window))
>  	      nil
> @@ -1352,16 +1352,19 @@
>  (defun mouse-yank-primary (click)
>    "Insert the primary selection at the position clicked on.
>  Move point to the end of the inserted text.
>  If `mouse-yank-at-point' is non-nil, insert at point
>  regardless of where you click."
>    (interactive "e")
>    ;; Give temporary modes such as isearch a chance to turn off.
>    (run-hooks 'mouse-leave-buffer-hook)
> +  ;; if region is active and _is_ primary (due to select-active-regions)
> +  ;; avoid doubling upon repeated consecutive clicks.
> +  (and select-active-regions (deactivate-mark))
>    (or mouse-yank-at-point (mouse-set-point click))
>    (let ((primary (x-get-selection 'PRIMARY)))
>      (if primary
>          (insert (x-get-selection 'PRIMARY))
>        (error "No primary selection"))))
 
>  (defun mouse-kill-ring-save (click)
>    "Copy the region between point and the mouse click in the kill ring.

> 2008-09-10  David De La Harpe Golden  <david@harpegolden.net>

> 	* select.el: allow x-set-selection to take a buffer object that is
> 	inspected on-demand to obtain selection data from its region.
> 	* simple.el: lazy implementation of select-active-regions.
> 	* mouse.el: fix time-ordering of deactivate-mark operations in
> 	mouse drag tracking. Avoid doubling of selection in
> 	mouse-yank-primary when select-active-regions is on.






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

* bug#902: select-active-regions only half-working
  2008-09-11  2:01                       ` Stefan Monnier
@ 2008-09-11  2:40                         ` David De La Harpe Golden
  2009-01-25 23:44                         ` David De La Harpe Golden
  1 sibling, 0 replies; 16+ messages in thread
From: David De La Harpe Golden @ 2008-09-11  2:40 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 902

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

> 
> You can use the double dash convention (i.e. "<prefix>--") to indicate
> that this function is "for internal use".
> 

Sorry, my bad, wasn't aware of that convention.  Though maybe it should
be added to Appendix D of the manual (doc/lispref/tips.texi)?

[revision using -- attached]


[-- Attachment #2: select-active-regions_lazybuf_r3.diff --]
[-- Type: text/x-patch, Size: 11739 bytes --]

Index: lisp/select.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/select.el,v
retrieving revision 1.44
diff -U 8 -r1.44 select.el
--- lisp/select.el	12 Jun 2008 03:56:16 -0000	1.44
+++ lisp/select.el	11 Sep 2008 02:19:45 -0000
@@ -123,16 +123,20 @@
 integer (or a cons of two integers or list of two integers).
 
 The selection may also be a cons of two markers pointing to the same buffer,
 or an overlay.  In these cases, the selection is considered to be the text
 between the markers *at whatever time the selection is examined*.
 Thus, editing done in the buffer after you specify the selection
 can alter the effective value of the selection.
 
+The selection may also be a buffer object. In that case, the selection is
+considered to be the region between the mark and point, if any, in that
+buffer, *at whatever time the selection is examined*.
+
 The data may also be a vector of valid non-vector selection values.
 
 The return value is DATA.
 
 Interactively, this command sets the primary selection.  Without
 prefix argument, it reads the selection in the minibuffer.  With
 prefix argument, it uses the text of the region as the selection value ."
   (interactive (if (not current-prefix-arg)
@@ -170,17 +174,18 @@
       (and (consp data)
 	   (markerp (car data))
 	   (markerp (cdr data))
 	   (marker-buffer (car data))
 	   (marker-buffer (cdr data))
 	   (eq (marker-buffer (car data))
 	       (marker-buffer (cdr data)))
 	   (buffer-name (marker-buffer (car data)))
-	   (buffer-name (marker-buffer (cdr data))))))
+	   (buffer-name (marker-buffer (cdr data))))
+      (bufferp data)))
 \f
 ;;; Cut Buffer support
 
 (declare-function x-get-cut-buffer-internal "xselect.c")
 
 (defun x-get-cut-buffer (&optional which-one)
   "Returns the value of one of the 8 X server cut-buffers.
 Optional arg WHICH-ONE should be a number from 0 to 7, defaulting to 0.
@@ -206,18 +211,24 @@
       (x-rotate-cut-buffers-internal 1))
   (x-store-cut-buffer-internal 'CUT_BUFFER0 string))
 
 \f
 ;;; Functions to convert the selection into various other selection types.
 ;;; Every selection type that Emacs handles is implemented this way, except
 ;;; for TIMESTAMP, which is a special case.
 
+(defun xselect--buffer-region-markers-cons (buffer)
+  (with-current-buffer buffer
+    (cons (mark-marker) (point-marker))))
+
 (defun xselect-convert-to-string (selection type value)
   (let (str coding)
+    (when (bufferp value)
+      (setq value (xselect--buffer-region-markers-cons value)))
     ;; Get the actual string from VALUE.
     (cond ((stringp value)
 	   (setq str value))
 
 	  ((overlayp value)
 	   (save-excursion
 	     (or (buffer-name (overlay-buffer value))
 		 (error "selection is in a killed buffer"))
@@ -291,16 +302,18 @@
 	    (error "Unknow selection type: %S" type))
 	   )))
 
       (setq next-selection-coding-system nil)
       (cons type str))))
 
 
 (defun xselect-convert-to-length (selection type value)
+  (when (bufferp value)
+    (setq value (xselect--buffer-region-markers-cons value)))
   (let ((value
 	 (cond ((stringp value)
 		(length value))
 	       ((overlayp value)
 		(abs (- (overlay-end value) (overlay-start value))))
 	       ((and (consp value)
 		     (markerp (car value))
 		     (markerp (cdr value)))
@@ -338,35 +351,41 @@
   (cond ((overlayp value)
 	 (buffer-file-name (or (overlay-buffer value)
 			       (error "selection is in a killed buffer"))))
 	((and (consp value)
 	      (markerp (car value))
 	      (markerp (cdr value)))
 	 (buffer-file-name (or (marker-buffer (car value))
 			       (error "selection is in a killed buffer"))))
+	((bufferp value)
+	 (buffer-file-name value))
 	(t nil)))
 
 (defun xselect-convert-to-charpos (selection type value)
+  (when (bufferp value)
+    (setq value (xselect--buffer-region-markers-cons value)))
   (let (a b tmp)
     (cond ((cond ((overlayp value)
 		  (setq a (overlay-start value)
 			b (overlay-end value)))
 		 ((and (consp value)
 		       (markerp (car value))
 		       (markerp (cdr value)))
 		  (setq a (car value)
 			b (cdr value))))
 	   (setq a (1- a) b (1- b)) ; zero-based
 	   (if (< b a) (setq tmp a a b b tmp))
 	   (cons 'SPAN
 		 (vector (cons (ash a -16) (logand a 65535))
 			 (cons (ash b -16) (logand b 65535))))))))
 
 (defun xselect-convert-to-lineno (selection type value)
+  (when (bufferp value)
+    (setq value (xselect--buffer-region-markers-cons value)))
   (let (a b buf tmp)
     (cond ((cond ((and (consp value)
 		       (markerp (car value))
 		       (markerp (cdr value)))
 		  (setq a (marker-position (car value))
 			b (marker-position (cdr value))
 			buf (marker-buffer (car value))))
 		 ((overlayp value)
@@ -379,16 +398,18 @@
 	     (setq a (count-lines 1 a)
 		   b (count-lines 1 b)))
 	   (if (< b a) (setq tmp a a b b tmp))
 	   (cons 'SPAN
 		 (vector (cons (ash a -16) (logand a 65535))
 			 (cons (ash b -16) (logand b 65535))))))))
 
 (defun xselect-convert-to-colno (selection type value)
+  (when (bufferp value)
+    (setq value (xselect--buffer-region-markers-cons value)))
   (let (a b buf tmp)
     (cond ((cond ((and (consp value)
 		       (markerp (car value))
 		       (markerp (cdr value)))
 		  (setq a (car value)
 			b (cdr value)
 			buf (marker-buffer a)))
 		 ((overlayp value)
Index: lisp/simple.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/simple.el,v
retrieving revision 1.945
diff -U 8 -r1.945 simple.el
--- lisp/simple.el	15 Aug 2008 00:30:44 -0000	1.945
+++ lisp/simple.el	11 Sep 2008 02:19:49 -0000
@@ -3416,44 +3416,55 @@
 is active, and returns an integer or nil in the usual way.
 
 If you are using this in an editing command, you are most likely making
 a mistake; see the documentation of `set-mark'."
   (if (or force (not transient-mark-mode) mark-active mark-even-if-inactive)
       (marker-position (mark-marker))
     (signal 'mark-inactive nil)))
 
+(defcustom select-active-regions nil
+  "If non-nil, an active region automatically becomes the window selection.
+
+In conjunction with this, to ape some other X11 apps, you might want to:
+rebind mouse-2 to `mouse-yank-primary', set `x-select-enable-primary' to nil,
+set `x-select-enable-clipboard' to non-nil and set `mouse-drag-copy-region'
+to nil."
+  :type 'boolean
+  :group 'killing
+  :version "23.1")
+
 ;; Many places set mark-active directly, and several of them failed to also
 ;; run deactivate-mark-hook.  This shorthand should simplify.
 (defsubst deactivate-mark ()
   "Deactivate the mark by setting `mark-active' to nil.
 \(That makes a difference only in Transient Mark mode.)
 Also runs the hook `deactivate-mark-hook'."
   (when transient-mark-mode
+    (and mark-active select-active-regions
+	  (x-selection-owner-p nil)
+	  (x-set-selection nil (x-get-selection nil)))
     (if (or (eq transient-mark-mode 'lambda)
 	    (and (eq (car-safe transient-mark-mode) 'only)
 		 (null (cdr transient-mark-mode))))
 	(setq transient-mark-mode nil)
       (if (eq (car-safe transient-mark-mode) 'only)
 	  (setq transient-mark-mode (cdr transient-mark-mode)))
       (setq mark-active nil)
       (run-hooks 'deactivate-mark-hook))))
 
 (defun activate-mark ()
   "Activate the mark."
   (when (mark t)
     (setq mark-active t)
     (unless transient-mark-mode
-      (setq transient-mark-mode 'lambda))))
+      (setq transient-mark-mode 'lambda))
+    (when select-active-regions
+      (x-set-selection nil (current-buffer)))))
 
-(defcustom select-active-regions nil
-  "If non-nil, an active region automatically becomes the window selection."
-  :type 'boolean
-  :group 'killing
-  :version "23.1")
 
 (defun set-mark (pos)
   "Set this buffer's mark to POS.  Don't use this function!
 That is to say, don't use this function unless you want
 the user to see that the mark has moved, and you want the previous
 mark position to be lost.
 
 Normally, when a new mark is set, the old one should go on the stack.
@@ -3466,20 +3477,22 @@
 store it in a Lisp variable.  Example:
 
    (let ((beg (point))) (forward-line 1) (delete-region beg (point)))."
 
   (if pos
       (progn
 	(setq mark-active t)
 	(run-hooks 'activate-mark-hook)
-	(and select-active-regions
-	     (x-set-selection
-	      nil (buffer-substring (region-beginning) (region-end))))
-	(set-marker (mark-marker) pos (current-buffer)))
+	(set-marker (mark-marker) pos (current-buffer))
+	(when select-active-regions
+	  (x-set-selection nil (current-buffer))))
+    (and mark-active select-active-regions
+	 (x-selection-owner-p nil)
+	 (x-set-selection nil (x-get-selection nil)))
     ;; Normally we never clear mark-active except in Transient Mark mode.
     ;; But when we actually clear out the mark value too,
     ;; we must clear mark-active in any mode.
     (setq mark-active nil)
     (run-hooks 'deactivate-mark-hook)
     (set-marker (mark-marker) nil)))
 
 (defcustom use-empty-active-region nil
Index: lisp/mouse.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/mouse.el,v
retrieving revision 1.347
diff -U 8 -r1.347 mouse.el
--- lisp/mouse.el	11 Aug 2008 01:23:05 -0000	1.347
+++ lisp/mouse.el	11 Sep 2008 02:19:50 -0000
@@ -906,16 +906,17 @@
 (defun mouse-drag-track (start-event  &optional
 				      do-mouse-drag-region-post-process)
     "Track mouse drags by highlighting area between point and cursor.
 The region will be defined with mark and point, and the overlay
 will be deleted after return.  DO-MOUSE-DRAG-REGION-POST-PROCESS
 should only be used by mouse-drag-region."
   (mouse-minibuffer-check start-event)
   (setq mouse-selection-click-count-buffer (current-buffer))
+  (deactivate-mark)
   (let* ((original-window (selected-window))
          ;; We've recorded what we needed from the current buffer and
          ;; window, now let's jump to the place of the event, where things
          ;; are happening.
          (_ (mouse-set-point start-event))
          (echo-keystrokes 0)
 	 (start-posn (event-start start-event))
 	 (start-point (posn-point start-posn))
@@ -950,17 +951,16 @@
     (if (< (point) start-point)
 	(goto-char start-point))
     (setq start-point (point))
     (if remap-double-click ;; Don't expand mouse overlay in links
 	(setq click-count 0))
     (mouse-move-drag-overlay mouse-drag-overlay start-point start-point
                              click-count)
     (overlay-put mouse-drag-overlay 'window start-window)
-    (deactivate-mark)
     (let (event end end-point last-end-point)
       (track-mouse
 	(while (progn
 		 (setq event (read-event))
                  (or (mouse-movement-p event)
                      (memq (car-safe event) '(switch-frame select-window))))
           (if (memq (car-safe event) '(switch-frame select-window))
 	      nil
@@ -1352,16 +1352,19 @@
 (defun mouse-yank-primary (click)
   "Insert the primary selection at the position clicked on.
 Move point to the end of the inserted text.
 If `mouse-yank-at-point' is non-nil, insert at point
 regardless of where you click."
   (interactive "e")
   ;; Give temporary modes such as isearch a chance to turn off.
   (run-hooks 'mouse-leave-buffer-hook)
+  ;; if region is active and _is_ primary (due to select-active-regions)
+  ;; avoid doubling upon repeated consecutive clicks.
+  (and select-active-regions (deactivate-mark))
   (or mouse-yank-at-point (mouse-set-point click))
   (let ((primary (x-get-selection 'PRIMARY)))
     (if primary
         (insert (x-get-selection 'PRIMARY))
       (error "No primary selection"))))
 
 (defun mouse-kill-ring-save (click)
   "Copy the region between point and the mouse click in the kill ring.

[-- Attachment #3: ChangeLog.select-active-regions_lazybuf_r3 --]
[-- Type: text/plain, Size: 425 bytes --]

2008-09-11  David De La Harpe Golden  <david@harpegolden.net>

	* select.el: allow x-set-selection to take a buffer object that is
	inspected on-demand to obtain selection data from its region.
	* simple.el: lazy implementation of select-active-regions.
	* mouse.el: fix time-ordering of deactivate-mark operations in
	mouse drag tracking. Avoid doubling of selection in
	mouse-yank-primary when select-active-regions is on.

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

* bug#902: select-active-regions only half-working
  2008-09-11  2:01                       ` Stefan Monnier
  2008-09-11  2:40                         ` David De La Harpe Golden
@ 2009-01-25 23:44                         ` David De La Harpe Golden
  1 sibling, 0 replies; 16+ messages in thread
From: David De La Harpe Golden @ 2009-01-25 23:44 UTC (permalink / raw)
  To: 902

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

Tiny update to apply to current cvs head without large fuzz, 
functionally unchanged.

Not sure it's handling 100% of cases - maybe it should be copying and 
"holding" as a string the selection before a command runs, and resetting 
it to the buffer object only if the mark hasn't been deactivated during 
the run of the command. Otherwise there may be some odd situations where 
the primary selection doesn't reflect the last _user visible_ highlight 
on screen, which is surprising.

OTOH, that sounds more complex, and such cases don't seem to arise much 
(or at all) in practice - at least I haven't noticed them.










[-- Attachment #2: select-active-regions_lazybuf_r4.diff.gz --]
[-- Type: application/x-gzip, Size: 3741 bytes --]

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

* bug#902: marked as done (select-active-regions only half-working)
  2008-09-06  5:53 ` bug#902: select-active-regions only half-working David De La Harpe Golden
       [not found]   ` <handler.902.B.122068042025981.ack@emacsbugs.donarmstrong.com>
  2008-09-06 19:50   ` bug#902: select-active-regions only half-working Stefan Monnier
@ 2009-07-15  1:40   ` Emacs bug Tracking System
  2 siblings, 0 replies; 16+ messages in thread
From: Emacs bug Tracking System @ 2009-07-15  1:40 UTC (permalink / raw)
  To: Chong Yidong

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


Your message dated Tue, 14 Jul 2009 21:31:04 -0400
with message-id <878wiqivfr.fsf@stupidchicken.com>
and subject line Re: select-active-regions only half-working
has caused the Emacs bug report #902,
regarding select-active-regions only half-working
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@emacsbugs.donarmstrong.com
immediately.)


-- 
902: http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=902
Emacs Bug Tracking System
Contact owner@emacsbugs.donarmstrong.com with problems

[-- Attachment #2: Type: message/rfc822, Size: 6331 bytes --]

[-- Attachment #2.1.1: Type: text/plain, Size: 1026 bytes --]

Package: emacs
Version: 23.0.60
Severity: normal
Tags: patch

The select-active-regions in-tree does not work in all cases - it works
when the region is changed by moving the mark but not when it is changed
by certain other means e.g. moving the point with the keyboard.
(Workaround:  C-x C-x C-x C-x ...).

There were a couple of approaches tried a while back (see emacs-devel
"Improved X Selections" thread).  The idle timer seemed to work best in
practice, despite (or because of) "coarse graining" i.e. a bunch of
small region changes in a row don't reupdate the selection for each
change, only kicks in when emacs has idled.

Attached patch is a simple idle-timer based implementation.

Concerns here may also apply to the activate-mark-hook (which states
in its documentation that it is rerun when region changes). Having that
work similarly with the same method would probably mean that
active region timer would have to be enabled whenever the mark is
active, not just when select-active-regions is on.
















[-- Attachment #2.1.2: select-active-regions-by-idle-timer.diff --]
[-- Type: text/x-patch, Size: 3131 bytes --]

Index: lisp/simple.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/simple.el,v
retrieving revision 1.945
diff -U 8 -r1.945 simple.el
--- lisp/simple.el	15 Aug 2008 00:30:44 -0000	1.945
+++ lisp/simple.el	6 Sep 2008 05:41:14 -0000
@@ -3439,18 +3439,46 @@
 
 (defun activate-mark ()
   "Activate the mark."
   (when (mark t)
     (setq mark-active t)
     (unless transient-mark-mode
       (setq transient-mark-mode 'lambda))))
 
+(defvar select-active-regions-timer nil)
+
+(defvar select-active-regions-last-region-hash nil)
+
+(defun select-active-regions-maybe-set-selection ()
+  "Implements `select-active-regions', called from idle timer
+`select-active-regions-timer'"
+  (and select-active-regions
+       (region-active-p)
+       (> (region-end) (region-beginning))
+       (if (or (not select-active-regions-last-region-hash)
+	       (x-selection-owner-p nil))
+	   (let ((region-hash
+		  (md5 (current-buffer) (region-beginning)
+		       (region-end) nil t)))
+	     (unless (string-equal region-hash
+				   select-active-regions-last-region-hash)
+	       (x-set-selection
+		nil (buffer-substring (region-beginning) (region-end)))
+	       (setq select-active-regions-last-region-hash
+		     region-hash)))
+	 (setq select-active-regions-last-region-hash nil))))
+
 (defcustom select-active-regions nil
-  "If non-nil, an active region automatically becomes the window selection."
+  "If non-nil, an active region automatically becomes the window selection.
+
+In conjunction with this, you might want to:
+rebind mouse-2 to `mouse-yank-primary', set `x-select-enable-primary' to nil,
+set `x-select-enable-clipboard' to non-nil, set `mouse-drag-copy-region'
+to nil, and turn on `transient-mark-mode'."
   :type 'boolean
   :group 'killing
   :version "23.1")
 
 (defun set-mark (pos)
   "Set this buffer's mark to POS.  Don't use this function!
 That is to say, don't use this function unless you want
 the user to see that the mark has moved, and you want the previous
@@ -3466,19 +3494,26 @@
 store it in a Lisp variable.  Example:
 
    (let ((beg (point))) (forward-line 1) (delete-region beg (point)))."
 
   (if pos
       (progn
 	(setq mark-active t)
 	(run-hooks 'activate-mark-hook)
-	(and select-active-regions
-	     (x-set-selection
-	      nil (buffer-substring (region-beginning) (region-end))))
+	(if select-active-regions
+	    (progn
+	      (unless select-active-regions-timer
+		(setq select-active-regions-timer
+		      (run-with-idle-timer
+		       0.1 t 'select-active-regions-maybe-set-selection)))
+	      (select-active-regions-maybe-set-selection))
+	  (when select-active-regions-timer
+	    (cancel-timer select-active-regions-timer)
+	    (setq select-active-regions-timer nil)))
 	(set-marker (mark-marker) pos (current-buffer)))
     ;; Normally we never clear mark-active except in Transient Mark mode.
     ;; But when we actually clear out the mark value too,
     ;; we must clear mark-active in any mode.
     (setq mark-active nil)
     (run-hooks 'deactivate-mark-hook)
     (set-marker (mark-marker) nil)))
 

[-- Attachment #2.1.3: ChangeLog.select-active-regions-by-idle-timer --]
[-- Type: text/plain, Size: 127 bytes --]

2008-09-06 David De La Harpe Golden <david@harpegolden.net>

	* simple.el: idle-timer implementation of select-active-regions


[-- Attachment #3: Type: message/rfc822, Size: 1595 bytes --]

From: Chong Yidong <cyd@stupidchicken.com>
To: David De La Harpe Golden <david@harpegolden.net>
Cc: 902-done@emacsbugs.donarmstrong.com
Subject: Re: select-active-regions only half-working
Date: Tue, 14 Jul 2009 21:31:04 -0400
Message-ID: <878wiqivfr.fsf@stupidchicken.com>

Hi David,

I've checked a revamped version of your patch into the trunk.  Thanks
very much for figuring out the details for making this work.

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

end of thread, other threads:[~2009-07-15  1:40 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <878wiqivfr.fsf@stupidchicken.com>
2008-09-06  5:53 ` bug#902: select-active-regions only half-working David De La Harpe Golden
     [not found]   ` <handler.902.B.122068042025981.ack@emacsbugs.donarmstrong.com>
2008-09-06 19:35     ` bug#902: Acknowledgement (select-active-regions only half-working) David De La Harpe Golden
2008-09-06 19:50   ` bug#902: select-active-regions only half-working Stefan Monnier
2008-09-06 20:22     ` David De La Harpe Golden
2008-09-07  3:53       ` Stefan Monnier
2008-09-07 20:27         ` David De La Harpe Golden
2008-09-07 21:20           ` Stefan Monnier
2008-09-09  0:42             ` David De La Harpe Golden
2008-09-09 14:50               ` Stefan Monnier
2008-09-09 19:20                 ` David De La Harpe Golden
2008-09-10 16:37                   ` Stefan Monnier
2008-09-10 21:45                     ` David De La Harpe Golden
2008-09-11  2:01                       ` Stefan Monnier
2008-09-11  2:40                         ` David De La Harpe Golden
2009-01-25 23:44                         ` David De La Harpe Golden
2009-07-15  1:40   ` bug#902: marked as done (select-active-regions only half-working) Emacs bug Tracking System

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