Re: Enhancements to "minor-mode-map-alist" functionality.

[storm@cua.dk (Kim F. Storm)]

"Stefan Monnier" <monnier+gnu/emacs@rum.cs.yale.edu> writes:

> 
> > But if we could support something like (keymap :filter fun ...),
> > it would satisfy the needs for cua.
> 
> I had something like that working in the past.  I don't think
> I have the code around any more, tho.  IIRC it wasn't too difficult
> to add.  The evaluation of the function was done in `get_keymap'
> (this is not to say that it was the right place to do it, but that
> it was my choice at the time, because it seemed simpler).

I'll give it a try...

> 
> Another idea I have been toying with (but it never got far enough
> for me to start writing any code, not even tentatively) is to use
> a "null event".  Assuming this event is called `nil', you could
> replace the above dynamic keymap with:
> 
> 	(defvar dynmap (make-sparse-keymap))
> 	(define-key dynmap [nil] '(menu-item "foo" data :filter fun))
> 
> Ideally this would also allow multiple such "same-level submaps"
> which would then provide multiple keymap inheritance.
> 
This is very similar to my new proposal for nested keymaps, but
it looks like a hack to use menu-item that way.

> > > 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!
> 
> I said "hack" because I think it's more ad-hoc than it should.
> 
> >   ;; 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))))
> 
> Indeed.
> 
> >       ;; View mode steals RET from us.
> >       (local-set-key [remap View-scroll-line-forward] 'help-follow)
> 
> That one looks completely bogus to me.  Do you really want to prevent
> the use of View-scroll-line-forward from a help-mode buffer even if
> it's bound to something else than RET ?

No, that wasn't the intention ... I must have been sleeping...

> 
> > [Actually, I think I'll install that change in any case].
> 
> Please don't.

I wont.  

But I still think the use of the minor-mode-overriding-map-alist for
this purpose is pretty obscure (although I suspect that is the
purpose for which it was invented...)

An alternative would be to put a keymap property on the link
like this:  [in CVS emacs, keymap properties take precedence over
minor mode keymaps!]

(defun help-xref-button (match-number type &rest args)
  "Make a hyperlink for cross-reference text previously matched.
MATCH-NUMBER is the subexpression of interest in the last matched
regexp.  TYPE is the type of button to use.  Any remaining arguments are
passed to the button's help-function when it is invoked.
See `help-make-xrefs'."
  ;; Don't mung properties we've added specially in some instances.
  (unless (button-at (match-beginning match-number))
    (make-text-button (match-beginning match-number)
                      (match-end match-number)
                      'type type 'help-args args 'keymap help-mode-map)))
                                                 ^^^^^^^^^^^^^^^^^^^^^


> 
> > > 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].
> 
> I don't care much for efficiency at this stage.

Neither do I !!

> 
> > Also, the keymap lookup code would also have to be careful not to
> > interpret the FUN part as a binding,
> 
> You can force it to be a symbol.

I would prefer not to do that; an alternative would be to put a
cons cell (:filter . FORM) into the keymap, causing the rest of
the keymap to be ignored if FORM returns nil.

Along this line, we could also add (lambda . FORM) in a keymap which
will evaluate FORM to get a nested keymap.

> 
> > and when we add binding to
> > a map, we should be careful not to split the ":filter FUN" pair.
> 
> Of course, it might need some care.  Doing it in get_keymap works
> around the above problem, tho since the keymap that's modified
> is the one returned by get_keymap.

I guess using (:filter . FORM) would make it easier to ensure this.

> 
> > So GC-wise this only makes things marginally worse :-/
> 
> I'd rather try to make it better than to make it marginally worse.

Sure.  

> 
> > > 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.
> 
> Despite the comment that's just above ?  Do you really think the
> coders went to the trouble of not using xmalloc/xrealloc and of
> sometimes returning an incorrect result (hoping that the cause of
> the problem will cause something else to abort later on) just
> for the fun of it ?

You are right.  I overlooked the finer details of that comment.

> 
> > 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.
> 
> And if some package decides to fiddle with it from an idle-hook, the
> post-command-hook will fail as well.  The question is not "is it theoretically
> safe" but "is it safe in practice" ?  I believe it'd be good enough in practice
> because I don't see much fiddling of minor-mode-map-alist outside of
> toplevel expressions.

I would be much easier if an element in the minor-mode-map-alist
could be tagged as `keep at head of list'.  IIRC we discussed this
some time ago on this list, and really didn't find a way to
accomplish that.

But maybe such modes could simply put their map on
minor-mode-overriding-map-alist instead ?  Is that "legal practice"?


-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk