Re: mouse-drag-and-drop-region

[martin rudalics <rudalics@gmx.at>]

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

We would probably have to change 'mouse-drag-region' for that to work.

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

What takes significant time?  Please elaborate.

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

I think that using 'mouse-set-point' as we do for 'mouse-drag-track' is
not necessarily a good idea.  The idea with 'mouse-drag-track' is that
its user usually wants point to go where the mouse moves.  The idea with
'mouse-drag-and-drop-region' is that the user wants to drop text where
the mouse goes but not necessarily also move point there.  If we want
'mouse-drag-and-drop-region' to do additional things (like killing the
region or moving point) we should make these customizable at least.

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

Do you mean that they move a file when the target directories differ and
copy it when they are the same?  Isn't that then the opposite of

 >>> Between the same volume (buffer), `drag' does `cut' instead of `copy'.

In either case I fail to see the volume/buffer connection relationship.
But maybe that's just me.

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

As mentioned, we can always add an option to kill the region instead.

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

IMO this is one of the two major things to fix before the release.
Think of someone using this function in a way you didn't anticipate (for
example, take the bug mentioned by Alex that someone hits some other key
while dragging).  If mouse tracking then throws an error and you changed
point or the region or leave the secondary overlay around, this can be
very irritating.  After all, we want people to have a smooth experience
especially when using a new function.

Obviously, if you do not move point, do not create a secondary overlay
or do not change the region, these are not needed.  Otherwise, the
function should restore the entire configuration (selected window,
point, region, overlays at least) that existed before dragging started
so that the user can restart from there.

More precisely, you should wrap 'track-mouse' and whatever you do after
it terminates in a 'condition-case' form and in the error part restore
everything to the saved values.  Like so:

(... ; save the state here
  (condition-case nil
      (progn
        (track-mouse
	 )
        )
    (error ... ; restore the state here
	  )))

If you have any problems coding that, please ask.

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

This should be a 'defcustom'.  I would write instead

(defcustom mouse-drag-and-drop-region-cut-when-buffers-differ nil
   "If non-nil, cut text also when source and destination buffers differ.
If this option is nil, `mouse-drag-and-drop-region' will leave
the text in the source buffer alone when dropping it in a
different buffer.  If this is non-nil, it will cut the text just
as it does when dropping text in the source buffer."
   :type 'boolean
   :version "27.1"
   :group 'mouse)

and likewise for 'mouse-drag-and-drop-region-show-tooltip' (note also
the use of active voice and non-nil instead of t).

 > +          (when mouse-drag-and-drop-region-show-tooltip
 > +            (tooltip-show value-selection))))

Still not quite right: Never show a tooltip on text-only frames.  Think
of people who want to show tooltips but occasionally work on TTYs.

 > +          (if buffer-read-only (message "Buffer is read-only")) ; (barf-if-buffer-read-only)

'buffer-read-only' is not sufficient.  Please handle read-only
text properties at this position.

 > +      ;; Intert the text to destination buffer under mouse.
               ^

 > +        (let ((window1 (selected-window)))
 > +          (select-window window)   ; Select window with source buffer.
 > +          (goto-char point) ; Move point to the original text on source buffer.
[...]
 >             (select-window window1))))

I think

(set-window-point window point)

instead of the above should suffice.

And if I'm not mistaken you still disallow to copy text into itself.

martin