[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Enhancements to "minor-mode-map-alist" functionality.
From: |
Kim F. Storm |
Subject: |
Re: Enhancements to "minor-mode-map-alist" functionality. |
Date: |
12 Apr 2002 11:31:27 +0200 |
User-agent: |
Gnus/5.09 (Gnus v5.9.0) Emacs/21.2.50 |
"Stefan Monnier" <monnier+gnu/address@hidden> 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 <address@hidden> http://www.cua.dk
- Enhancements to "minor-mode-map-alist" functionality., Kim F. Storm, 2002/04/11
- Re: Enhancements to "minor-mode-map-alist" functionality., Stefan Monnier, 2002/04/11
- Re: Enhancements to "minor-mode-map-alist" functionality.,
Kim F. Storm <=
- Re: Enhancements to "minor-mode-map-alist" functionality., Kim F. Storm, 2002/04/12
- Re: Enhancements to "minor-mode-map-alist" functionality., Stefan Monnier, 2002/04/12
- Message not available
- Message not available
- Re: Enhancements to "minor-mode-map-alist" functionality., Kim F. Storm, 2002/04/14
- Re: Enhancements to "minor-mode-map-alist" functionality., Richard Stallman, 2002/04/16
- Re: Enhancements to "minor-mode-map-alist" functionality., Kim F. Storm, 2002/04/16
- Re: Enhancements to "minor-mode-map-alist" functionality., Richard Stallman, 2002/04/18
- Re: Enhancements to "minor-mode-map-alist" functionality., Kim F. Storm, 2002/04/18
- Re: Enhancements to "minor-mode-map-alist" functionality., Stefan Monnier, 2002/04/19
- Re: Enhancements to "minor-mode-map-alist" functionality., Kim F. Storm, 2002/04/19
- Re: Enhancements to "minor-mode-map-alist" functionality., Stefan Monnier, 2002/04/19