Re: [Emacs-diffs] master 134ba45: Allow two mouse functions to work with Rectangle Mark mode

[Federico Tedin]

> > I thought you were actually referring to another problem. Consider the
> > following example, where a buffer contains the text (spaces added for
> > clarity):
> >
> >  a
> >  b b
> >  c c c
> >  d d d d
> >
> > ...and the user selects the following rectangular region:
> >
> > [a      ]
> > [b b    ]
> > [c c c  ]
> > [d d d d]
> >
> > In this case, the first overlay will actually only contain "a", so
> > subtracting (current-column) in its end and start will yield 1. This
> > makes sense, as evaluating (car (region-bounds)) yields (1 . 2).
>
> Ah, right, that's yet another problem, indeed.
>
> > Something I also realised is that the variables 'region-width' and
> > 'region-height' are 1) rectangular region-specific
>
> Sorry for not mentioning it earlier.

No problem, I should've noticed it when I added it in.

> > and 2) are only used when checking if the region is being dragged into
> > itself (setq drag-but-negligible ...).
>
> I don't understand enough of the "dragged into itself" test to know what
> should be done here.  I think part of the issue is that for rectangular
> regions, the "insert-for-yank" will actually not just insert, so we
> can't just test "is insertion-point inside region-bounds?" (which could
> be easily implemented in a generic way).  But at the same time, why
> should we disallow dragging the rectangle to a place that overlaps its
> original location?

Here I'll refer to Martin's explanation, and add a bit more.

For contiguous regions (i.e. the original code),
mouse-drag-and-drop-region does the following during a drag operation
(with some details omitted):

1) Create an overlay with start and end equal to the region's
2) Get the string contents of the region and save them in 'value-selection'
3) Check if the drag is negligible: if the point where the user
   dragged the region lies *inside* the overlay, then the operation is
   marked as negligible.

Then, if the drag is not negligible:

4) 'value-selection' is inserted where the user dragged the region
5) The original selection is deleted using delete-region, using the
   overlay's start and end as parameters

If the check for 'drag-but-negligible' wasn't there, then the
following could happen: the user could drag the region into itself,
then the contents would be inserted, and then both the original and
the newly inserted contents would be deleted with the call to
delete-region (because the text would have been inserted *inside* the
overlay).

With rectangular regions, I chose to use a list of overlays to represent
the selected region. In my mind this made sense as I could keep the code
that dealt with overlays when checking for read-only properties or when
calling delete-region (with the overlay's start and end). The problem with
this is that, in some cases, a rectangle can be dragged in a way where *some*
parts of it are re-inserted into the overlays tracking the original selection.
For example:

Select a square:

 a a a
[b b]b
[c c]c

Insert it at point marked with "|":

 a|a a
[b b]b
[c c]c

 a b b a a
[b c c b]b
[c c]c

Call delete-region using the two overlays' boundaries:

a b b a a
b
c

...which probably isn't what the user was expecting. By adding
'rectangle-intersect-p', and checking for a possible intersection
between the original rectangular region and the potentially newly
inserted rectangle, these cases could be avoided.

The check for drag-but-negligible is easier to understand for contiguous
regions: if dragging a region into itself was somehow possible, the text
would remain unchanged, so the operation wouldn't make sense even from
the user's perspective. With rectangles, however, it is harder to define
which cases 'make sense'. For example, what should happen if a rectangle
is dragged one column to the right, into itself?

I think mouse-drag-and-drop-region could be greatly improved if we
somehow implemented a way of checking the region's "type" (as we were
talking) and if the results of dragging a rectangle were well
defined and specified, from a user's perspective.

> > Maybe their definition could be moved to
> > that specific part of the code (and only define them when the region
> > is rectangular).
>
> If that can be done, that sounds good.
>
> > I believe the only parts of the code that is rectangle-specific are:
> >
> > 1) Checking if the drag is negligible
> > 2) Inserting the text in the new position
> > 3) Re-activating the region after a successful or failed drag
>
> insert-for-yank takes care of (2), doesn't it?

Yes, you're right. insert-for-yank is already "region type"-aware
(because it uses the yank-handler property).