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

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 51974 in the body.
You can then email your comments to 51974 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: miha <at> kamnitnik.top
To: bug-gnu-emacs <at> gnu.org
Subject: 29.0.50; [PATCH] Fix deactivation of outline-minor-mode-cycle-map
 bindings
Date: Fri, 19 Nov 2021 12:41:57 +0100

[Message part 1 (text/plain, inline)]

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.

[0001-Fix-deactivation-of-outline-cycle-bindings.patch (text/x-patch, inline)]

From ab15d70f3d8d182a44ea8797732c239aa9f7a8ae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miha=20Rihtar=C5=A1i=C4=8D?= <miha <at> 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

[signature.asc (application/pgp-signature, inline)]

Message #8 received at 51974 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: miha <at> kamnitnik.top
Cc: 51974 <at> debbugs.gnu.org
Subject: Re: bug#51974: 29.0.50; [PATCH] Fix deactivation of
 outline-minor-mode-cycle-map bindings
Date: Sat, 20 Nov 2021 10:45:16 +0100

miha <at> 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

Message #11 received at 51974 <at> debbugs.gnu.org (full text, mbox):

From: <miha <at> kamnitnik.top>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 51974 <at> debbugs.gnu.org
Subject: Re: bug#51974: 29.0.50; [PATCH] Fix deactivation of
 outline-minor-mode-cycle-map bindings
Date: Sat, 20 Nov 2021 11:45:40 +0100

[Message part 1 (text/plain, inline)]

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> miha <at> 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.

[signature.asc (application/pgp-signature, inline)]

Message #14 received at 51974 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: <miha <at> kamnitnik.top>
Cc: 51974 <at> debbugs.gnu.org
Subject: Re: bug#51974: 29.0.50; [PATCH] Fix deactivation of
 outline-minor-mode-cycle-map bindings
Date: Sun, 21 Nov 2021 09:12:50 +0100

<miha <at> 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

Message #17 received at 51974 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 51974 <at> debbugs.gnu.org, miha <at> kamnitnik.top
Subject: Re: bug#51974: 29.0.50; [PATCH] Fix deactivation of
 outline-minor-mode-cycle-map bindings
Date: Sun, 21 Nov 2021 19:55:43 +0200

>> 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.

Message #20 received at 51974 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 51974 <at> debbugs.gnu.org, miha <at> kamnitnik.top
Subject: Re: bug#51974: 29.0.50; [PATCH] Fix deactivation of
 outline-minor-mode-cycle-map bindings
Date: Sun, 21 Nov 2021 19:58:17 +0200

> 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).

Message #25 received at 51974-done <at> debbugs.gnu.org (full text, mbox):

From: <miha <at> kamnitnik.top>
To: Juri Linkov <juri <at> linkov.net>, Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 51974-done <at> debbugs.gnu.org
Subject: Re: bug#51974: 29.0.50; [PATCH] Fix deactivation of
 outline-minor-mode-cycle-map bindings
Date: Tue, 11 Jan 2022 20:24:23 +0100

[Message part 1 (text/plain, inline)]

Juri Linkov <juri <at> 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.

[signature.asc (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.