unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: martin rudalics <rudalics@gmx.at>
To: Federico Tedin <federicotedin@gmail.com>
Cc: 31240@debbugs.gnu.org, charles@aurox.ch
Subject: bug#31240: 26.1; mouse-save-then-kill does not kill rectangles
Date: Sun, 30 Sep 2018 09:59:50 +0200	[thread overview]
Message-ID: <5BB08276.1020309@gmx.at> (raw)
In-Reply-To: <CAA8GjP=CKn4G8pLCWQ4jXU8U98fm0WzTiWGkbYS3EAZBsF8fVA@mail.gmail.com>

 > After trying out some solutions and reading these two suggestions, I've
 > decided to implement the feature like this. The problem of correctly handling
 > the cases where the dragged rectangular text would overlap with the original
 > one was more complex than I'd thought, and I have some doubts if the usefulness
 > of the feature justifies this added complexity.
 >
 > I'm attaching a new patch with all my changes to mouse.el (and rect.el) so far.
 > I've created two new helper functions in rect.el to avoid cluttering mouse.el
 > with more functions.
 >
 > So, the cases to test out are:
 >
 > 1) Dragging and dropping non-rectangular regions should be exactly the
 > same as before.
 > 2) Dragging and dropping a rectangle _outside_ of itself should insert
 > it in the new
 > position, and then delete the original.
 > 3) Dragging and dropping a rectangle _inside_ of itself should leave
 > everything unchanged.
 >
 > After evaluating "(setq mouse-drag-and-drop-region 'shift)":
 >
 > 4) Dragging and dropping a rectangle inside or outside of itself, while holding
 > the Shift key when dropping, should insert it there, without deleting
 > the original.
 >
 > When I say 'outside of itself' I mean that there shouldn't be any
 > overlapping at all
 > between the original and the newly inserted rectangles.

Thank you. From what I can tell, the patch correctly addresses the
cases enumerated.  Since I don't use rectangle functions I'd urge
someone who does use them on a more regular basis to test it.

A few issues:

I'd rewrite the doc-string of 'rectangle-position-as-coordinates' as
something like

(defun rectangle-position-as-coordinates (position)
   "Return cons of the column and line values of POSITION.
POSITION specifies a position of the current buffer.  The value
returned is a cons of the current column of POSITION and its line
number."

because doc-strings have to describe all arguments of a functions.
Also,

                   (count-lines 1 position))))

should become

                   (count-lines (point-min) position))))

And I'd rewrite the doc-string of 'rectangle-intersect-p' like

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

because the first line of a doc-string must be a complete
sentence.  Also I'd rewrite forms like

              (<= (+ x2 w2)
                  x1)

as

              (<= (+ x2 w2) x1)
	
although this still won't make your patch short enough to qualify as
"tiny change".  So if you haven't done so already, please start the
paperwork process so we can apply this patch.

Thanks again for working on this, martin





  reply	other threads:[~2018-09-30  7:59 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-22 18:35 bug#31240: 26.1; mouse-save-then-kill does not kill rectangles Charles A. Roelli
2018-08-20  2:26 ` Federico Tedin
2018-08-30 20:06   ` Charles A. Roelli
2018-09-12  0:39   ` Federico Tedin
2018-09-12 18:14     ` Charles A. Roelli
2018-09-22 20:05       ` Federico Tedin
2018-09-23 10:16         ` Charles A. Roelli
2018-09-23 22:23           ` Federico Tedin
2018-09-24 20:04             ` Charles A. Roelli
2018-09-26  0:33               ` Federico Tedin
2018-09-27 20:34                 ` Charles A. Roelli
2018-09-27 23:45                   ` Federico Tedin
2018-09-28  7:47                     ` martin rudalics
2018-09-29 23:18                       ` Federico Tedin
2018-09-30  7:59                         ` martin rudalics [this message]
2018-09-30 15:45                         ` Charles A. Roelli
2018-09-30 16:20                           ` Federico Tedin
2018-09-30 17:18                             ` Eli Zaretskii
2018-09-30 17:50                             ` martin rudalics
2018-09-30 18:25                               ` Federico Tedin
2018-10-01  8:33                                 ` martin rudalics
2018-10-01 21:34                                   ` Federico Tedin
2018-10-02  7:39                                     ` martin rudalics
2018-10-02 12:37                                       ` Federico Tedin
2018-10-02 13:17                                         ` martin rudalics
2018-10-04  2:56                                           ` Homeros Misasa
2018-10-05  6:57                                             ` martin rudalics
2018-10-05  9:28                                               ` Tak Kunihiro
2018-10-05 12:15                                                 ` Federico Tedin
2018-10-06 17:08                                                   ` martin rudalics
2018-10-06 20:16                                                     ` Federico Tedin
2018-10-07  6:17                                                       ` martin rudalics
2018-10-08 10:25                                                         ` Tak Kunihiro
2018-10-08 23:18                                                           ` Federico Tedin
2018-10-09  7:43                                                             ` martin rudalics
2018-10-10  6:19                                                               ` martin rudalics
2018-10-12  0:42                                                                 ` Federico Tedin
2018-10-12  8:44                                                                   ` martin rudalics
2018-10-12 22:08                                                                     ` Federico Tedin
2018-10-13  8:18                                                                       ` martin rudalics
2018-10-13 14:01                                                                         ` Federico Tedin
2018-10-15  7:56                                                                           ` martin rudalics
2018-10-17  7:28                                                                             ` martin rudalics
2018-10-19  0:02                                                                               ` Federico Tedin
2018-10-19  7:40                                                                                 ` martin rudalics
2018-10-19 12:53                                                                                   ` Federico Tedin
2018-10-11  2:14                                                               ` Tak Kunihiro
2018-09-29 10:07                     ` Charles A. Roelli

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

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5BB08276.1020309@gmx.at \
    --to=rudalics@gmx.at \
    --cc=31240@debbugs.gnu.org \
    --cc=charles@aurox.ch \
    --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 public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).