From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Drew Adams Newsgroups: gmane.emacs.bugs Subject: bug#25777: 25.1; [PATCH] `rectangle--pos-cols' should not move point Date: Wed, 1 Mar 2017 18:32:12 -0800 (PST) Message-ID: <2560c8d7-532e-4882-b3d4-3845312d218f@default> References: <1efbacdb-7d86-454d-b0e0-7a783c47b804@default> <87poi4e7mv.fsf@users.sourceforge.net> <877f4beok7.fsf@users.sourceforge.net> <87tw7edi9d.fsf@users.sourceforge.net> <512a3db8-49d0-4802-aecc-2d71049e0b25@default> <87zih4bhhz.fsf@users.sourceforge.net> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Trace: blaine.gmane.org 1488422001 6107 195.159.176.226 (2 Mar 2017 02:33:21 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Thu, 2 Mar 2017 02:33:21 +0000 (UTC) Cc: 25777@debbugs.gnu.org To: npostavs@users.sourceforge.net Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Thu Mar 02 03:33:15 2017 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1cjGYM-0000zz-3B for geb-bug-gnu-emacs@m.gmane.org; Thu, 02 Mar 2017 03:33:14 +0100 Original-Received: from localhost ([::1]:49802 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cjGYR-0002vv-TP for geb-bug-gnu-emacs@m.gmane.org; Wed, 01 Mar 2017 21:33:19 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:45061) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cjGYE-0002t1-0S for bug-gnu-emacs@gnu.org; Wed, 01 Mar 2017 21:33:09 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cjGYA-0001u1-QC for bug-gnu-emacs@gnu.org; Wed, 01 Mar 2017 21:33:06 -0500 Original-Received: from debbugs.gnu.org ([208.118.235.43]:36958) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1cjGYA-0001tw-NV for bug-gnu-emacs@gnu.org; Wed, 01 Mar 2017 21:33:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1cjGYA-0008AX-Be for bug-gnu-emacs@gnu.org; Wed, 01 Mar 2017 21:33:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Drew Adams Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 02 Mar 2017 02:33:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 25777 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 25777-submit@debbugs.gnu.org id=B25777.148842194431340 (code B ref 25777); Thu, 02 Mar 2017 02:33:02 +0000 Original-Received: (at 25777) by debbugs.gnu.org; 2 Mar 2017 02:32:24 +0000 Original-Received: from localhost ([127.0.0.1]:35154 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cjGXY-00089P-84 for submit@debbugs.gnu.org; Wed, 01 Mar 2017 21:32:24 -0500 Original-Received: from userp1040.oracle.com ([156.151.31.81]:36666) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cjGXW-00089C-Lg for 25777@debbugs.gnu.org; Wed, 01 Mar 2017 21:32:23 -0500 Original-Received: from userv0021.oracle.com (userv0021.oracle.com [156.151.31.71]) by userp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id v222WFYc017903 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 2 Mar 2017 02:32:15 GMT Original-Received: from userv0121.oracle.com (userv0121.oracle.com [156.151.31.72]) by userv0021.oracle.com (8.14.4/8.14.4) with ESMTP id v222WF6S029380 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 2 Mar 2017 02:32:15 GMT Original-Received: from abhmp0009.oracle.com (abhmp0009.oracle.com [141.146.116.15]) by userv0121.oracle.com (8.14.4/8.13.8) with ESMTP id v222WEXA015791; Thu, 2 Mar 2017 02:32:14 GMT In-Reply-To: <87zih4bhhz.fsf@users.sourceforge.net> X-Priority: 3 X-Mailer: Oracle Beehive Extensions for Outlook 2.0.1.9.1 (1003210) [OL 12.0.6753.5000 (x86)] X-Source-IP: userv0021.oracle.com [156.151.31.71] X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:130026 Archived-At: > > But IMHO, it is generally better to scope a `save-excursion' > > as tightly as possible around the movement that you want to > > control (hide, erase, undo). >=20 > Well, my opinion is just about the opposite, but I won't block > the patch for that. >=20 > > Why? Because it makes the code much clearer. It tells you > > that outside the `save-excursion' zones point is unlikely > > to be moved. And that makes maintenance easier and less > > error-prone. >=20 > I find wider scoping of `save-excursion' to be clearer. It tells you > that the code outside the save-excursion zone cares about the original > value of point. It cannot really tell you that code outside the zone cares about point. That's not what `s-e' does. `s-e' only acts on code inside it. It can only tell you that code inside it will not move point (because of the `s-e'). Which means that any code outside it CAN care about (rely on) point. And that applies to the code outside the more narrowly defined zones too: they CAN care about point. A maintainer can use the value of point anywhere outside those narrowly defined zones. Whether the current code actually cares about point outside a given `s-e' is less important for maintenence than knowing where code in general _could_ care about point. And if do want `s-e' placement to suggest something about some code outside it actually using point then `s-e', i.e., if you want to adopt that as a coding convention (since it is not something that `s-e' tells you, itself), then such a convention does a poor job of telling you which outside code does the using. The `s-e' would have to be placed so that the _use_ of point were immediately next to the `s-e' occurrence (_just_ outside it). Otherwise, your principle would not be followed - you would not be masking as much point movement as possible, just up to where point is actually used. Such placement is too coarse - far coarser than placement with the intention (and the actual effect, because it is what `s-e' does) of indicating where `s-e' NEEDS to protect from point movement. But I accord that people can disagree about whether `s-e' placement should be done only where it is _needed_ or as widely as possible, just barely beyond where point is used. > If there are multiple `save-excursion' zones, I have > to look at what comes after each of them, to see what > they're using point for. No, you don't. What you can be pretty sure of is that _any_ code there _could_ use point. IOW, the normal state of affairs pertains outside the `s-e': you can use point. The default in Emacs is to not use `save-excursion' gratuitously. Code uses it only when/where it matters - where it's needed. Limiting its scope to where it really matters makes it clearer just where/when that is. There is no need to wrap wider, so why do it? > Therefore maintenance becomes harder and more error-prone > with many smaller save-excursion calls. That doesn't follow. Outside those zones, including anywhere else within the function body, maintenance is free to use or change point anyway that is called for. And on the contrary: If you put lots of stuff inside `s-e', stuff that need not use point, then when you go to make a change to something there you have to figure out whether - at that position inside - you could use point if you moved the `s-e' a bit etc. Where `s-e' is should tell you, "Here we're moving point only temporarily. We undo any such movement, so that beyond this zone you can continue to use it normally." > With a single save-excursion, it's immediately clear that > the value of point is unchanged by the function. By the function! Sure. But it tells you nothing about the code within the function. You seem to be assuming that all that matters is whether the function moves point. And that abstracts totally from the possibility of making future changes to the function code. What's more: that argument you just made is the same one I'm making, but for the code where the movement can be undone. IOW, you are making the same argument as I, but you are applying it only to the whole function. The whole function _need not_ have point-movement-must-be-undone imposed on it. That stops you from making changes to the code of the function that might need, here or there, to make use of point. > And it's clear that adding more code to the function will > preserve that property, so maintenance is easier. If you wrap the entire world with a `s-e' then clearly you can add code to the world and it will still be true that the world will not move point. However, you will be unable to treat any of the code in the function itself as places where you can make _use_ of point. You will not be able to use point anywhere in the world, if you've wrapped it all in `s-e'. > It comes down to doing something > (grabbing/releasing a mutex or saving/restoring point) > only where/when it's needed That's _my_ argument. I put it only where it is _needed_, which is where point movement _needs_ to be undone. You gratuitously want to envelope also other code, where point movement does _not_ need to be undone. Doing that, you tie your hands for future changes to other parts of the same function where you might want to use point. > (which would be once at the beginning and end of the > function in this case) rather than many redundant times > (which would be each time goto-char is called). We seem to have different meanings of "needing" to cancel (undo) point movement. It seems clear to me that if only the parts that actually move point have that movement canceled the result is correct behavior. Which means that there is no need to wrap more code. Wrapping more code is gratuitous, not necessary. And those places where point is moved can be independent. There is nothing inherently "redundant" about them. > > If you confirm that you really want that wider scope here > > for `s-e' then I'll do that. Otherwise, I'll keep the > > `s-e' occurrences where they are but do the renaming and > > add a doc string. Let me know. Thx. >=20 > As I've explained, I prefer wider scope. However, the Emacs project > doesn't have a policy on this as far as I know, so we're in a similar > situation with pcase vs cond thing: the person making the patch makes > the final decision. If you found my arguments for it unconvincing, > and still feel strongly that narrower scope is better, I'll accept a > patch with the small scopes too. And I feel just as accommodating. ;-) I wanted to make my argument, and have done so, but I really don't care what is done here to fix the bug. My suggestion, since I think we've both had our say and are unlikely to convince one another on the level of principle or guideline, is for you to please make the fix. Since it is you who will kindly make the actual code change and commit, you should/can do it however you like. I have no problem with that. But do you really need another patch for that, especially since wrapping widely, a single time, is so much simpler? ;-) Could you please make the change? Your way is fine by me, for this. Thx.