From mboxrd@z Thu Jan 1 00:00:00 1970 Path: main.gmane.org!not-for-mail From: storm@cua.dk (Kim F. Storm) Newsgroups: gmane.emacs.devel Subject: Re: Enhancements to "minor-mode-map-alist" functionality. Date: 12 Apr 2002 11:31:27 +0200 Sender: emacs-devel-admin@gnu.org Message-ID: <5xd6x5i7ps.fsf@kfs2.cua.dk> References: <5xbscpg7zl.fsf@kfs2.cua.dk> <200204112243.g3BMhmI01190@rum.cs.yale.edu> NNTP-Posting-Host: localhost.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: main.gmane.org 1018600428 1367 127.0.0.1 (12 Apr 2002 08:33:48 GMT) X-Complaints-To: usenet@main.gmane.org NNTP-Posting-Date: Fri, 12 Apr 2002 08:33:48 +0000 (UTC) Cc: emacs-devel@gnu.org Return-path: Original-Received: from quimby.gnus.org ([80.91.224.244]) by main.gmane.org with esmtp (Exim 3.33 #1 (Debian)) id 16vwVA-0000Lw-00 for ; Fri, 12 Apr 2002 10:33:48 +0200 Original-Received: from fencepost.gnu.org ([199.232.76.164]) by quimby.gnus.org with esmtp (Exim 3.12 #1 (Debian)) id 16vwlL-0001bf-00 for ; Fri, 12 Apr 2002 10:50:31 +0200 Original-Received: from localhost ([127.0.0.1] helo=fencepost.gnu.org) by fencepost.gnu.org with esmtp (Exim 3.34 #1 (Debian)) id 16vwUT-0002H6-00; Fri, 12 Apr 2002 04:33:05 -0400 Original-Received: from mail.filanet.dk ([195.215.206.179]) by fencepost.gnu.org with smtp (Exim 3.34 #1 (Debian)) id 16vwS5-0000bh-00 for ; Fri, 12 Apr 2002 04:30:37 -0400 Original-Received: from kfs2.cua.dk.cua.dk (kfs2.local.filanet.dk [192.168.1.182]) by mail.filanet.dk (Postfix) with SMTP id EBAB97C047; Fri, 12 Apr 2002 08:30:35 +0000 (GMT) Original-To: "Stefan Monnier" In-Reply-To: <200204112243.g3BMhmI01190@rum.cs.yale.edu> Original-Lines: 176 User-Agent: Gnus/5.09 (Gnus v5.9.0) Emacs/21.2.50 Errors-To: emacs-devel-admin@gnu.org X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.0.9 Precedence: bulk List-Help: List-Post: List-Subscribe: , List-Id: Emacs development discussions. List-Unsubscribe: , List-Archive: Xref: main.gmane.org gmane.emacs.devel:2569 X-Report-Spam: http://spam.gmane.org/gmane.emacs.devel:2569 "Stefan Monnier" writes: > > However, this can be dramatically simplified by two (simple) > > enhancements to the minor-mode-map-alist functionality: > > > > 1) Provide a separate emulation-mode-map-alists variable where > > modes like cua can insert their own mode-map-alist, > > As you probably know, I'm not too excited about this, since it shouldn't > be necessary if we could enforce a bit more discipline in the way > entries are added to minor-mode-map-alist. I doubt we can find a way to enforce that. > [ Yes, I wish we had "watchers" so we could trap modifications of > particular variables. ] > Which would still not ensure anything -- if multiple packages are watching minor-mode-map-alist and rearranges it, we'd still not be able to _ensure_ any specific ordering... > But I guess that I can live with it. Good :-) > > BTW, should emulation-mode-map-alists take precedence over > minor-mode-overriding-map-alist or not ? Based on the two existing uses of minor-mode-overriding-map-alist, I don't think it matters much, but e-m-m-a should probably take precedence over m-m-o-m-a... I'll change that. > > Also, have you contacted Michael Kifer to see if that would satisfy his > needs for viper (I know he also had to do some funky post-command > fiddling of the minor-mode-map-alist). > As far as I can see, it is _exactly_ the same problem (viper has 9 keymaps). > > 2) Allow selection of the active keymaps in the alist to > > be based on evaluating a form in addition to a simple variable. > > I generally like the idea of having the keymaps more dynamic, > but I'd rather not add another form of dynamism that isn't quite > good enough for everything. To be more precise, currently you can get > the above "select the keymap dynamically" in a more generic way > (i.e. not just for minor mode maps) by using an entry of the form: > > '(menu-item "foo" data :filter fun) > > It is mostly used for dynamic menus, but also works in other keymaps. > Problem is, it only works for submaps because it is not itself > a keymap and thus can't be used for a toplevel map. I need this at the top-level maps, so it is not applicable here. But if we could support something like (keymap :filter fun ...), it would satisfy the needs for cua. > So if the dynamism you need is only in the `command-remap' submap, > for example, your new hack is not needed. I don't think this is a hack! If so, the minor-mode-map-alist itself is a hack, since it has several obvious limitations. One of which is remedied by the minor-mode-overriding-map-alist! Actually, we could discard that feature (don't say that we should) and use either the dynamic keymap selection suggested, e.g. >From diff-mode.el: ;; Neat trick from Dave Love to add more bindings in read-only mode: (add-to-list (make-local-variable 'minor-mode-overriding-map-alist) (cons 'buffer-read-only diff-mode-shared-map)) could be replaced by: ;; Neat trick from Dave Love to add more bindings in read-only mode: (add-to-list 'minor-mode-map-alist `(lambda . (and diff-minor-mode buffer-read-only '(diff-minor-mode . ,diff-mode-shared-map)))) Or maybe a simple command remapping the the buffer's local map would do just as well, e.g. >From help-mode.el: ;; View mode steals RET from us. (set (make-local-variable 'minor-mode-overriding-map-alist) (list (cons 'view-mode (let ((map (make-sparse-keymap))) (set-keymap-parent map view-mode-map) (define-key map "\r" 'help-follow) map)))) could simply be ;; View mode steals RET from us. (local-set-key [remap View-scroll-line-forward] 'help-follow) [Actually, I think I'll install that change in any case]. That's the two uses of minor-mode-overriding-map-alist I could find. > > I'd rather not add a hack that's specific to minor-mode-map-alist > if we could do the same for all cases instead. > Sure. But what device do you suggest then? (keymap :filter fun ...) ? I don't object to that, but it would be _less_ efficient, since we have to search the keymap for the :filter property (like we do for a menu-item, but that is much shorter) [unless we ensure it stays at the head of the keymap list]. Also, the keymap lookup code would also have to be careful not to interpret the FUN part as a binding, and when we add binding to a map, we should be careful not to split the ":filter FUN" pair. > > I have already implemented these features (see attached patch), > > which allows me to configure all the minor mode keymaps needed > > by cua once and for all: > > In the patch, you end up evaluating elisp code from a function > that already has a comment about the fact that it needs to be extra-careful > about memory allocation. I know, but given the other options, I would expect this to do a lot less memory allocations (if any!) than the alternative (of regenerating minor-mode-map-alist after every command - and setting the 9 destinct variables needed to control it - as viper does [more or less]). In my case (cua), I don't think I make any memory allocations there at all since I only test a number of variables -- not modify anything. > Also I'm not sure that all places that call this > function are ready to have elisp code executed at that point (you might > need some GCPROs added here and there). As I've pointed out before, a lot of functions in keymap already lack proper GCPROs due to the autoload enhancements to get_keymap... And now you tell me that get_keyelt can GC as well... I don't see any GCPROs related to that either... So GC-wise this only makes things marginally worse :-/ > > In other words, maybe we shouldn't evaluate the form inside this > current_minor_maps function. > I still think it is pretty safe to do so. But I think it would be better to use menu_item_eval_property than safe_eval for this purpose. > > Stefan > > PS: by the way, isn't it enough to fiddle with minor-mode-map-alist > in after-load-hook rather than in post-command-hook ? > [ assuming we had such an after-load-hook. ] > > If some packages decide to fiddle with minor-mode-map-alist when you activate the mode the first time (rather than on load), I don't think an after-load-hook will catch that. -- Kim F. Storm http://www.cua.dk