> > because one is fbound and the other is not. The background for this is

> > that I want to collect all loaded keymaps in the system:

> >

> > (defun collect-keymaps ()

> >   (cl-loop for k being the symbol when (keymapp k) collect k))

>

> The fix here is to always check the `(symbol-value k)` never `k`.

> IOW think of it "I'm looking for all the loaded strings" except that

> you're looking for keymaps instead of strings.

I realized that and was doing something similar, admittedly less

effeciently because I didn't know I can trust that all keymaps have the

value slot set. I thought, that some had function slot set, and other

value slot, and some both.

I was wondering why it was done this way, that keymap is stored into the

function slot, but I assumed it was some optimization

somewhere. Secondly, I was also in doubt since keymamp does the special

thing and accepts the symbol for fbounded ones. I thought there must be

some reason why it was done that way, and wondered why dereferencing

symbol was done only for fbounded symbols. I assumed that some keymaps

don't have value slot set.

> you're looking for keymaps instead of strings.  You wouldn't do

> `(stringp k)` but `(stringp (symbol-value k))`, right?

Or with bufferp, or some other "typep". Most (all?) work on objects and

not on symbols. I am aware of that one. But as explained above, I was

not sure what really to expect in the case of keymapp.

> It is rue that keymaps (contrary to strings) can *also* be represented

> by symbols (where it works much like it does for functions).  I don't

> know why this was done, but I consider as an historical accident and

> don't recommend making use of that "feature".

No, no, I am not trying to make use of it all; on the contrary :-). I

just wanted to find *all* prefix keys in all maps, and was a bit

confused why some maps where not found when I knew they should be.

> It's mostly used in very old code that uses the weird

> `define-prefix-command` function, such as the code that sets up some of

> the "core" keymaps like `ctl-x-5-prefix` `ESC-prefix` `ctl-x-4-prefix`,

> `Control-X-prefix`.

I was actually wondering why this was done, if it is just an

optimization or if there is some other reason? I am just curious, I

don't really understand the idea behind. It would be interesting to

learn, if somebody knows and is willing to explain, since it is atypical

to store value data in function slot.

> > As a suggestion, the attached patch dereferences symbol value slots

> > as well.

>

> It's simpler to fix *your* code rather than the rest of the world's.  🙂

Yeah, I understand that :).

Partially, as said above, I didn't know I can rely that *all* keymaps

have value slot set, and tbh I didn't know I can skipp the boundp

check. The code I used actually looked like this:

(defun collect-keymaps ()

  "Collect all keymaps in the system"

  (let (keymaps)

    (mapatoms

     (lambda (s)

       (if (keymapp s)

           (push s keymaps)

         (and (boundp s) (keymapp (symbol-value s))

              (push s keymaps)))))

    keymaps))

which is quite bad and innefficient. I see now I can do:

(defun xcv-collect-keymaps ()

  (cl-loop for s being the symbol when (keymapp (symbol-value s)) collect s))

When I check the length of the result list, it seems to find all maps.

> > If it is not a wrong thing to do for some reason I am not aware

> > of, I would like to suggest it as a "fix" or a "feature request",

> > whichever is best describing the issue.

>

> It would break existing uses when the symbol has both

> a `symbol-function` value and a `symbol-value` value.  We could consider

I saw the other functions using get_keymap, so I wasn't sure if they

would complain or not. I have run Emacs today, the entire day, with that

patch, I haven't noticed anything weird. But I am not an advanced

user. I probably don't hit any functions that might touch that

functionality and I don't have any tests neither.

Not pushing for it, more as a curiosa: what do you think about not

touching get_keymap at all, and refactoring out keymmapp like this:

DEFUN ("keymapp", Fkeymapp, Skeymapp, 1, 1, 0,

  (Lisp_Object object)

{

  if (NILP (object))

    return Qnil;

  if (CONSP (object) && EQ (XCAR (object), Qkeymap))

    return object;

  if (SYMBOLP (object))

    {

      Lisp_Object tem = indirect_function (object);

      if (CONSP (tem) && EQ (XCAR (tem), Qkeymap))

      return tem;

 

      tem = find_symbol_value (object);

      if (CONSP (tem) && EQ (XCAR (tem), Qkeymap))

      return tem;

    }

  return Qnil;

}

I don't see any use of Fkeymapp from C code, but it is used quite a lot

in Lisp code. I do run that keymapp version now in my Emacs when I write

this, but that does not mean it is always right.

> adding it as a fallback (i.e. use the `symbol-value` slot only *after*

> checking the `symbol-function` slot), but that would be

> a somewhat weird/unreliable feature, so we'd need a compelling use-case

> (the only advantage I can see of using `symbol-value` over

> `symbol-function` is that it can be made buffer-local, so you could

> e.g. make a keymap whose parent changes dynamically depending on the

> current-buffer, but I don't know what would be the corresponding

> "compelling use-case").

I am not sure I have a compelling reason. It would be just slightly more

efficient and slightly more elegant to just check for the keymmapp and

not have to take symbol-value, but my biggest motivation for the

suggestion was the confusion when I discovered what was hapening. It

wasn't clear why one symbol worked but other didn't. Now, when I know

that the value slot can always be used, it is not an issue.  I am

affraid that is the best I can come up with. Buffer local keymaps can be

achieved with a buffer-local variable, whose value is switched between

two different keymaps too, but it would be more elegant if they could be

buffer local. I don't see myself a use-case for neither, but what do I

know; someone else might have one.

Thanks for looking at it and for the detailed answer.

best regards

/a