Re: mouse-drag-and-drop-region

[Tak Kunihiro]

I revised mouse-drag-and-drop-region.  I'm sending replay to
https://lists.gnu.org/archive/html/emacs-devel/2017-11/msg00387.html,
ChangeLog, and patch.

* ChangeLog

2017-11-21 Tak Kunihiro <address@hidden>

        Improve comments and have two new options

        * lisp/mouse.el (mouse-drag-and-drop-region): Make tooltip preview to 
be an option.  Have an option to make dragging cut among buffers.
        (mouse-drag-and-drop-region-cut-among-buffer): New flag.  When t, text 
is cut instead of copy when dragged among buffers.
        (mouse-drag-and-drop-region-show-tooltip): New flag.  When t, text is 
shown by a tooltip in a graphic display.

* Replay

> Did you ever consider using the "r" interactive switch instead of the
> "e" one?

I suppose you suggest something like below.  I cannot make this work.
Thus this revision, I do not use "r" interactive switch.

(defun mouse-drag-and-drop-region (event start and)
  (interactive "e\nr")
  ...)

>>> (2) Activating the secondary overlay can be distracting and should
>>>     be made optional (facultatively keeping the primary overlay in
>>>     place provided (4) below is done).
>>
>> Region on the other window is not visible.  Thus original region
>> should be hi-lighted somehow.  Since mouse-drag-and-drop-region is
>> located on mouse.el, mouse-secondary-overlay was used.
>
> You mean that you highlight the original region with the secondary
> overlay so that the overlay shows up on the target window as well and
> the user is informed that dropping there might lead to say "dubious"
> results?

I think yes.  Also, since value-selection would be inserted before the
point and mark, I thought it is handy to keep track where was the
region by an overlay.

> I mean something like
>
> (defcustom mouse-drag-and-drop-region-show-secondary-overlay t ...)
>
> where setting this to nil means to just not show any overlay.  And
> your code would have to accept that there is no secondary overlay at
> the time of the drop.

I found this takes significant time.  I skip to get rid of usage of
secondary-overlay on this revision.

>>> (3) Showing tooltips can be distracting and should be optional.
>>>     Note also, that usurping tooltips this way may prevent them from
>>>     showing interesting properties of the drop area like whether the
>>>     text there is read only.  OTOH we might consider retaining
>>>     properties of the text in (non-GTK) tooltips.
>>
> I would say
>
> (defcustom mouse-drag-and-drop-region-show-tooltip t)
>
> and never show a tooltip on text-only terminals even if this is t.

OK.

>>> (4) The (deactivate-mark) and (mouse-set-point event) trick to allow
>>>     showing point the way ‘mouse-drag-track’ does can be distracting
>>>     and should be made optional.
>>
>> I think that this is very related to 2, 3, and 5 (as you inferred).
>
> Yes.  It's irritating to drag a block cursor with the mouse.  Do you
> know of any other applications which do such a thing?

I cannot think of a way to show insert point besides (mouse-set-point
event).  Do you suggest overlay-put "|" or change cursor-type to 'bar
during drag?  I thought who prefers bar changes the default cursor
anyway.  I did not revise code in this respect.

>>> (5) The mouse pointer shape should take care of indicating the exact
>>>     position where the drop occurs.  We should probably also signal
>>>     whenever the current drop position is invalid.  This is IIUC
>>>     usual practice on most window systems now.
>>
>> I think it is a good idea.
>
> We probably need to define additional cursors for this so it's not for
> Emacs 26.

I did not revise code in this aspect.

>>> (7) IMO either cutting should be the default too when the drop
>>>     occurs in a different buffer or copying would be the default and
>>>     pressing the modifier should produce a cut instead.  The current
>>>     behavior wants me to always keep in mind whether the target
>>>     buffer is the same as the source buffer.  At least, this
>>>     behavior should be made optional.
>>
>> Default behavior followed that of file browser on `drag' a file.
>
> Which file browser?

Finder on macOS and File Explorer on Windows 10 behave like that way.

>> Between the same volume (buffer), `drag' does `cut' instead of `copy'.
>> I prefer current behavior as default but have
>> no objection to have option something like
>>   (defvar mouse-drag-and-drop-region-cut-hungry t)
>
> I would be more radical with
>
> (defcustom mouse-drag-and-drop-region-cut-or-copy t)
>
> where the values 'cut and 'copy would always categorically cut or copy
> (unless the value of 'mouse-drag-and-drop-region' implies to copy
> instead of cut) and t means your original behavior.

A new option was created.  Now `cut' can be default even among
buffers.

