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

Stefan Monnier <[email protected]> writes:

>> 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.
>
> I thought "returns non-nil" is the way to say whether to continue
> processing the keymap or not.

Consider that (run-hooks-with-args-until-success ...) returns non-nil
and we thus executed the matching command by side effect.

But in the API I described, each condition can only return nil, command,
or keymap. If our (org-metaright--try-hook) already executed the
command, it cannot return nil - that would make us search further, which
we do not want. It cannot return a command or keymap either - we do not
want to execute those. Maybe, we can treat non-keymap, non-command
return value as "stop here, we are done". That's an option, yes.

>> 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.
>
> I see.  I'm trying to look at it from the `C-h k` side where we want the
> API to be minimalist (the hard coding of sub-elements would be fine on
> the side of helper macros/functions to define&set such "dynamic keymaps").

That's what I meant by

	  (dynamic-bind
	    :filter   FILTER-FN      ; runs hook: hook =E2=86=92 command/keymap/nil
	    :doc      DOC-FN         ; (dynamic-binding) =E2=86=92 string for C-h k
	    :enumerate ENUM-FN       ; (dynamic-binding) =E2=86=92 list for where-=
is
	    :default  COMMAND)       ; fallback when hook returns nil

:doc is a custom function that will take care about rendering
documentation. The `keymap-dynamic-bind' will define a particular
standard shape of :doc function, but it can be anything.

>>> 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-o=
p.
>
> I think we can handle that with the DEPTH arg to `add-hook`.

That can work. I just feel that some users will shot themselves by
passing 99 depth sooner rather than later. But I do not have hard
feeling about this particular design detail.

>>>> 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.
>
> But `:doc` is in the argument to the macro, so the question is where
> does it get stashed in the output of the macro, if not in the function's
> docstring (where it's a bit of an abuse)?

What I have in mind is

(keymap-dynamic-bind org-metaright-filters
 (:doc "Overall keymap docstring")
 (#'org-metaright--at-heading :doc "Condition-specific docstring"))

>>>> (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-metarigh=
t`)
>>> 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 ...))?
>
> Because, `org-metaright` will then not appear inside the keymap, so
> `lookup-key` still won't return what you want.
> IOW, the users need to
> dig through the source code to see that they can refer to that object
> using the variable `org-metaright` (which will signal `void-variable`
> if they do so before the package is loaded).

What about (keymap-set org-mode-map "M-<right>" 'org-metaright) ; <- symbol

> OTOH, it's fine *if* `lookup-key` doesn't evaluate the conditions and
> just returns the whole dynamic keymap.

Sure.

>> 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 =
=E2=97=8F
>>    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 =E2=97=8B.
>
> I'm wondering if there's a benefit to allowing dynamic keymaps to contain
> a mix of keymaps and commands.  I get the impression that the only
> meaningful cases will be either "all commands" or "all keymaps".
> And I also get the impression that a lot of machinery you describe would
> work well for "all commands" but wouldn't bring much benefit over the
> current `menu-item` thingies for "all keymaps".
>
> IOW my intuition tells me we had better treat those two as different
> cases with different problems to solve.  E.g. for "all keymaps" we're
> kind of forced to test the `:when` conditions during the keymap-lookup
> (so they can't have side effects), whereas for the "all commands" we can
> delay those tests to the moment we actually try to run the command.

I think https://elpa.nongnu.org/nongnu/casual.html as a whole is a
counterexample to your intuition. It defines whole keymaps depending on
cursor location. For example, see
https://elpa.nongnu.org/nongnu/doc/casual.html#Dired-Usage

Another example is context menus :)

>>>> /Example 3: replacing =3Dindent-for-tab-command=3D/
>>> ...
>>> 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.
>
> Don't know what you mean by "what you have in mind":
> - "I don't really know how to write a usable `completion-at-point-availab=
le-p`"
>   means literally that I don't know how we could write such a function
>   such that it provides a good behavior.  I think I'd probably end up
>   using something like just checking that `char-before` is not
>   whitespace which is a far cry from testing "available-p".
> - "integration of indentation and completion goes the other way around"
>   just describes what `indent-for-tab-command` does:
> ...
> <indent-for-tab-command source omitted>

One important thing in this code that we did not discuss yet is prefix
argument. It will need to be passed to the conditions.

Otherwise, let me try:

(defun indent--should-run-completion ()
   (and (eq tab-always-indent 'complete)
             (or (eq last-command this-command)
                 (let ((syn (syntax-class (syntax-after (point)))))
                   (pcase tab-first-completion
                     ('nil t)
                     ('eol (eolp))
                     ('word (not (eql 2 syn)))
                     ('word-or-paren (not (memq syn '(2 4 5))))
                     ('word-or-paren-or-punct (not (memq syn '(2 4 5 1)))))=
))))

(defun indent--for-tab-1 (arg)
 "Maybe run indent-for-tab-command. If it did not run or did nothing,
 return nil."
    (let ((old-tick (buffer-chars-modified-tick))
          (old-point (point))
	  (old-indent (current-indentation)))

      ;; Indent the line.
      (or (not (eq (indent--funcall-widened indent-line-function) 'noindent=
))
          (indent--default-inside-comment)
          (when (or (<=3D (current-column) (current-indentation))
                    (not (eq tab-always-indent 'complete)))
            (indent--funcall-widened (default-value 'indent-line-function))=
))

      (cond
       ;; If the text was already indented right, try completion.
       ((and (eql old-point (point))
             (eql old-tick (buffer-chars-modified-tick))
             (indent--should-run-completion))
        nil) ;; <<------ nil here

       ;; If a prefix argument was given, rigidly indent the following
       ;; sexp to match the change in the current line's indentation.
       (arg
        (let ((end-marker
               (save-excursion
                 (forward-line 0) (forward-sexp) (point-marker)))
              (indentation-change (- (current-indentation) old-indent)))
          (save-excursion
            (forward-line 1)
            (when (and (not (zerop indentation-change))
                       (< (point) end-marker))
              (indent-rigidly (point) end-marker indentation-change)
              t ;; <--- non-nil
              )))))))

(keymap-set global-map "TAB"
  (keymap-dynamic-bind indent-for-tab-filters
    (:doc "Indent or complete, depending on context.")
    (:when #'use-region-p #'indent-region)
    (:when
     (or ;; indent-to-left-margin is only meant for indenting,
	;; so we force it to always insert a tab here.
	(eq indent-line-function 'indent-to-left-margin)
	(and (not tab-always-indent)
	     (or (> (current-column) (current-indentation))
		 (eq this-command last-command))))
     #'insert-tab)
    #'indent--for-tab-1
    (:when #'indent--should-run-completion #'completion-at-point))))

--=20
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>