From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Federico Tedin Newsgroups: gmane.emacs.devel Subject: Re: [Emacs-diffs] master 134ba45: Allow two mouse functions to work with Rectangle Mark mode Date: Sat, 20 Oct 2018 00:21:30 -0300 Message-ID: References: <20181017063829.3775.67018@vcs0.savannah.gnu.org> <20181017063831.03DCB2044D@vcs0.savannah.gnu.org> <810f1e04-1117-476d-9a7d-d57002609bf8@default> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" X-Trace: blaine.gmane.org 1540005594 6666 195.159.176.226 (20 Oct 2018 03:19:54 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sat, 20 Oct 2018 03:19:54 +0000 (UTC) Cc: emacs-devel@gnu.org To: monnier@iro.umontreal.ca Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sat Oct 20 05:19:50 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 1gDhnp-0001dV-Lx for ged-emacs-devel@m.gmane.org; Sat, 20 Oct 2018 05:19:49 +0200 Original-Received: from localhost ([::1]:53573 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gDhpw-0007sS-1c for ged-emacs-devel@m.gmane.org; Fri, 19 Oct 2018 23:22:00 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:56563) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gDhpm-0007sJ-Js for emacs-devel@gnu.org; Fri, 19 Oct 2018 23:21:51 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gDhpk-0001sh-J6 for emacs-devel@gnu.org; Fri, 19 Oct 2018 23:21:50 -0400 Original-Received: from mail-lj1-x22a.google.com ([2a00:1450:4864:20::22a]:44628) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1gDhph-0001qB-UV for emacs-devel@gnu.org; Fri, 19 Oct 2018 23:21:46 -0400 Original-Received: by mail-lj1-x22a.google.com with SMTP id v6-v6so32409812ljc.11 for ; Fri, 19 Oct 2018 20:21:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=nXtCMj4vQYkT+5cHSUD0tF5IywQlKeqVPQTmvUhlAVs=; b=mFB7lBpdOBV+vrg7w0TiIjfwb0RjGijgemF20ZoA/7qf+ommjC3HcgLp9doxjxd5io 12mDws9RX917afkzSlyM1PMLU5lNnD0VmYRbOgTWcTv0IF0IyJ+txGmL8ZerkPrtK1sG TVKSALP/oNRt2VkiQ3//fQz6VKsAc0dr022WHV+SursfhHOBU0aPE/xJyotf7LsY6tSP 030jJLyWE7aXolhK1YzrcQ/zkzx92BY8p0wPtyXRzgV/6rplqTiNl8rTshidVshXovDT 9ikVHzmN6i9klGIBSD1fS4ZGu4KNuWJnkZe98b1f821ncld4dj5GDySERx67tv0GQEDH LhQw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=nXtCMj4vQYkT+5cHSUD0tF5IywQlKeqVPQTmvUhlAVs=; b=PdgsUxcWCygxrM+m/OGRQdjxGT3E0F6TzjDeUfAm6Axo2+HdyxSIrlcmKzFhGLRK5E EzXlcqp8s+Py1LowqhqvzfuODiuSy1m2RiiwplUizh0a6BxLmH9/Mt4nbi9258GkinUF VmC2D1a4MFjvVz4jZ1p7FvMI6grE67Jo1pRKileVEwAQYRtayA2dgCSAIh2m37R8y/U0 6nIsF1QzD2dq9sW3v85zCH/OtNZVmtEWKgsWqZMmu7wfhOrfms7dA/ZI4sZlBS4OHflk XMCgsH86tDv5m/bsTm7jGwnwR0Sd4MNEWYvmodGU65i6ZL/sio8j7rwcdy911NP0OGzB aMUA== X-Gm-Message-State: AGRZ1gJJvZOScRJ5nSP5bLMX9hPEFuGpRRSmJUjgMYeiTQ0/0UWx1/WN XcaB6+G8zJqeU0PJC53qRfiPcq6Ju2N2DayNfi2DIJI+kDc= X-Google-Smtp-Source: ACcGV63NDH0HPxyV5ohpfjAZWd3fdh6uhsFxWZjsOzbP1kqvdoAWYI8czYl5ATNlp4GS2f9ssSqz+9LcFxAd89ayHGo= X-Received: by 2002:a2e:8457:: with SMTP id u23-v6mr2347042ljh.154.1540005703736; Fri, 19 Oct 2018 20:21:43 -0700 (PDT) In-Reply-To: X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2a00:1450:4864:20::22a 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:230515 Archived-At: Stefan, I'm addressing your feedback below: > I don't understand the comment: why is it necessary to remove the > yank-handler in order to insert the text via insert-rectangle? When I wrote the patch, I thought the only way of inserting a rectangle was through insert-rectangle, which receives a list of strings without the yank-handler property. I now see that when killing and yanking a rectangular region, insert-for-yank calls rectangle--insert-for-yank, which then calls insert-rectangle with the string list. > 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))))))) 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 now though that my addition is incorrect (as you mentioned) because it checks the char not contained in the region. I'm interested in testing you last example for setting text-from-read-only, as it is more readable than the others (and runs the same checks). > 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)? I agree with your last point. If the region's "special mode" could be determined somehow, then different actions could be taken depending on its value. In my patch, I treated noncontiguous regions as always rectangular. > 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. Just tested this and yes, it works for both cases. As you mentioned, I should keep the yank-handler and just use insert-for-yank. > 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. > We usually write this using some kind of "pattern". > E.g. "Return a value of the form (COLUMN . LINE)". I'll apply the change you suggested here. > 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. As you said, the "1-" could be removed and the rest of the code would work the same. > 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)." I'll also apply the change you suggested here. > 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. 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. Answering to your other message: > The code I quoted said: > > (when region-noncontiguous > (rectangle-mark-mode))) > > I don't know why he needs to enable rectangle-mark-mode here, but if > it's needed in the case of rectangular regions (apparently his code > assumes that "noncontiguous" is synonym with "rectangular") maybe some > other mode might be needed for other noncontinuous regions, in which > case, the region should maybe carry with itself its associated mode. > > But to know what to do, we first need to know more about why he has code > like the above. 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. If there was a way of getting the region's mode, then the problems in mouse-drag-and-drop-region would be easier to fix, and would also allow implementing drag operations for other region modes. I'm not sure of how to continue from here. Should I create a new patch fixing the problems you mentioned, or should I wait until there's a better way of determining the region's mode? Thanks for your feedback.