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

[Federico Tedin]

> > I assumed (get-text-property start 'read-only) was an optimization to
> > cover cases where the region was large, and at least the beginning 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.

I see. Understood, then.

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

I agree.

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

No problem, I'll remove the call to "1-".

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

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

Something I also realized is that the variables 'region-width' and
'region-height' are 1) rectangular region-specific and 2) are only
used when checking if the region is being dragged into itself (setq
drag-but-neglibible ...). Maybe their definition could be moved to
that specific part of the code (and only define them when the region
is rectangular). Also, region-width could be calculated using 'start'
and 'end', probably using one of the functions in rect.el. Some of the
functions in rect.el are not commented so I'll have to read them
carefully to see if they can be used for this.

On the other hand, using overlays to delete the original selection
(whether it is rectangular or not) works fine, so maybe keeping them for
that purpose is a good idea.

> > 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 non contiguous 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?

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

Then, the command you propose would be helpful for point 3. Points 1
and 2 would still need to be handled in a specific way, depending on
the region's "mode". For that, we would need a way to get the current
region's "mode" (or the one that was active when
mouse-drag-and-drop-region was called).