(defvar mouse-drag-and-drop-region-cut-among-buffer nil
  "When t, text is cut instead of copy when dragged among buffers.")

>>> (8) Read-only text should be handled.  An attempt to drop text into
>>>     a read-only area should be signalled to the user.  An attempt to
>>>     cut text from a read-only text/buffer should be signalled as
>>>     well.
>>
>>> For the latter, we copy text instead (cf C-w in read-only text).  So
>>> I think this feature should behave similarly, at least by default.
>>
>>> I just noticed that the code does use ‘kill-region’ already.  This
>>> has the side-effect of putting the text into the kill ring,
>>> something which should be documented at least.  But I think the code
>>> should rather use ‘delete-region’ instead with the behavior you
>>> proposed.
>>
>> I suppose that you suggest to tell user more explicitly in echo area
>> how he cannot change the text.
>
> The echo area should be avoided during tracking.  Think of users who
> invoke 'mouse-drag-and-drop-region' on a frame without a minibuffer.

OK.

> I think that first of all you should use 'delete-region' instead of
> 'kill-region' to avoid the above-mentioned side effect.  Then your
> code should silently swallow any bugs for cutting from read-only
> text provided 'kill-read-only-ok' is t.  For that purpose, simply
> peruse the corresponding code from 'kill-region'.

OK.  Now `delete-region' is used.

> A final drop into read-only text should always give an error.  We
> should be able to redeem that by providing a feedback in form of a
> mouse cursor when the mouse is either over read-only text or no
> buffer text at all.

I revised only to give message.  I could not tell how to bring
condition-case to code.

>> On an user perspective, operation `drag-and-drop-region' is a
>> combination of `cut' and `paste'.  Thus text was killed instead of
>> deleted.  How you think delete is more preferred?  I have no
>> objection to document it.

> IMO using 'kill-region' is an unexpected side effect.  If people
> think differently, we could add yet another option like
>
> (defucstom mouse-drag-and-drop-region-cut-does-kill-region t)

Now `delete-region' is used instead of `kill-region'.

* Patch

diff --git a/lisp/mouse.el b/lisp/mouse.el
old mode 100644
new mode 100755
index 17d1732..08f70d4
--- a/lisp/mouse.el
+++ b/lisp/mouse.el
@@ -2361,6 +2361,12 @@ text is copied instead of being cut."
   :version "26.1"
   :group 'mouse)
 
