From: Stefan Monnier <monnier@IRO.UMontreal.CA>
To: Federico Tedin <federicotedin@gmail.com>
Cc: emacs-devel@gnu.org
Subject: Re: [Emacs-diffs] master 134ba45: Allow two mouse functions to work with Rectangle Mark mode
Date: Wed, 17 Oct 2018 11:47:30 -0400 [thread overview]
Message-ID: <jwvtvlktxc1.fsf-monnier+emacsdiffs@gnu.org> (raw)
In-Reply-To: <20181017063831.03DCB2044D@vcs0.savannah.gnu.org> (Martin Rudalics's message of "Wed, 17 Oct 2018 02:38:30 -0400 (EDT)")
> Allow two mouse functions to work with Rectangle Mark mode
>
> * lisp/mouse.el (mouse-save-then-kill): Make
> mouse-save-then-kill work with rectangular regions, including
> when mouse-drag-copy-region is set to t. (Bug#31240)
> (mouse-drag-and-drop-region): Allow dragging and dropping
> rectangular regions. (Bug#31240)
> * rect.el (rectangle-intersect-p)
> (rectangle-position-as-coordinates): New functions.
Cool, thanks.
> @@ -2455,7 +2466,11 @@ is copied instead of being cut."
> ;; Obtain the dragged text in region. When the loop was
> ;; skipped, value-selection remains nil.
> (unless value-selection
> - (setq value-selection (buffer-substring start end))
> + (setq value-selection (funcall region-extract-function nil))
> + ;; Remove yank-handler property in order to re-insert text using
> + ;; the `insert-rectangle' function later on.
> + (remove-text-properties 0 (length value-selection)
> + '(yank-handler) value-selection)
> (when mouse-drag-and-drop-region-show-tooltip
> (let ((text-size mouse-drag-and-drop-region-show-tooltip))
> (setq text-tooltip
I don't understand the comment: why is it necessary to remove the
yank-handler in order to insert the text via insert-rectangle?
> @@ -2468,12 +2483,18 @@ is copied instead of being cut."
> value-selection))))
>
> ;; Check if selected text is read-only.
> - (setq text-from-read-only (or text-from-read-only
> - (get-text-property start 'read-only)
> - (not (equal
> - (next-single-char-property-change
> - start 'read-only nil end)
> - end)))))
> + (setq text-from-read-only
> + (or text-from-read-only
> + (get-text-property start 'read-only)
> + (get-text-property end 'read-only)
> + (catch 'loop
> + (dolist (bound (region-bounds))
> + (unless (equal
> + (next-single-char-property-change
> + (car bound) 'read-only nil (cdr bound))
> + (cdr bound))
> + (throw 'loop t)))))))
> +
> (setq window-to-paste (posn-window (event-end event)))
> (setq point-to-paste (posn-point (event-end event)))
> ;; Set nil when target buffer is minibuffer.
Why do we use start/end additionally to checking each chunk?
Also why check (get-text-property end 'read-only) which looks at the
read-only property of the char right *after* the region?
I'd have expected instead something like
(setq text-from-read-only
(or text-from-read-only
(catch 'loop
(dolist (bound (region-bounds))
(unless (and (null (get-text-property (car bound) 'read-only))
(equal (next-single-char-property-change
(car bound) 'read-only nil (cdr bound))
(cdr bound)))
(throw 'loop t)))))))
Also I notice that this code (the old version, your new version, and
the version I just wrote above) have the odd property of using get-text-property
in one place and next-single-char-property-change in the other, meaning
that one part only check text-properties while the other checks both
text-properties and overlays.
IIRC `read-only` is only special in text-properties and not in overlays,
in which case we could maybe use just
(setq text-from-read-only
(or text-from-read-only
(catch 'loop
(dolist (bound (region-bounds))
(unless (text-property-not-all
(car bound) (cdr bound) 'read-only nil)
(throw 'loop t)))))))
> @@ -2581,7 +2624,9 @@ is copied instead of being cut."
> (select-window window)
> (goto-char point)
> (setq deactivate-mark nil)
> - (activate-mark))
> + (activate-mark)
> + (when region-noncontiguous
> + (rectangle-mark-mode)))
> ;; Modify buffers.
> (t
> ;; * DESTINATION BUFFER::
Hmm... while it's true that currently, I don't know of a package which
provides non contiguous regions other than rectangular regions, there
hopefully will be such a package in the future (which lets you build
arbitrary non-contiguous region, chunk by chunk, or create
a non-contiguous region holding all matches of a particular regexp).
Maybe we need to extend the API of noncontiguous regions so you can
tell which "special mode" is associated with it (if any)?
> @@ -2590,11 +2635,17 @@ is copied instead of being cut."
> (setq window-exempt window-to-paste)
> (goto-char point-to-paste)
> (push-mark)
> - (insert value-selection)
> +
> + (if region-noncontiguous
> + (insert-rectangle (split-string value-selection "\n"))
> + (insert value-selection))
Hmm... I think if you use insert-for-yank (and don't strip
the yank-handler earlier), then it will just work for both the
contiguous and the rectangular cases.
> ;; On success, set the text as region on destination buffer.
> (when (not (equal (mark) (point)))
> (setq deactivate-mark nil)
> - (activate-mark))
> + (activate-mark)
> + (when region-noncontiguous
> + (rectangle-mark-mode)))
Same as above. Maybe those 3 lines should be moved to their own
little function.
> +... The value
> +returned is a cons of the current column of POSITION and its line
> +number."
We usually write this using some kind of "pattern".
E.g. "Return a value of the form (COLUMN . LINE)".
> + (save-excursion
> + (goto-char position)
> + (let ((col (current-column))
> + (line (1- (line-number-at-pos))))
> + (cons col line))))
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.
> +(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."
Same as above, we could write it as
POS1 and POS2 specify the positions of the upper-left corners of
the first and second rectangle as conses of the form (COLUMN . LINE).
SIZE1 and SIZE2 specify the dimensions of the first and
second rectangle, as conses of the form (WIDTH . HEIGHT)."
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).
Stefan
next parent reply other threads:[~2018-10-17 15:47 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20181017063829.3775.67018@vcs0.savannah.gnu.org>
[not found] ` <20181017063831.03DCB2044D@vcs0.savannah.gnu.org>
2018-10-17 15:47 ` Stefan Monnier [this message]
2018-10-17 16:17 ` [Emacs-diffs] master 134ba45: Allow two mouse functions to work with Rectangle Mark mode Drew Adams
2018-10-19 15:24 ` Stefan Monnier
2018-10-20 3:21 ` Federico Tedin
2018-10-20 3:48 ` Stefan Monnier
2018-10-20 20:21 ` Federico Tedin
2018-10-20 20:56 ` Federico Tedin
2018-10-20 21:21 ` Stefan Monnier
2018-10-21 8:22 ` martin rudalics
2018-10-22 13:04 ` Federico Tedin
2018-10-25 23:24 ` Federico Tedin
2018-10-26 17:25 ` Stefan Monnier
2018-10-26 22:07 ` Federico Tedin
2018-10-28 0:30 ` Stefan Monnier
2018-10-29 23:40 ` Federico Tedin
2018-10-30 12:52 ` Stefan Monnier
2019-01-07 17:40 ` Stefan Monnier
2019-01-26 21:58 ` Federico Tedin
2019-01-28 9:12 ` Stefan Monnier
2019-01-29 4:02 ` Federico Tedin
2019-01-29 9:28 ` Stefan Monnier
2019-01-30 3:39 ` Federico Tedin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=jwvtvlktxc1.fsf-monnier+emacsdiffs@gnu.org \
--to=monnier@iro.umontreal.ca \
--cc=emacs-devel@gnu.org \
--cc=federicotedin@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.