bug#60426: 29.0.60; Binding to outline-minor-mode-cycle-map correctly is unintuitive and hard

[Juri Linkov <juri@linkov.net>]

[-- Attachment #1: Type: text/plain, Size: 1387 bytes --]

>>   (outline-minor-mode-cycle--bind map (kbd "TAB") #'outline-cycle)
>>
>> Maybe this requirement should be documented in the docstring
>> and mentioned in NEWS?
>
> I think this is a good idea since people who have already keys bound
> on outline-minor-mode-cycle-map are likely to be as surprised as I was.
>
> But I also think that it is more important to mention this in the
> docstring for outline-minor-mode-cycle-map. Since its current
> docstring "Keymap used by outline-minor-mode-cycle." is misleading in
> my opinion, especially since any key there is going to be active
> if outline-minor-mode is on, regardless of the value of
> `outline-minor-mode-cycle`.

Thanks for suggestions.  The following patch addresses all the points
that have been raised, except obsoleting `outline-minor-mode-cycle-map`.
It's more convenient to bind keys in this map since `outline-minor-mode-map`
is not explicitly defined unlike `outline-mode-map`.

> Also I was confused by the two hyphens in the name
> outline-minor-mode-cycle--bind and thought that it was a private
> function even though in retrospect they are in the wrong place
> for it to be private.

Two hyphens in the name rather indicated that the implementation
is subject to change without notice - an assumption that is true
for internal functions.  So now I'm not sure if adding an alias
without two hyphens is worth to do.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: outline-minor-mode-cycle--bind.patch --]
[-- Type: text/x-diff, Size: 3245 bytes --]

diff --git a/etc/NEWS b/etc/NEWS
index a28f5c9a65a..6ae79e6cebc 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -350,6 +350,15 @@ next button, even if the mode has bound it to something else.  This
 also means that 'TAB' on a button in an 'outline-minor-mode' heading
 will move point instead of collapsing the outline.
 
+---
+** 'outline-minor-mode-cycle-map' is now parent of 'outline-minor-mode'.
+Instead of adding font-lock text property 'keymap' with
+'outline-minor-mode-cycle' on outline headings in 'outline-minor-mode',
+the keymap 'outline-minor-mode-cycle' is now active in the whole buffer.
+But keybindings in 'outline-minor-mode-cycle' are still take effect
+only on outline headings because they are bound with the help of
+'outline-minor-mode-cycle--bind' that checks if point is on a heading.
+
 ---
 ** 'Info-default-directory-list' is no longer populated at Emacs startup.
 If you have code in your init file that removes directories from
diff --git a/lisp/outline.el b/lisp/outline.el
index 91f6040687b..26e41735a01 100644
--- a/lisp/outline.el
+++ b/lisp/outline.el
@@ -210,7 +210,12 @@ outline-minor-mode-cycle-filter
 
 (defvar outline-minor-mode-cycle)
 (defun outline-minor-mode-cycle--bind (map key binding &optional filter)
-  (define-key map key
+  "Define KEY as BINDING in MAP using FILTER.
+The key takes effect only on the following conditions:
+`outline-minor-mode-cycle' is non-nil, point is located on the heading line,
+FILTER or `outline-minor-mode-cycle-filter' is nil or returns non-nil.
+The argument MAP is optional and defaults to `outline-minor-mode-cycle-map'."
+  (define-key (or map outline-minor-mode-cycle-map) key
     `(menu-item
       "" ,binding
       ;; Filter out specific positions on the heading.
@@ -227,8 +232,16 @@ outline-minor-mode-cycle-map
   (let ((map (make-sparse-keymap)))
     (outline-minor-mode-cycle--bind map (kbd "TAB") #'outline-cycle)
     (outline-minor-mode-cycle--bind map (kbd "<backtab>") #'outline-cycle-buffer)
+    (keymap-set map "<left-margin> <mouse-1>" 'outline-cycle)
+    (keymap-set map "<right-margin> <mouse-1>" 'outline-cycle)
+    (keymap-set map "<left-margin> S-<mouse-1>" 'outline-cycle-buffer)
+    (keymap-set map "<right-margin> S-<mouse-1>" 'outline-cycle-buffer)
     map)
-  "Keymap used by `outline-minor-mode-cycle'.")
+  "Keymap used as a parent of the `outline-minor-mode' keymap.
+It contains key bindings that can be used to cycle visibility.
+The recommended way to bind keys is with `outline-minor-mode-cycle--bind'
+when the key should be enabled only when `outline-minor-mode-cycle' is
+non-nil and point is located on the heading line.")
 
 (defvar outline-mode-map
   (let ((map (make-sparse-keymap)))
@@ -518,10 +532,6 @@ outline-minor-mode
   :keymap (define-keymap
             :parent outline-minor-mode-cycle-map
             "<menu-bar>" outline-minor-mode-menu-bar-map
-            "<left-margin> <mouse-1>" 'outline-cycle
-            "<right-margin> <mouse-1>" 'outline-cycle
-            "<left-margin> S-<mouse-1>" 'outline-cycle-buffer
-            "<right-margin> S-<mouse-1>" 'outline-cycle-buffer
             (key-description outline-minor-mode-prefix) outline-mode-prefix-map)
   (if outline-minor-mode
       (progn