From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel 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 Message-ID: References: <20181017063829.3775.67018@vcs0.savannah.gnu.org> <20181017063831.03DCB2044D@vcs0.savannah.gnu.org> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: blaine.gmane.org 1539791177 18938 195.159.176.226 (17 Oct 2018 15:46:17 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Wed, 17 Oct 2018 15:46:17 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) Cc: emacs-devel@gnu.org To: Federico Tedin Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Wed Oct 17 17:46:13 2018 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1gCo1U-0004q6-J9 for ged-emacs-devel@m.gmane.org; Wed, 17 Oct 2018 17:46:12 +0200 Original-Received: from localhost ([::1]:37767 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gCo3b-0007cC-7n for ged-emacs-devel@m.gmane.org; Wed, 17 Oct 2018 11:48:23 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:53008) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gCo2t-0007US-Io for emacs-devel@gnu.org; Wed, 17 Oct 2018 11:47:41 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gCo2o-0005mb-MR for emacs-devel@gnu.org; Wed, 17 Oct 2018 11:47:39 -0400 Original-Received: from chene.dit.umontreal.ca ([132.204.246.20]:59023) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gCo2o-0005c0-DJ for emacs-devel@gnu.org; Wed, 17 Oct 2018 11:47:34 -0400 Original-Received: from fmsmemgm.homelinux.net (lechon.iro.umontreal.ca [132.204.27.242]) by chene.dit.umontreal.ca (8.14.7/8.14.1) with ESMTP id w9HFlUvC015925; Wed, 17 Oct 2018 11:47:30 -0400 Original-Received: by fmsmemgm.homelinux.net (Postfix, from userid 20848) id 8E646AE145; Wed, 17 Oct 2018 11:47:30 -0400 (EDT) In-Reply-To: <20181017063831.03DCB2044D@vcs0.savannah.gnu.org> (Martin Rudalics's message of "Wed, 17 Oct 2018 02:38:30 -0400 (EDT)") X-NAI-Spam-Flag: NO X-NAI-Spam-Threshold: 5 X-NAI-Spam-Score: 0 X-NAI-Spam-Rules: 2 Rules triggered EDT_SA_DN_PASS=0, RV6397=0 X-NAI-Spam-Version: 2.3.0.9418 : core <6397> : inlines <6935> : streams <1801582> : uri <2732446> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 132.204.246.20 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:230451 Archived-At: > 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