Subject: Re: bug#31240: 26.1; mouse-save-then-kill does not kill rectangles

From: martin rudalics <[email-masked]>
Date: Sun, 30 Sep 2018 09:59:50 +0200

> After trying out some solutions and reading these two suggestions, I've
 > decided to implement the feature like this. The problem of correctly handling
 > the cases where the dragged rectangular text would overlap with the original
 > one was more complex than I'd thought, and I have some doubts if the usefulness
 > of the feature justifies this added complexity.
 >
 > I'm attaching a new patch with all my changes to mouse.el (and rect.el) so far.
 > I've created two new helper functions in rect.el to avoid cluttering mouse.el
 > with more functions.
 >
 > So, the cases to test out are:
 >
 > 1) Dragging and dropping non-rectangular regions should be exactly the
 > same as before.
 > 2) Dragging and dropping a rectangle _outside_ of itself should insert
 > it in the new
 > position, and then delete the original.
 > 3) Dragging and dropping a rectangle _inside_ of itself should leave
 > everything unchanged.
 >
 > After evaluating "(setq mouse-drag-and-drop-region 'shift)":
 >
 > 4) Dragging and dropping a rectangle inside or outside of itself, while holding
 > the Shift key when dropping, should insert it there, without deleting
 > the original.
 >
 > When I say 'outside of itself' I mean that there shouldn't be any
 > overlapping at all
 > between the original and the newly inserted rectangles.

Thank you. From what I can tell, the patch correctly addresses the
cases enumerated.  Since I don't use rectangle functions I'd urge
someone who does use them on a more regular basis to test it.

A few issues:

I'd rewrite the doc-string of 'rectangle-position-as-coordinates' as
something like

(defun rectangle-position-as-coordinates (position)
   "Return cons of the column and line values of POSITION.
POSITION specifies a position of the current buffer.  The value
returned is a cons of the current column of POSITION and its line
number."

because doc-strings have to describe all arguments of a functions.
Also,

                   (count-lines 1 position))))

should become

                   (count-lines (point-min) position))))

And I'd rewrite the doc-string of 'rectangle-intersect-p' like

(defun rectangle-intersect-p (pos1 size1 pos2 size2)
   "Return non-nil if two rectangles intersect.
POS1 and POS2 specify the positions of the upper-left corners of
the first and second rectangle as conses of their column and line
values.  SIZE1 and SIZE2 specify the dimensions of the first and
second rectangle, as conses of their width and height measured in
columns and lines."

because the first line of a doc-string must be a complete
sentence.  Also I'd rewrite forms like

              (<= (+ x2 w2)
                  x1)

as

              (<= (+ x2 w2) x1)
	
although this still won't make your patch short enough to qualify as
"tiny change".  So if you haven't done so already, please start the
paperwork process so we can apply this patch.

Thanks again for working on this, martin