Re: On keybindings and the slow erosion of help's utility

Stefan Monnier <[email protected]> writes:

> Thanks, sorry for the delay, and here are some questions/comments:
>
>> - nil → not applicable; try the next function
>> - =#'command= → run this command interactively
>> - =#'keymap= → return this keymap (Emacs enters it for further lookup)
>> - ='keymap-variable= → a symbol whose value is returned as a keymap
>
> Not completely sure what you mean by the above notation.
> Do you mean that the function should return values of the form
> `(function CMD)` and `(quote VAR)`?
> If so, why use those notations that make it look like an ELisp
> expression instead of using something like `(:command CMD)`
> and `(:keymap KM)`?

I meant that the return value could be similar to what we already have
inside keymaps (as in 23.3 Format of Keymaps section of Elisp manual):
a command to run, another keymap, etc.

> Also, what is the use-case for `(quote KEYMAP-VARIABLE)`?
> Is it supposed to behave differently than returning
> `(function KEYMAP)` where KEYMAP is the value of KEYMAP-VARIABLE?

My idea was having something like

(keymap-dynamic-bind ...
  (:when #'org-at-table-p 'org-table-fedit-map))

instead of 

(keymap-dynamic-bind ...
  (lambda () (when (org-at-table-p) org-table-fedit-map)))

>> Two special variables govern impure functions:
>>
>>     dynamic-bind-status       ;; nil = handled; 'continue = try next case
>>     dynamic-bind-active-case   ;; current case's description (read-only)
>
> I don't understand what you mean by that (nor what you mean by "impure
> functions").

Let me explain with the example

	  (defun org-metaright--try-hook ()
	    "Run org-metaright-functions; continue if none handles it."
	    (interactive)
	    (unless (run-hook-with-args-until-success
		     'org-metaright-functions)
	      (setq dynamic-bind-status 'continue)))

	  (keymap-set org-mode-map "M-<right>"
	    (keymap-dynamic-bind org-metaright-filters
	      (:doc "Demote heading, list item, or move table column right.")
	      #'org-metaright--try-hook))

In the above, org-metaright-functions can either do nothing (every
abnormal hook returns nil), or perform an action and return non-nil.
There is no separation between checking condition when to run a common
and running the command itself. (Recall our earlier discussion about
completion-at-point-functions)

My idea how to handle such scenario is having a global variable where we
can flag whether we should continue processing the keymap or not.
The advantage of global variable is also that it can be set by
underlying functions down the call chain.

Hope it is clear now.

"Impure" here refers to #'fn in keymap-dynamic-bind (see the table) -
custom function that will combine :when filter and running command. It
is impure in a sense that it does not separate things.

>> /C-level binding head: =dynamic-bind=/
>>
>> Recognized by =access_keymap= (analogous to =menu-item=):
>>
>>     (dynamic-bind
>>       :filter   FILTER-FN      ; runs hook: hook → command/keymap/nil
>>       :doc      DOC-FN         ; (dynamic-binding) → string for C-h k
>>       :enumerate ENUM-FN       ; (dynamic-binding) → list for where-is
>>       :default  COMMAND)       ; fallback when hook returns nil
>
> - Where is the HOOK?  Is it hidden within FILTER-FN?
> - Can `:doc` be replaced by the docstring of FILTER-FN?
> - Not sure we need/want enumerate.  Can't we use FILTER-FN for it?

This is the proposed new entry inside '(keymap ...)
The FILTER-FN, DOC-FN, and ENUM-FN are what defines the exact form of
how the conditions are processed, the documentation is formed, and
things like

	    ○ True if point is at a block.        → org-indent-block
	    ● At a heading.                       → org-do-demote

are formatted.

I proposed a very particular shape for those (to be implemented inside
user-level `keymap-dynamic-bind'), but we do not have to hard-code them.

>> /Lisp macro: =keymap-dynamic-bind=/
>>
>>     (keymap-dynamic-bind VARIABLE
>>       (:doc STRING)
>>       CASE1
>>       CASE2
>>       ...
>>       CASE-N)
>
> We can drop `:doc` and just recognize a string as being the docstring, no?

Yes, we can. But (:doc ...) is more extendable as one may potentially
want a function inside. But can be a simple string. Not critical for me.

>> Case forms:
>
>> | Macro form                                   | In the hook variable... |
>> |----------------------------------------------+-----------------------------------------------------------|
>> | =#'fn=                                         | Added as-is |
>> | =(:name NAME :when #'condition TARGET)=        | Generates named function NAME; added to hook              |
>> | =(:name NAME :when #'condition TARGET :doc X)= | Same, with explicit docstring                             |
>> | =(:when #'condition TARGET)=                   | Stores anonymous lambda in hook (always impure for =C-h k=) |
>> | =(:when #'condition TARGET :doc X)=            | Anonymous, with explicit docstring                        |
>> | =(:default TARGET)=                            | Anonymous default; fires unconditionally at the end       |
>> | =(:name NAME :default TARGET)=                 | Named default function |
>> | =(:default TARGET :doc X)=                     | Anonymous default with docstring                          |
>
> AFAIK "in the end" what we add to the hook is a function and nothing
> more, so I think we should focus the discussion on that.
> `:default` is a trivial function with an `always` condition, so we don't
> need anything special for it.

No, it is somewhat special.
Consider the other proposed idea I had:

(keymap-dynamic-bind org-metaright-filters ...)
(add-hook 'org-metaright-filters #'org-metaright--at-heading)

The add-hook should not add anything *after* :default. That will be no-op.

> ... `:doc` can be the function's docstring,
> so again nothing special is needed.
> But how is `:when` encoded into the function such that it can be
> used for the *Help*?

Consider the following example:

○ True if point is at a block.        → org-indent-block

and

(:when #'org-at-block-p #'org-indent-block :doc "True if point is at a block.")

○ comes from evaluating `org-at-block-p' (nil -> ○, non-nil -> ●,
org-at-block-p not declared side-effect-free -> ?)

"True if point is at a block." comes from :doc

"org-indent-block" comes from #'org-indent-block.

>>     (:name org-metaright--at-heading :when #'org-at-heading-p #'org-do-demote)
>>
>> the macro generates:
>>
>>     (defun org-metaright--at-heading ()
>>       "At a heading."                  ; first line of org-at-heading-p's docstring
>>       (declare (side-effect-free t))   ; inherited from org-at-heading-p
>>       (when (org-at-heading-p) #'org-do-demote))
>>
>>     (add-hook 'org-metaright-filters #'org-metaright--at-heading)
>
> Ah, so you use the function's docstring as the description of the
> `:when` condition.  I guess it'd work, but it's a bit of an abuse (that
> docstring does not describe what the function does).

Yes, unless :doc is explicitly defined.

>> (keymap-set org-mode-map "M-<right>"
>>   (keymap-dynamic-bind org-metaright-filters
>> [...]
>
> I think I'd want to give a name to the metafunction (e.g. `org-metaright`)
> Otherwise, how will users (and packages like Evil) bind it to some
> other key (that's one of the problems with `menu-item`: if you do
> `(keymap-set ... (keymap-lookup ...))` you rebind only the mapping
> currently returned by the `:filter` rather than the whole `menu-item`).

Why not simply

(defvar org-metaright (keymap-dynamic-bind ...))?

Thinking about it now, keymap-dynamic-bind is already quite similar to
defvar-keymap, but my idea was that org-metaright-filters will become a
hook (not a keymap), that can be manipulated.
We can go other way, have something like

(defvar-keymap-dynamic org-metaright
  <same arguments as in keymap-dynamic-bind>)

and then also allow

(add-hook (keymap-hook org-metaright) ...)

to manipulate the conditions inside.

>> =C-h k M-<right>= reports:
>>
>>     M-<right> runs a dynamic binding.
>>
>>     Demote heading, list item, or move table column right.
>>
>>     Cases, tried in order:
>>       ? Run org-metaright-functions; continue if none handles it.
>>       ○ Is the cursor at a table?          → org-table-move-column
>>       ○ True if point is at a drawer.       → org-indent-drawer
>>       ○ True if point is at a block.        → org-indent-block
>>       ● At a heading.                       → org-do-demote
>>       ○ Is the cursor at a plain list item? → org-indent-item
>>       ○ (default)                           → forward-word
>>
>>     '?' marks cases never called during documentation lookup
>>     (not declared side-effect-free).
>>     '●' marks the case that would match at point.
>
> I like this.  So we'd try to dynamically generate a docstring that plays
> the role of what we currently write by hand in metafunctions (like
> `indent-for-tab-command`), with the added twist that we can try to
> pinpoint which alternative is currently "active".
>
> BTW, if the hook returns a keymap rather than a command what do we put
> into the docstring?

IMHO, the simplest approach is just putting a link that will call
describe-keymap. I do not have strong preferences here and see it as a
rather minor detail.

> In that same case, won't the users sometimes want
> to do `C-h k M-<right> FOO` so as to see the binding they get for FOO in
> that keymap (especially if that keymap pops a menu) instead of (or in
> addition to) seeing the docstring of the metafunction?

Fair point.

We already have similar scenario for prefix bindings.
C-x ? will show prefix keymap description while
C-x <other keys> will show specific binding.

For the case of dynamic keymap, I think that the most sensible approach
will be:
1. If C-h k M-<right> resolves to a keymap, continue reading FOO and
   show the final documentation
2. If C-h k M-<right> resolves to a command, display the metacommand
   documentation instead. The command itself will be marked there with ●
   and will contain a link to command-specific documentation.
3. C-h k M-<right> ? will force displaying metacommand documentation,
   even when it resolves to the keymap
4. If none of the conditions in C-h k M-<right> triggers, and no key
   binding is resolved, display the metacommand documentation, with all
   the conditions marked ○.

>> Users can override individual named cases:
>>
>>     (defun my-heading-action ()
>>       "My custom heading handler."
>>       (declare (side-effect-free t))
>>       (when (org-at-heading-p) #'my-custom-command))
>>
>>     (remove-hook 'org-metaright-filters #'org-metaright--at-heading)
>>     (add-hook 'org-metaright-filters #'my-heading-action)
>
> Hopefully the `remove-hook` is not needed (especially since it's an
> internal function, as evidenced by the "--" in its name).

That's just an example. What I meant is that 'org-metaright-filters
(condition list in the keymap) can be manipulated, not just re-defined
wholesale.

I used "--" simply to follow the example:

	    (keymap-dynamic-bind org-metaright-filters
	      (:doc "Demote heading, list item, or move table column right.")
	      #'org-metaright--try-hook
	      (:name org-metaright--at-heading :when #'org-at-heading-p #'org-do-demote)
              ....

The idea is that we can reuse :name.

>> /Example 2: anonymous =:when= (impure)/
>
> I'm not fond of (ab)using `side-effect-free`, to be honest.
> There are several different notions of "pure", so I wouldn't be
> surprised if such an abuse could bite us in some cases.

Sure. But what are our options? With can follow how menu filters are
described and simply *ask* people use side-effect-free functions inside.
Asking explicit declaration is just a stronger urge.

>> /Example 3: replacing =indent-for-tab-command=/
>>
>>     (defun indent-for-tab--try-indentation ()
>>       "Indent; if nothing changed, continue."
>>       (interactive)
>>       (let ((prev (current-column)))
>>         (indent-according-to-mode)
>>         (when (eq prev (current-column))
>>           (setq dynamic-bind-status 'continue))))
>>
>>     (keymap-set global-map "TAB"
>>       (keymap-dynamic-bind indent-for-tab-filters
>>         (:doc "Indent or complete, depending on context.")
>>         (:name indent-for-tab--completion
>>                :when #'completion-at-point-available-p
>>                #'completion-at-point
>>                :doc "Completion is available at point.")
>>         #'indent-for-tab--try-indentation
>>         (:default #'indent-relative))))
>
> Side note: I don't really know how to write a usable
> `completion-at-point-available-p` here.  Also `indent-for-tab-command`s
> integration of indentation and completion goes the other way around: we
> use completion only after trying indentation.

The key is `dynamic-bind-status'. It can be used to affect what should
count as a command and what should be simply treated as keymap condition check.
If you tell me more about what is needed (and point to sources), I can
try to see how we can implement what you have in mind using the proposed API.

>> In =access_keymap= (src/keymap.c):
>>
>> 1. Define symbols =Qdynamic_bind=, =QCfilter=, =QCdoc=, =QCenumerate=, =QCdefault=.
>>
>> 2. After the existing =menu-item= check, add a check for =dynamic-bind=:
>>    - Extract =:filter= from the plist.
>>    - Call it with the binding and =:default=.
>>    - If it returns non-nil, use that as the resolved binding.
>>    - Otherwise use =:default= or nil (fallthrough to parent keymaps).
>
> IOW it behaves just like it does for `menu-item` and arguably we could
> just use `menu-item` instead here (modulo the limitations of `menu-item`
> for the other parts of the functionality we want).

Agree. Maybe we can even merge menu-item into this new functionality.

>> Integration with =where-is=
>> ============================
>>
>> The =:enumerate= function calls each =side-effect-free= function in the
>> hook and collects the returned command symbols.  For keymap targets, the
>> keymap is traced recursively via =where-is-internal=.  For impure
>> functions, the result is not statically known — a documented limitation.
>
> IIRC currently `where-is-internal` does not run any ELisp code (except
> the one used initially to get the `current-active-maps`).  E.g. it does
> not run `:filter`s for `menu-item`s.
>
> IIRC this was the case many years ago for technical reasons, but
> I can't remember exactly why.  Maybe this is not a problem any more (I
> have the vague impression the problem was linked with GCPRO issues and
> may have disappeared when we switched to a conservative stack scanning).
>
> It would be nice to get `where-is` to be able to find both the binding of
> `org-metaright` *and* the binding of `org-table-move-column` within it.

>> The =:doc= function only calls functions whose =side-effect-free=
>> property is non-nil — the same contract as =menu-item :filter=.
>
> Hmm... wasn't aware of this use of `side-effect-free` for
> menu-item filters.  Could you point me to the corresponding code/commit?

This is simply a reference to the fact that menu-item :filter items are
*supposed* to be side-effect-free.

   ‘:filter FILTER-FN’
        This property provides a way to compute the menu item dynamically.
        The property value FILTER-FN should be a function of one argument;
        when it is called, its argument will be REAL-BINDING.  The function
        should return the binding to use instead.

        Emacs can call this function at any time that it does redisplay or
        operates on menu data structures, so you should write it, so it can
        safely be called at any time.

>> Nested dynamic bindings rebind =dynamic-bind-status= and
>> =dynamic-bind-active-case= per dispatch.
>
> I didn't understand this comment either.

I hope that the above examples made things more clear.

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>