+(defvar mouse-drag-and-drop-region-cut-among-buffer nil
+  "When t, text is cut instead of copy when dragged among buffers.")
+
+(defvar mouse-drag-and-drop-region-show-tooltip t
+  "When t, text is shown by a tooltip in a graphic display.")
+
 (defun mouse-drag-and-drop-region (event)
   "Move text in the region to point where mouse is dragged to.
 The transportation of text is also referred as `drag and drop'.
@@ -2369,64 +2375,98 @@ modifier key was pressed when dropping, and the value 
of the
 variable `mouse-drag-and-drop-region' is that modifier, the text
 is copied instead of being cut."
   (interactive "e")
-  (require 'tooltip)
-  (let ((start (region-beginning))
+  (let ((mouse-drag-and-drop-region-show-tooltip
+         (and mouse-drag-and-drop-region-show-tooltip
+              (display-multi-frame-p)
+              (require 'tooltip)))
+        (start (region-beginning))
         (end (region-end))
         (point (point))
         (buffer (current-buffer))
         (window (selected-window))
-        value-selection)
+        no-modifier-on-drop
+        value-selection)    ; This remains nil when event was "click".
     (track-mouse
-      ;; When event was click instead of drag, skip loop
+      ;; When event was "click" instead of "drag", skip loop.
       (while (progn
                (setq event (read-event))
                (or (mouse-movement-p event)
                    ;; Handle `mouse-autoselect-window'.
                    (eq (car-safe event) 'select-window)))
-        (unless value-selection ; initialization
+        ;; Obtain the text in region.  When the loop was skipped,
+        ;; value-selection remains nil.
+        (unless value-selection
           (delete-overlay mouse-secondary-overlay)
           (setq value-selection (buffer-substring start end))
           (move-overlay mouse-secondary-overlay start end)) ; (deactivate-mark)
-        (ignore-errors (deactivate-mark) ; care existing region in other window
-                       (mouse-set-point event)
-                       (tooltip-show value-selection)))
-      (tooltip-hide))
-    ;; Do not modify buffer under mouse when "event was click",
-    ;;                                       "drag negligible", or
-    ;;                                       "drag to read-only".
-    (if (or (equal (mouse-posn-property (event-end event) 'face) 'region) ; 
"event was click"
-            (member 'secondary-selection ; "drag negligible"
-                    (mapcar (lambda (xxx) (overlay-get xxx 'face))
-                            (overlays-at (posn-point (event-end event)))))
-            buffer-read-only)
-        ;; Do not modify buffer under mouse.
+        (ignore-errors
+          (deactivate-mark)         ; Maintain region in other window.
+          (mouse-set-point event)
+          (when mouse-drag-and-drop-region-show-tooltip
+            (tooltip-show value-selection))))
+      ;; Check if modifier was pressed on drop.
+      (setq no-modifier-on-drop
+            (not (member mouse-drag-and-drop-region (event-modifiers event))))
+      (when mouse-drag-and-drop-region-show-tooltip (tooltip-hide)))
+    ;; Do not modify buffer under mouse when event is "click", "drag
+    ;; but negligible", or "drag to read-only".  Operation "drag but
+    ;; negligible" is defined as drag-and-drop the text to
+    ;; secondary-selection without modifier pressed.  When pressed,
+    ;; the text will be inserted to inside of secondary-selection.
+    (if (or (equal (mouse-posn-property (event-end event) 'face) 'region) ; 
"click"
+            (and (member 'secondary-selection ; "drag but negligible"
+                         (mapcar (lambda (xxx) (overlay-get xxx 'face))
+                                 (overlays-at (posn-point (event-end event)))))
+                 no-modifier-on-drop)
+            buffer-read-only)           ; "drag to read-only"
         (cond
-         ;; "drag negligible" or "drag to read-only", restore region.
+         ;; Set back the original text as region on "drag but negligible"
+         ;; or "drag to read-only".
          (value-selection
-          (select-window window) ; In case miss drag to other window
+          (if buffer-read-only (message "Buffer is read-only")) ; 
(barf-if-buffer-read-only)
+          (select-window window) ; Select the source windows back on miss-drag 
to other window.
           (goto-char point)
           (setq deactivate-mark nil)
           (activate-mark))
-         ;; "event was click"
+         ;; Move point within region on "click".
          (t
           (deactivate-mark)
           (mouse-set-point event)))
-      ;; Modify buffer under mouse by inserting text.
+      ;; Intert the text to destination buffer under mouse.
       (push-mark)
       (insert value-selection)
-      (when (not (equal (mark) (point))) ; on success insert
+      ;; On success, set the text as region on destination buffer.
+      (when (not (equal (mark) (point)))
         (setq deactivate-mark nil)
-        (activate-mark)) ; have region on destination
-      ;; Take care of initial region on source.
-      (if (equal (current-buffer) buffer) ; when same buffer
-          (let (deactivate-mark) ; remove text
-            (unless (member mouse-drag-and-drop-region (event-modifiers event))
-              (kill-region (overlay-start mouse-secondary-overlay)
-                           (overlay-end mouse-secondary-overlay))))
-        (let ((window1 (selected-window))) ; when beyond buffer
-          (select-window window)
-          (goto-char point) ; restore point on source window
-          (activate-mark) ; restore region
+        (activate-mark))
+      ;; Set back the original text as region or delete the original
+      ;; text on source buffer.
+      (if (equal (current-buffer) buffer)
+          ;; When source buffer and destination buffer are the same,
+          ;; remove the original text.
+          (let (deactivate-mark)
+            (if no-modifier-on-drop
+                (delete-region (overlay-start mouse-secondary-overlay)
+                               (overlay-end mouse-secondary-overlay))))
+        ;; When source buffer and destination buffer are different,
+        ;; keep (set back the original text as region) or remove the
+        ;; original text.
+        (let ((window1 (selected-window)))
+          (select-window window)   ; Select window with source buffer.
+          (goto-char point) ; Move point to the original text on source buffer.
+          (if (or (if no-modifier-on-drop
+                      (not mouse-drag-and-drop-region-cut-among-buffer)
+                    mouse-drag-and-drop-region-cut-among-buffer)
+                  buffer-read-only)
+              ;; Set back the original text as region on source buffer
+              ;; like operation `copy'.
+              (progn
+                (if buffer-read-only (message "Source is read-only")) ; 
(barf-if-buffer-read-only)
+                (activate-mark))
+            ;; Remove the original text from source buffer like
+            ;; operation `cut'.
+            (delete-region (overlay-start mouse-secondary-overlay)
+                           (overlay-end mouse-secondary-overlay)))
           (select-window window1))))
     (delete-overlay mouse-secondary-overlay)))