Subject: Re: bug#26725: patch for mouse.el

From: Eli Zaretskii <[email-masked]>
Date: Sat, 13 May 2017 20:38:37 +0300

> Date: Mon, 01 May 2017 14:43:08 +0900 (JST)
> From: Tak Kunihiro <[email-masked]>
> Cc: [email-masked]
> 
> Drag and drop a file is already supported by Emacs.  This patch
> extends drag and drop to region (text).

Thanks.  Some comments below.

> +When the region already exists and `mouse-drag-and-drop-region'
> +is non-nil, this moves text on a region to point where mouse is
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^
"this moves the entire region of text"

> +(defcustom mouse-drag-and-drop-region nil
> +  "If non-nil, dragging mouse of the region moves text."

"If non-nil, dragging the mouse drags the region, when that exists.

> +(defun mouse-on-region-p (position &optional start end)
> +  "Return if POSITION is in between START and END in the current buffer.
> +When START and END are nil but there is active region, those of
> +active region is fed."
> +  (when (region-active-p)
> +    (setq start (or start (region-beginning)))
> +    (setq end (or end (region-end))))
> +  (let ((point (posn-point position)))
> +    (and
> +     (numberp start)
> +     (numberp end)
> +     (numberp point)
> +     (<= start point)
> +     (<= point end))))

This algorithm will fail with bidirectional text, where buffer
positions don't increase monotonically with screen coordinates.  How
about testing the face of the character under mouse instead?

> +(defun mouse-drag-region-pasting (event)

The function's name is confusing.  Why not name it like the defcustom?

> +  "Move text on a region to point where mouse is dragged over to.
                ^^
"in"

> +The transportation of text is also referred as `drag and drop'.
> +When text is dragged over to different buffer, the text is copied
                               ^
"a" is missing here.

> +instead of cut.

"instead of being cur".

>                      This works similar to
> +`mouse-drag-secondary-moving' but expects region on launch and
> +specifies point later, by mouse.

I'd lose this sentence, it doesn't add anything to the doc string.

> +To try this function, evaluate the following line.
> +	(global-set-key [down-mouse-3] \\='mouse-drag-region-pasting)
> +Then have a region and grab-and-drag it by mouse to point to move
> +to."

I think this is inappropriate for a doc string.

Please add a NEWS entry and some minimal documentation in the user
manual.