bug#51974: 29.0.50; [PATCH] Fix deactivation of outline-minor-mode-cycle-map bindings

bug#51974: 29.0.50; [PATCH] Fix deactivation of outline-minor-mode-cycle-map bindings

[miha--- via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1.1: Type: text/plain, Size: 268 bytes --]

Customize 'outline-minor-mode-cycle' to t and enable outline-minor-mode.
Put point on a heading and TAB cycles visibility as expected. However,
disabling the minor mode, the keymap remains active and TAB still cycles
on this heading. I propose this patch to fix this.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-Fix-deactivation-of-outline-cycle-bindings.patch --]
[-- Type: text/x-patch, Size: 2943 bytes --]

From ab15d70f3d8d182a44ea8797732c239aa9f7a8ae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miha=20Rihtar=C5=A1i=C4=8D?= <miha@kamnitnik.top>
Date: Fri, 19 Nov 2021 11:38:34 +0100
Subject: [PATCH] Fix deactivation of outline cycle bindings

* lisp/outline.el (outline-minor-mode):
(outline-font-lock-keywords): Don't modify the 'keymap' text property
directly.  Modify 'outline-cycle-keymap' property and have the minor
mode set it up as an alias to the 'keymap' property, or reset it on
deactivation.
---
 lisp/outline.el | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/lisp/outline.el b/lisp/outline.el
