Re: mouse-drag-and-drop-region

[Tak Kunihiro]

Thank you for the replay.

>>> 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 meant I need to spend significant time to make secondary-overlay
option.  I have started to fix things from easy ones.


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

How to show insert point?
Do you mean restore active cursor back to source text?


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

When an Icon of c:/runemacs.exe is dragged to d:/, copy.  When an Icon
of c:/runemacs.exe is dragged to c:/emacs-26.0.90/bin, cut.


>> 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.
...
> (... ; save the state here
>  (condition-case nil
>      (progn
>        (track-mouse
>        )
>        )
>    (error ... ; restore the state here
>         )))
>
> If you have any problems coding that, please ask.

OK.  I will (but not yet).


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

OK.


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

I think the patch covered that.  The approach is to overwrite
mouse-drag-and-drop-region-show-tooltip locally.

 (let ((mouse-drag-and-drop-region-show-tooltip
         (and mouse-drag-and-drop-region-show-tooltip
              (display-multi-frame-p)
              (require 'tooltip)))


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

OK.


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

To select window is necessary for the case.  I try to use it when
fits.


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

I forgot to mention, that was fixed.


I think that there are two significant TODOs.
- Make drag robust using condition-case
- Make secondary-overlay option