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: propose adding Icicles to Emacs Date: Mon, 11 Jun 2007 16:21:21 -0700 Message-ID: References: NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-Trace: sea.gmane.org 1181604154 22863 80.91.229.12 (11 Jun 2007 23:22:34 GMT) X-Complaints-To: usenet@sea.gmane.org NNTP-Posting-Date: Mon, 11 Jun 2007 23:22:34 +0000 (UTC) Cc: rms@gnu.org, emacs-devel@gnu.org To: "Stefan Monnier" Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Tue Jun 12 01:22:31 2007 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.50) id 1HxtDN-0000pz-0b for ged-emacs-devel@m.gmane.org; Tue, 12 Jun 2007 01:22:25 +0200 Original-Received: from localhost ([127.0.0.1] helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1HxtDM-0006o8-FV for ged-emacs-devel@m.gmane.org; Mon, 11 Jun 2007 19:22:24 -0400 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1HxtDH-0006i6-PL for emacs-devel@gnu.org; Mon, 11 Jun 2007 19:22:19 -0400 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1HxtDF-0006bm-H0 for emacs-devel@gnu.org; Mon, 11 Jun 2007 19:22:18 -0400 Original-Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1HxtDF-0006bM-7I for emacs-devel@gnu.org; Mon, 11 Jun 2007 19:22:17 -0400 Original-Received: from rgminet01.oracle.com ([148.87.113.118]) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1HxtDD-0007Rj-IZ; Mon, 11 Jun 2007 19:22:15 -0400 Original-Received: from rgmgw1.us.oracle.com (rgmgw1.us.oracle.com [138.1.186.110]) by rgminet01.oracle.com (Switch-3.2.4/Switch-3.1.6) with ESMTP id l5BNMC5k029028; Mon, 11 Jun 2007 17:22:12 -0600 Original-Received: from acsmt351.oracle.com (acsmt351.oracle.com [141.146.40.151]) by rgmgw1.us.oracle.com (Switch-3.2.4/Switch-3.1.7) with ESMTP id l5BMTnvm012568; Mon, 11 Jun 2007 17:22:12 -0600 Original-Received: from dhcp-4op11-4op12-west-130-35-178-179.us.oracle.com by acsmt350.oracle.com with ESMTP id 2888458171181604082; Mon, 11 Jun 2007 16:21:22 -0700 X-Priority: 3 (Normal) X-MSMail-Priority: Normal X-Mailer: Microsoft Outlook IMO, Build 9.0.6604 (9.0.2911.0) In-Reply-To: Importance: Normal X-MimeOLE: Produced By Microsoft MimeOLE V6.00.2900.3028 X-Whitelist: TRUE X-Whitelist: TRUE X-Brightmail-Tracker: AAAAAQAAAAI= X-detected-kernel: Linux 2.4-2.6 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:72655 Archived-At: > >> Redefining completing-read is a no-no in my book. Please try > >> to rewrite your code so as not to do that (e.g. using > >> minibuffer-setup-hook). > > > I don't see how to do that. Concrete suggestions welcome. Please have > > a look at the existing code. Likewise for the other, related functions > > (`read-file-name', etc.). > > I'd rather it be the other way around: tell us what you're trying > to do (and finding you cannot do). As I said, I don't know how to approach that. What I'm trying to do now should be clear from the code, but see below, for some concrete examples. > > I'm not sure why the redefinition within Icicle mode is a > > no-no. I'm not arguing, but I don't see the reason for the > > prohibition. > > It's OK for an external package (tho I prefer defadvice), but for > reasons of maintainability and general principle it's not good > inside Emacs. We probably do have such things already inside > Emacs, but we want to reduce such occurrences rather than increase > them. > > > Perhaps if you explain what you mean about using minibuffer-setup-hook, > > there will be no need. > > E.g. Why not do the fiddling of initial-input with icicle-initial-value in > a minibuffer-setup-hook? That particular initialization is only appropriate for `completing-read', not, for example, for `read-file-name' or other functions that cause `minibuffer-setup-hook' to be invoked. In particular, `read-file-name' has a very different treatment of the initial value. I'm pretty sure that, had it been feasible to put the stuff that is done in `completing-read' onto `minibuffer-setup-hook', that would have been the first thing that I would have done. I don't recall now the details behind each of the decisions to do what is done now in `completing-read' (or in `read-file-name' etc.), and I would be reluctant to simply try to move any such code to a hook. I guess I'm asking that you trust, to some extent, that I did what I could in that regard, but I'm open to concrete suggestions if you have them. I'm grateful for new pairs of eyes and greater expertise and knowledge of Emacs code. What I don't want to do is risk breaking things. As I said, there is a lot going on, and it's not easy or obvious how to test everything after making a fundamental change. I really propose that the basic implementation be incorporated as it is now. That should be OK, since the redefinitions are encapsulated in a minor mode. We could revisit possible optimizations, cleanup, and other code improvements later, carefully, and little by little. For example, we could go through the Icicle-mode version of `completing-read' piece by piece, trying to see whether such-and-such a piece could be better done elsewhere or in a different way - just as I just did above for `icicle-initial-value'. I believe that what is there now is specific to `completing-read', and would be inappropriate (it wouldn't work) if it were moved, say, to `minibuffer-setup-hook'. Each of the pieces of code in the Icicles definition of `completing-read' has a comment explaining the intention, so I think that it should be possible for someone to make a suggestion regarding such a piece. For example: ;; Override REQUIRE-MATCH as needed. (setq require-match (case icicle-require-match-flag ((nil) require-match) (no-match-required nil) (partial-match-ok t) (full-match-required 'full-match-required)) icicle-require-match-p require-match) Here, the intent of the first `setq' pair is to override the REQUIRE-MATCH argument, based on the value of `icicle-require-match-flag', which is bound in some commands, but is generally nil. This piece of code is common to both `completing-read' and `read-file-name'; other pieces are specific to one or the other (or to other such minibuffer functions, such as `read-from-minibuffer'). Do you have a suggestion how or where better to do that? It cannot be done in `minibuffer-setup-hook', because it redefines the value of a `completing-read' argument. The intention of the second `setq' pair is not commented, but it is simply to record the REQUIRE-MATCH argument, so that it can be tested elsewhere. One place that information is used is in `icicle-narrow-candidates', which is bound to `M-*' and which you use for progressive completion, to provide a new match pattern. That command launches a new `completing-read' or a new `read-file-name' with appropriate arguments according to the current context, and the appropriate value to pass for REQUIRE-MATCH is the same value that was used originally. There are currently global variables such as `minibuffer-completion-table', but AFAIK there is no variable for the current REQUIRE-MATCH parameter. So, here's a case where, if we added such a global variable to Emacs, this bit of code in the redefined `completing-read' would go away. Again, I agree that it would be worthwhile to examine the code in such detail, with an eye to seeing what might in fact be needed in Emacs generally (such as a `minibuffer-require-match' variable, perhaps), and also with an eye to seeing how I might better do some things elsewhere and differently than by redefining `completing-read'. I just don't expect that there is one simple solution for all such stuff, and I don't think we should try to do that now, except possibly for something easy and obvious. Similarly, for the other code pieces. We could examine them one by one, but I'm not sure what that would gain us. In the end, I suspect that there will be a fair amount of redefinition that is needed, whatever else is tried. (I didn't mention it, as I thought it was obvious, but the reason I change the behavior of `completing-read' etc. is so that any code that uses such functions can automatically take advantage of Icicles features.) Some of the function redefinitions could perhaps be migrated to the general Emacs code, as I mentioned earlier. I also mentioned that I think that should not be attempted now, but after leaving it only in Icicles for a while. An example of this would be the Icicles redefinition of `read-face-name'. The only change Icicles makes here is to display the face names in *Completions* using their own faces. That is an obvious candidate for inclusion in vanilla Emacs, IMO. This is the kind of thing that you guys would generally do with a patch, but from a third-party library perspective, I needed to do it by redefining (or advising) `read-face-name' temporarily for the minor mode.