index a4d2a3b7d7..e41ec364f4 100644
--- a/lisp/outline.el
+++ b/lisp/outline.el
@@ -226,9 +226,11 @@ outline-font-lock-keywords
                          (if outline-minor-mode-cycle
                              (if outline-minor-mode-highlight
                                  (list 'face (outline-font-lock-face)
-                                       'keymap outline-minor-mode-cycle-map)
+                                       'outline-cycle-keymap
+                                       outline-minor-mode-cycle-map)
                                (list 'face nil
-                                     'keymap outline-minor-mode-cycle-map))
+                                     'outline-cycle-keymap
+                                     outline-minor-mode-cycle-map))
                            (if outline-minor-mode-highlight
                                (list 'face (outline-font-lock-face))))
                        (outline-font-lock-face))
@@ -428,6 +430,11 @@ outline-minor-mode
           (if (and global-font-lock-mode (font-lock-specified-p major-mode))
               (progn
                 (font-lock-add-keywords nil outline-font-lock-keywords t)
+                (setq-local char-property-alias-alist
+                            (copy-alist char-property-alias-alist))
+                (cl-pushnew
+                 'outline-cycle-keymap
+                 (alist-get 'keymap char-property-alias-alist))
                 (font-lock-flush))
             (outline-minor-mode-highlight-buffer)))
 	;; Turn off this mode if we change major modes.
@@ -438,8 +445,10 @@ outline-minor-mode
 	;; Cause use of ellipses for invisible text.
 	(add-to-invisibility-spec '(outline . t)))
     (when (or outline-minor-mode-cycle outline-minor-mode-highlight)
-      (if font-lock-fontified
-          (font-lock-remove-keywords nil outline-font-lock-keywords))
+      (when font-lock-fontified
+        (font-lock-remove-keywords nil outline-font-lock-keywords)
+        (when-let ((as (assq 'keymap char-property-alias-alist)))
+          (setcdr as (remq 'outline-cycle-keymap (cdr as)))))
       (remove-overlays nil nil 'outline-overlay t)
       (font-lock-flush))
     (setq line-move-ignore-invisible nil)
-- 
2.34.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

bug#51974: 29.0.50; [PATCH] Fix deactivation of outline-minor-mode-cycle-map bindings

[Lars Ingebrigtsen]

miha@kamnitnik.top writes:

> -                                       'keymap outline-minor-mode-cycle-map)
> +                                       'outline-cycle-keymap
> +                                       outline-minor-mode-cycle-map)

I'm not very enthusiastic about messing with char-property-alias-alist
for something like this, but perhaps this is the best solution here?  I
guess we don't have a mechanism to make a text property keymap "go away"
when a minor mode is deactivated?

Anybody got an opinion here?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51974: 29.0.50; [PATCH] Fix deactivation of outline-minor-mode-cycle-map bindings

[miha--- via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> miha@kamnitnik.top writes:
>
>> -                                       'keymap outline-minor-mode-cycle-map)
>> +                                       'outline-cycle-keymap
>> +                                       outline-minor-mode-cycle-map)
>
> I'm not very enthusiastic about messing with char-property-alias-alist
> for something like this, but perhaps this is the best solution here?  I
> guess we don't have a mechanism to make a text property keymap "go away"
> when a minor mode is deactivated?

We have, it's called 'font-lock-extra-managed-props'. Using it, we can
do this in a simpler way:

diff --git a/lisp/outline.el b/lisp/outline.el
index a4d2a3b7d7..7f78c04866 100644
--- a/lisp/outline.el
+++ b/lisp/outline.el
@@ -427,6 +427,9 @@ outline-minor-mode
         (when (or outline-minor-mode-cycle outline-minor-mode-highlight)
           (if (and global-font-lock-mode (font-lock-specified-p major-mode))
               (progn
+                (when outline-minor-mode-cycle
+                  (add-to-list 'font-lock-extra-managed-props 'keymap
+                               nil #'eq))
                 (font-lock-add-keywords nil outline-font-lock-keywords t)
                 (font-lock-flush))
             (outline-minor-mode-highlight-buffer)))

This has a small disadvantage: deactivating outline-minor-mode will
remove all 'keymap' text properties from the whole buffer, even if they
are added by other minor or major modes or whatever. With
'char-property-alias-alist', we can create a sort of namespacing to
ensure that outline-minor-mode messes only with its own text properties.

'font-lock-mode' minor mode modifies 'char-property-alias-alist' in a
similar way for a similar reason.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

bug#51974: 29.0.50; [PATCH] Fix deactivation of outline-minor-mode-cycle-map bindings

[Lars Ingebrigtsen]

<miha@kamnitnik.top> writes:

> This has a small disadvantage: deactivating outline-minor-mode will
> remove all 'keymap' text properties from the whole buffer, even if they
> are added by other minor or major modes or whatever.

Yes, that's not good, either.  Alternatively, it could remove all the
`keymap' entries where the keymap is the outline keymap.  (This would
break if the keymap is redefined, but...)

> With 'char-property-alias-alist', we can create a sort of namespacing
> to ensure that outline-minor-mode messes only with its own text
> properties.

Yup.  Hm.  Well, perhaps it is the best solution here.  It's just pretty
unusual to use char-property-alias-alist for something like this.  I
guess other minor modes use overlays, which makes it easier to identify
what things belong to the minor mode?  (Text properties have a vaguer
identity.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51974: 29.0.50; [PATCH] Fix deactivation of outline-minor-mode-cycle-map bindings

[Juri Linkov]

>> This has a small disadvantage: deactivating outline-minor-mode will
>> remove all 'keymap' text properties from the whole buffer, even if they
>> are added by other minor or major modes or whatever.
>
> Yes, that's not good, either.  Alternatively, it could remove all the
> `keymap' entries where the keymap is the outline keymap.  (This would
> break if the keymap is redefined, but...)
>
>> With 'char-property-alias-alist', we can create a sort of namespacing
>> to ensure that outline-minor-mode messes only with its own text
>> properties.
>
> Yup.  Hm.  Well, perhaps it is the best solution here.  It's just pretty
> unusual to use char-property-alias-alist for something like this.  I
> guess other minor modes use overlays, which makes it easier to identify
> what things belong to the minor mode?  (Text properties have a vaguer
> identity.)

Looks like the best solution.

Also outline-minor-mode-highlight-buffer that is a non-font-lock-based
counterpart of outline-font-lock-keywords needs an opposite function
to unhighlight the non-font-lock buffer by removing only faces and keymaps
that outline-minor-mode-highlight-buffer added to the buffer.
But this should be easier to do since it uses overlays.

bug#51974: 29.0.50; [PATCH] Fix deactivation of outline-minor-mode-cycle-map bindings

[Juri Linkov]

> Also outline-minor-mode-highlight-buffer that is a non-font-lock-based
> counterpart of outline-font-lock-keywords needs an opposite function
> to unhighlight the non-font-lock buffer by removing only faces and keymaps
> that outline-minor-mode-highlight-buffer added to the buffer.
> But this should be easier to do since it uses overlays.

Sorry, I forgot it already does (remove-overlays nil nil 'outline-overlay t).

bug#51974: 29.0.50; [PATCH] Fix deactivation of outline-minor-mode-cycle-map bindings

[miha--- via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Juri Linkov <juri@linkov.net> writes:

>> Also outline-minor-mode-highlight-buffer that is a non-font-lock-based
>> counterpart of outline-font-lock-keywords needs an opposite function
>> to unhighlight the non-font-lock buffer by removing only faces and keymaps
>> that outline-minor-mode-highlight-buffer added to the buffer.
>> But this should be easier to do since it uses overlays.
>
> Sorry, I forgot it already does (remove-overlays nil nil 'outline-overlay t).

Fixed by commit Mon Jan 10 20:20:09 2022 +0200
(2b7a486605c01f7927de47ec2788f1eb2a4c0142)

Thanks.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]