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

[Stefan Monnier]

> I assumed (get-text-property start 'read-only) was an optimization to
> cover cases where the region was large, and at least the beggining of
> it was read only. I added (get-text-property end 'read-only) to
> complement this check, but on the ending.

I see.  No, the (get-text-property start 'read-only) was there because
the next test only verified if there's a *change* somewhere, so in order
to make sure it's nil everywhere you needed both: make sure it's nil
somewhere (in this case, at the beginning) and then make sure there's no
change anywhere.

>> Same as above.  Maybe those 3 lines should be moved to their own
>> little function.
>
> I'm not sure of what the function could do. Would it call
> activate-mark and then enable a special region mode? I imagine it
> would need to receive a parameter indicating what mode to choose.

Not sure what it should do, indeed, but AFAIK it does the same at both
places.  My point was only to combine those two places to avoid code
duplication: what the content should be is orthogonal.

>> Why "1-"?  AFAIK you only compare those line numbers between themselves,
>> so this "1-" doesn't make any difference to your code and simply causes
>> this code to return "non-standard" line numbers.
>
> I wanted the "coordinates" to start at (0, 0) as I thought it would be
> easier to reason about when comparing them to other coordinates.

For better or worse, we use (0, 1) as "origin" and I think it's better
to stick to this odd convention than to muddle the water even further.

>> BTW, your "width" is computed AFAICT as
>>
>>     (- (overlay-end (car mouse-drag-and-drop-overlays))
>>        (overlay-start (car mouse-drag-and-drop-overlays)))
>>
>> which is a char-distance and not a column-distance (big difference in
>> the presence of TABs and double-width chars).
>
> You are right, I overlooked this. Using an overlay list then doesn't
> seem like a good option for tracking the contents of rectangular
> regions.

Not sure why you think so.
You can probably just use.

    (save-excursion
      (let ((ol (car mouse-drag-and-drop-overlays)))
        (goto-char (overlay-end ol))
        (- (current-column)
           (progn (goto-char (overlay-start ol)) (current-column)))))

> Maybe I could change the code to make it look more like the
> original, and use rectangle--* functions with 'start' and 'end' where
> its needed. To do this, however, I would need to know the preferred
> way of checking the region's geometry. Right now I'm using
> 'region-noncontiguous-p' and assuming that t means rectangular, which
> could break things in the future.

Maybe we should try and figure out what is special about rectangles, and
what should the code do for non-rectangular non-contiguous regions.
That might help us decide how best to get the needed information.

> The code you mentioned is needed to ensure that when the user selects
> a rectangular region and drags it, the resulting content is selected
> with rectangular-mark-mode enabled.  The same goes for when the drag
> operation fails. In both cases, the user expects that after dragging
> (or trying to drag) a rectangle, the resulting selected region will
> also be rectangular.

Hmm... how 'bout attacking the problem as follows.  There are 2 cases:
1- the drag failed, so presumably nothing was changed.  In that case,
   naive thinks, there's nothing to do, the region is still active (and
   rectangle-mark-mode is still active if applicable).  If not why not?
   In any case, if the region is not active any more (and
   rectangle-mark-mode is hence also deactivated), there should be a way
   to undo a region deactivation, i.e. "reactivate the region".
2- the drag succeeded, so the text was just inserted somewhere with
   insert-for-yank.  Here as well, we just need some way to "activate"
   that which we just inserted.

So I think what we need is a generic `reactivate-region` command and
some way for it to work correctly for noncontiguous regions (and the
insert-for-yank code should be able to set it up so that reactivating
the region after an insert-for-yank would activate it in the proper
mode as well).

IIUC, this should solve your problems and be generally useful, right?


        Stefan