From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: "Drew Adams" Newsgroups: gmane.emacs.devel Subject: RE: `isearch-allow-scroll' - a misnomer and a bad design Date: Fri, 9 Sep 2011 16:07:02 -0700 Message-ID: <7002A9DA9A804F0B9F6F251FD3A2B263@us.oracle.com> References: <20110909215255.GD2733@acm.acm> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-Trace: dough.gmane.org 1315609643 9316 80.91.229.12 (9 Sep 2011 23:07:23 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Fri, 9 Sep 2011 23:07:23 +0000 (UTC) Cc: emacs-devel@gnu.org To: "'Alan Mackenzie'" Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sat Sep 10 01:07:18 2011 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([140.186.70.17]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1R2AAH-000305-Dz for ged-emacs-devel@m.gmane.org; Sat, 10 Sep 2011 01:07:17 +0200 Original-Received: from localhost ([::1]:40575 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R2AAG-0002Eq-Tv for ged-emacs-devel@m.gmane.org; Fri, 09 Sep 2011 19:07:16 -0400 Original-Received: from eggs.gnu.org ([140.186.70.92]:56506) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R2AAD-0002EH-D6 for emacs-devel@gnu.org; Fri, 09 Sep 2011 19:07:14 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R2AAB-0002Cb-Hd for emacs-devel@gnu.org; Fri, 09 Sep 2011 19:07:13 -0400 Original-Received: from rcsinet15.oracle.com ([148.87.113.117]:64674) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R2AAB-0002CX-7x for emacs-devel@gnu.org; Fri, 09 Sep 2011 19:07:11 -0400 Original-Received: from rtcsinet21.oracle.com (rtcsinet21.oracle.com [66.248.204.29]) by rcsinet15.oracle.com (Switch-3.4.4/Switch-3.4.4) with ESMTP id p89N77Vd016976 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 9 Sep 2011 23:07:09 GMT Original-Received: from acsmt356.oracle.com (acsmt356.oracle.com [141.146.40.156]) by rtcsinet21.oracle.com (8.14.4+Sun/8.14.4) with ESMTP id p89N76sa026126 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 9 Sep 2011 23:07:07 GMT Original-Received: from abhmt102.oracle.com (abhmt102.oracle.com [141.146.116.54]) by acsmt356.oracle.com (8.12.11.20060308/8.12.11) with ESMTP id p89N70vl004765; Fri, 9 Sep 2011 18:07:00 -0500 Original-Received: from dradamslap1 (/10.159.51.14) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Fri, 09 Sep 2011 16:07:00 -0700 X-Mailer: Microsoft Office Outlook 11 In-Reply-To: <20110909215255.GD2733@acm.acm> Thread-Index: AcxvO1/sLI+byqnORp23/67hxGA/JQAAdUTw X-MimeOLE: Produced By Microsoft MimeOLE V6.00.2900.6109 X-Source-IP: rtcsinet21.oracle.com [66.248.204.29] X-CT-RefId: str=0001.0A090203.4E6A9C1D.00EE,ss=1,re=-2.300,fgs=0 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 1) X-Received-From: 148.87.113.117 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 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-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:143833 Archived-At: > Hi, Drew, Hi Alan, > I kind of feel myself being addressed here. :-) Consider it a compliment that I now see some use for this general feature - beyond scrolling (which I personally won't use it for). ;-) > > 1. The doc for this option, as well as its name, give the > > impression that it is > > about allowing scrolling. It is not. > > It is largely about scrolling. I meant that it is not _necessarily_ about scrolling. I understand that that was the original motivation, and it remains an important use case (for people who want scrolling). > > 1. For one thing, a non-nil value allows *any* command > > bound to a key in `isearch-mode-map' to take advantage > > of a prefix arg. That is, `C-u' is passed > > through to the command if the option value is non-nil. > > The command need not have anything to do with scrolling. > > I'm not sure that's quite true. Or if it is, keys like C-s > don't react to it. AFAICT, it is 100% true. For one thing, that's how `isearch-query-replace(-regexp)' works. Those commands do _not_ have property `isearch-scroll' or `scroll-command' on their symbols. They simply use the feature that `C-u' gets passed through if `isearch-allow-scroll' is non-nil. > > And there are already at least two such vanilla commands: > > `isearch-query-replace' and `isearch-query-replace-regexp' > > (`M-%', `C-M-%'). A `C-u' changes the command behavior in > > a way that has nothing to do with scrolling. > > It's good that the C-u works, though. ;-) That's my point. Like those commands, I want to make use of this `C-u' feature. But I do not want to also allow scrolling during Isearch. For example, I have an Icicles command (bound to `S-TAB') that runs from Isearch, and if you use `C-u S-TAB' then its behavior is different. But that means that to take advantage of this users need to turn on `isearch-allow-scroll'. That would be OK if that only had the effect of allowing `C-u' pass-through. But it also allows scrolling, and it's not obvious that users who want to use `C-u S-TAB' also want Isearch to allow scrolling. > > At a minimum, the doc should be adjusted. The option name > > should also be changed to fit what it really does. > > What would you suggest? What does the option really do, in your > judgement? Scrolling is an essential part of the option. AFAICT, it does two things: 1. It invokes any command that has property `isearch-scroll' or `scroll-command' on its symbol - without necessarily exiting Isearch. The function used to look up such commands is `isearch-lookup-scroll-key', but there is nothing in that function's definition that deals with scrolling or scroll commands (apart from the names of the properties checked). 2. It passes a prefix arg through to any key (its command) that follows it. Neither of these has anything to do with scrolling, necessarily. Change the names to remove `scroll' and you will see that. Now, yes, it happens that the scrolling commands use property `scroll-command'. That was one of my points: separate the property used to distinguish susceptible commands for Isearch from a property that is associated with scrolling. For scrolling commands, if you want pass-through from Isearch then you just give them property `isearch-scroll' (a bad name), in addition to property `scroll-command'. (And the code that gives the scrolling commands their Isearch behavior should not even be in isearch.el, IMO.) > > Likewise, the functions (e.g. `isearch-lookup-scroll-key') > > and other code and doc strings in isearch.el that paint this > > feature as having to do with scrolling. > > They're stricly internal functions, so changing their > names/doc strings wouldn't be a big deal. Right, but my real point is not about the names but about freeing up the two pass-through behaviors (one for specific commands, the other for `C-u') from their coupling to scrolling. Once freed, nothing would prevent you from re-associating them: telling Isearch that you want scrolling commands to benefit from the pass-through feature. > > 1b. For another thing (i.e., forgetting about `C-u'), *any* > > command can benefit from the same Isearch feature as a > > scrolling command. It suffices to put a non-nil > > `isearch-scroll' property on the command symbol. > > This isn't true. It is true, AFAICT. Nothing prevents you from putting property `isearch-scroll' on *any* command, to get Isearch to pass through to it. Now that does not mean that every command is a reasonable candidate for using this feature. It just means that the feature is not in any way scrolling-specific. > Only commands which don't foul up the isearch can be > permitted to use the feature. Only a few commands meet the criteria. > `universal-argument' is such a command. Even if you are right about that, nothing says that that particular set of commands is limited to scrolling commands. And even if that were also the case, nothing in fact prevents using an inappropriate command this way. The code looks for only one thing: property `isearch-scroll'. > > 2. I would like to see us separate the treatment of a > > prefix arg - whether it gets passed it through to a > > command or it exits Isearch - from the other > > uses/effects of this option. > > I agree, that would be a good idea. It wouldn't take very much work. Very glad to hear that. > Need it be an option? Why not just let C-u through no matter what? Dunno. That would probably be OK for me, but I can imagine RMS or others chiming in that they are used to having `C-u' immediately exit Isearch. Having an option means not disturbing the general, traditional behavior. But as far as I'm concerned, I think I would be in favor of always letting prefix-arg keys pass through without exiting Isearch. (It would be up to the command they are passed through to to decide whether to exit. In the case of the Icicles feature that uses this, the command does in fact exit Isearch, after picking up some Isearch info. But the important thing is to be able to pass a prefix arg to the command, which is bound to a key on `isearch-mode-map'.) > As a matter of interest, have you tried setting > isearch-allow-scroll? If > you have and didn't like it, what about it didn't you like? I have tried it. I don't recall just what I didn't like. I wouldn't say that I'm against it or I don't like it. (For one thing, I'm not used to it.) But I do recall trying it out a fair amount a few years back. You know, everyone is different. The fact that someone might not like to use it doesn't mean much. > > 3. Wrt controlling which commands are affected by the > > option (i.e., forgetting about `C-u' now): The current > > design makes the library designer responsible for > > this choice, not the user. That is another flaw, IMO. > > A user should be able to easily (using Customize, not just > > putting `put' here and there) choose which commands are affected. > > I disagree totally here. The sophisticated user can set a suitable > command to be a "scrolling" command. The unsophisticated > user is going to get his Emacs into a thorough mess by messing > around in this area. I suspect you are still thinking of this in terms of scroll commands. I am not. Granted that users might not want to mess around with the scrolling commands (whether via `put' or Customize), perhaps you will recognize that a user might want to not have Isearch pass through to some command that library `foo' thinks it is a great idea to pass through to by default. Currently, it is all or nothing: all scroll commands plus any others that might have property `isearch-scroll' or else none of them. Especially if we open this up to intentionally be about pass-through in general and not just scrolling, users should have a way to easily pick and choose which commands are affected by it. (I say "intentionally" because it is already open but without any advertising and partially hidden behind the names "scroll" this-and-that. > > Instead of a Boolean option (`isearch-allow-scroll'), users > > should have an option that specifies the affected commands. > > (It could also configure any specifics for each command, if > > there are such.) > > Again, familiarise yourself with just how restricted the permissible > commands are. There's a list of criteria in isearch.el > ~L1760 (think - number of yards in a mile :-). I read that comment, and I read the code. Again, I think you are thinking only in terms of scrolling commands. The feature is in fact only about pass-through to a command, letting it run without necessarily exiting Isearch. It is just like the `C-u' pass-through, but without the `C-u'. The code is not "scroll"-specific at all, AFAICT. That said, I don't want to belabor this part. My main interest is in freeing up `C-u', which you have already agreed to. My point about the non-C-u pass-through was mainly to say that it is already there, and it has nothing, a priori, to do with scrolling. That its predefined use might currently be limited to scrolling does not mean that the code actually recognizes scrolling. Likewise, if it turns out that no one will ever want to use it for any command other than scrolling. The code itself is agnostic - scrolling-blind (AFAICT). > > 4. Why are there currently _two_ different properties that > > turn on this sensitivity (i.e., "scrolling")? From the > > code comments: > > > ;; ... property called `isearch-scroll'. > > ;; If a command's symbol has the value t for this property > > ;; or for the `scroll-command' property, it is a scrolling > > ;; command. > > I haven't a clue. I've no idea who put `scroll-command' in or why. I presume that it is for scrolling commands, outside of any Isearch context. > > This means that if someone adds property `scroll-command' > > for some command then it automatically acts as if s?he > > also added property `isearch-scroll'. Why couple these > > two things? Why assume that every `scroll-command' command > > is also one for which Isearch should allow scrolling. > > Pretty much all standard commands that can be "scrolling" commands > already are. Let me know if there are any I have missed. I know. My question was why. But I don't really care. My point was that these are two different things that need not be automatically coupled. They can instead be explicitly coupled if that's a good idea. IOW, instead of having Isearch automatically recognize property `scroll-command', just use property `isearch-scroll' for all pertinent commands (even if they already have property `scroll-command'). Again, though. I don't want to insist on using pass-through for non-scrolling commands. I don't really care about that. I would like to see `C-u' passed through - that's the main point. Either systematically or optionally, but if optionally then separately from allowing scrolling. Passing `C-u' through to a command (e.g. `isearch-query-replace') really has nothing to do, logically, with allowing scrolling. Users who would like to allow `C-u' pass-through should not also have to allow scrolling. That's my main point, and I think you agree with that much.