[PATCH] font-core: add font-lock-{enable, disable}-global-for functions

[PATCH] font-core: add font-lock-{enable, disable}-global-for functions

[Michal Nazarewicz]

From: Michal Nazarewicz <mina86@mina86.com>

* lisp/font-core.el (font-lock-enable-global-for)
(font-lock-disable-global-for): New functions for easy manipulation
of `font-lock-global-modes' variable.

* test/automated/font-core.el (font-lock-test-disable-global-for)
(font-lock-test-enable-global-for): New tests for aforementioned
functions.

* lisp/speedbar.el: Use `font-lock-disable-global-for' instead of
implementing the same functionality.
---
 lisp/font-core.el           | 41 ++++++++++++++++++++++++++++++
 lisp/speedbar.el            | 10 ++------
 test/automated/font-core.el | 62 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 105 insertions(+), 8 deletions(-)
 create mode 100644 test/automated/font-core.el

diff --git a/lisp/font-core.el b/lisp/font-core.el
index 5dd6ad3..667d650 100644
--- a/lisp/font-core.el
+++ b/lisp/font-core.el
@@ -276,6 +276,47 @@ means that Font Lock mode is turned on for buffers in C and C++ modes only."
     (let (inhibit-quit)
       (turn-on-font-lock))))
 
+;;;###autoload
+(defun font-lock-disable-global-for (mode)
+  "Disable `global-font-lock-mode' for MODE major mode.
+Modify `font-lock-global-modes' variable so that
+`global-font-lock-mode' is disabled for MODE.
+
+If `font-lock-global-modes' is a '(not …) list, make sure MODE is
+on the list; if it's a list, make sure MODE is not on it; if it's
+nil, do nothing; otherwise changes the variable to '(not MODE)."
+  (let ((modes font-lock-global-modes))
+    (cond
+     ;; t, change to '(not MODE) to disable for MODE
+     ((not (listp modes))
+      (setq font-lock-global-modes (list 'not mode)))
+     ;; (not …), add MODE to the list to disable in MODE
+     ((eq (car modes) 'not)
+      (unless (memq mode (cdr modes))
+        (setcdr modes (cons mode (cdr modes)))))
+     ;; list of modes (may be nil), make sure MODE is not on it
+     ((setq font-lock-global-modes (delq mode modes))))))
+
+;;;###autoload
+(defun font-lock-enable-global-for (mode)
+  "Enable `global-font-lock-mode' for MODE major mode.
+Modify `font-lock-global-modes' variable so that
+`global-font-lock-mode' is enabled for MODE.
+
+If `font-lock-global-modes' is a '(not …) list, make sure MODE is
+not on the list; if it's a list, make sure MODE is on it; if it's
+nil, change the variable to '(MODE) list; otherwise do nothing."
+  (let ((modes font-lock-global-modes))
+    (cond
+     ;; t, nothing to do, already enabled for all modes
+     ((not (listp modes)))
+     ;; (not …), make sure MODE is not on the list
+     ((eq (car modes) 'not)
+      (unless (setcdr modes (delq mode (cdr modes)))
+        (setq font-lock-global-modes t)))
+     ;; list of modes (may be nil), make sure MODE is on it
+     ((add-to-list 'font-lock-global-modes mode)))))
+
 (define-globalized-minor-mode global-font-lock-mode
   font-lock-mode turn-on-font-lock-if-desired
   ;; What was this :extra-args thingy for?  --Stef
diff --git a/lisp/speedbar.el b/lisp/speedbar.el
index 2989274..3c980f7 100644
--- a/lisp/speedbar.el
+++ b/lisp/speedbar.el
@@ -4068,14 +4068,8 @@ TEXT is the buffer's name, TOKEN and INDENT are unused."
 	    (def-edebug-spec speedbar-with-writable def-body)))
 
 ;; Fix a font lock problem for some versions of Emacs
-(and (boundp 'font-lock-global-modes)
-     font-lock-global-modes
-     (if (eq font-lock-global-modes t)
-	 (setq font-lock-global-modes '(not speedbar-mode))
-       (if (eq (car font-lock-global-modes) 'not)
-	   (add-to-list 'font-lock-global-modes 'speedbar-mode t)
-	 (setq font-lock-global-modes (delq 'speedbar-mode
-					    font-lock-global-modes)))))
+(when (fboundp 'font-lock-disable-global-for)
+  (font-lock-disable-global-for 'speedbar-mode))
 \f
 ;;; Obsolete variables and functions
 
diff --git a/test/automated/font-core.el b/test/automated/font-core.el
new file mode 100644
index 0000000..647488b
--- /dev/null
+++ b/test/automated/font-core.el
@@ -0,0 +1,62 @@
+;;; font-core.el --- ERT tests for font-core.el -*- lexical-binding: t -*-
+
+;; Copyright (C) 2015 Free Software Foundation, Inc.
+
+;; Author: Michal Nazarewicz <mina86@mina86.com>
+;; Keywords: languages, faces
+;; Package: emacs
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; This package defines regression tests for the font-core package.
+
+;;; Code:
+
+(require 'ert)
+(require 'font-core)
+
+(defun font-lock-test-global-modes (func expected-pairs)
+  (dolist (pair expected-pairs)
+    (let ((font-lock-global-modes (car pair)))
+      (funcall func 'foo-mode)
+      (should (equal (cdr pair) font-lock-global-modes)))))
+
+(ert-deftest font-lock-test-disable-global-for ()
+  "Test `font-lock-disable-global-for' function"
+  (font-lock-test-global-modes 'font-lock-disable-global-for
+                               '((nil . nil)
+                                 (t . (not foo-mode))
+                                 ((not bar-mode) . (not foo-mode bar-mode))
+                                 ((not foo-mode) . (not foo-mode))
+                                 ((foo-mode) . nil)
+                                 ((foo-mode bar-mode) . (bar-mode)))))
+
+(ert-deftest font-lock-test-enable-global-for ()
+  "Test `font-lock-enable-global-for' function"
+  (font-lock-test-global-modes 'font-lock-enable-global-for
+                               '((nil . (foo-mode))
+                                 (t . t)
+                                 ((not bar-mode) . (not bar-mode))
+                                 ((not foo-mode bar-mode) . (not bar-mode))
+                                 ((not foo-mode) . t)
+                                 ((foo-mode) . (foo-mode))
+                                 ((bar-mode) . (foo-mode bar-mode)))))
+
+(provide 'font-core-tests)
+
+;;; font-core.el ends here
-- 
2.2.0.rc0.207.ga3a616c

Re: [PATCH] font-core: add font-lock-{enable, disable}-global-for functions

[Stefan Monnier]

> (font-lock-disable-global-for): New functions for easy manipulation
> of `font-lock-global-modes' variable.

Can you give some background about why/when we'd need that?
Also, do you happen to know why speedbar.el does this funny dance to
avoid global-font0lock-mode from enabling font-lock in speedbar-mode?


        Stefan

Re: [PATCH] font-core: add font-lock-{enable, disable}-global-for functions

[Michal Nazarewicz]

On Sat, Jan 03 2015, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>> (font-lock-disable-global-for): New functions for easy manipulation
>> of `font-lock-global-modes' variable.

> Can you give some background about why/when we'd need that?

This happened when I've been asked how to disable global-font-lock-mode
in compile-mode buffers.  Simply replying:

    (setq font-lock-global-modes '(not compilation-mode))

is not a good answer since it may destroy any previous configuration
user might have in their init.el.  I'd imagine those functions would be
used in init.el.

I also thought they could be useful for major modes which wish to
disable GFF, but then I've discovered that all of them just do:

    (set (make-local-variable 'font-lock-global-modes) nil)

> Also, do you happen to know why speedbar.el does this funny dance to
> avoid global-font-lock-mode from enabling font-lock in speedbar-mode?

No idea, but it's not the only major mode to disable font-lock-mode.
I assumed that was one of the use cases for the font-lock-global-modes
variable.  I also had troubles with font-lock interacting with notmuch,
see <http://notmuchmail.org/pipermail/notmuch/2012/012244.html>, so
I assume there is something that breaks speedbar when font-lock is
enabled.

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--

Re: [PATCH] font-core: add font-lock-{enable, disable}-global-for functions

[Stefan Monnier]

> This happened when I've been asked how to disable global-font-lock-mode
> in compile-mode buffers.  Simply replying:

>     (setq font-lock-global-modes '(not compilation-mode))

How 'bout

   (add-hook 'compilation-mode-hook (lambda () (font-lock-mode -1)))

> I also thought they could be useful for major modes which wish to
> disable GFF, but then I've discovered that all of them just do:
>     (set (make-local-variable 'font-lock-global-modes) nil)

Hmm... makes me wonder also why those modes would disable gffm.

>> Also, do you happen to know why speedbar.el does this funny dance to
>> avoid global-font-lock-mode from enabling font-lock in speedbar-mode?
> No idea, but it's not the only major mode to disable font-lock-mode.
> I assumed that was one of the use cases for the font-lock-global-modes
> variable.  I also had troubles with font-lock interacting with notmuch,
> see <http://notmuchmail.org/pipermail/notmuch/2012/012244.html>, so
> I assume there is something that breaks speedbar when font-lock is
> enabled.

But disabling gffm doesn't disable font-lock-mode, and it is easier
to really "disable" font-lock-mode: just don't set font-lock variables.


        Stefan

Re: [PATCH] font-core: add font-lock-{enable, disable}-global-for functions

[Michal Nazarewicz]

On Sat, Jan 03 2015, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>> This happened when I've been asked how to disable global-font-lock-mode
>> in compile-mode buffers.  Simply replying:
>
>>     (setq font-lock-global-modes '(not compilation-mode))
>
> How 'bout
>
>    (add-hook 'compilation-mode-hook (lambda () (font-lock-mode -1)))

So what is font-lock-global-modes for anyway?  If a hook is the best way
to disable font-lock-mode, what does font-lock-global-modes give us?

>> I also thought they could be useful for major modes which wish to
>> disable GFF, but then I've discovered that all of them just do:
>>     (set (make-local-variable 'font-lock-global-modes) nil)
>
> Hmm... makes me wonder also why those modes would disable gffm.
>
>>> Also, do you happen to know why speedbar.el does this funny dance to
>>> avoid global-font-lock-mode from enabling font-lock in speedbar-mode?
>> No idea, but it's not the only major mode to disable font-lock-mode.
>> I assumed that was one of the use cases for the font-lock-global-modes
>> variable.  I also had troubles with font-lock interacting with notmuch,
>> see <http://notmuchmail.org/pipermail/notmuch/2012/012244.html>, so
>> I assume there is something that breaks speedbar when font-lock is
>> enabled.
>
> But disabling gffm doesn't disable font-lock-mode, and it is easier
> to really "disable" font-lock-mode: just don't set font-lock variables.
>
>
>         Stefan

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--

Re: [PATCH] font-core: add font-lock-{enable, disable}-global-for functions

[Stefan Monnier]

>> (add-hook 'compilation-mode-hook (lambda () (font-lock-mode -1)))
> So what is font-lock-global-modes for anyway?

I don't know.


        Stefan

Re: [PATCH] font-core: add font-lock-{enable, disable}-global-for functions

[Dmitry Gutov]

On 01/04/2015 04:39 PM, Stefan Monnier wrote:
>>> (add-hook 'compilation-mode-hook (lambda () (font-lock-mode -1)))
>> So what is font-lock-global-modes for anyway?
>
> I don't know.

For centralized user customization, I believe.

It's a nice pattern: you see all the major modes font-lock-mode won't be 
enabled in. I've copied it in company-mode when users asked for this 
capability.

Re: [PATCH] font-core: add font-lock-{enable, disable}-global-for functions

[Michal Nazarewicz]

On Sun, Jan 04 2015, Dmitry Gutov <dgutov@yandex.ru> wrote:
> On 01/04/2015 04:39 PM, Stefan Monnier wrote:
>>>> (add-hook 'compilation-mode-hook (lambda () (font-lock-mode -1)))
>>> So what is font-lock-global-modes for anyway?
>>
>> I don't know.
>
> For centralized user customization, I believe.
>
> It's a nice pattern: you see all the major modes font-lock-mode won't be 
> enabled in. I've copied it in company-mode when users asked for this 
> capability.

It doesn't work with derived modes though, does it?

Also, if that's a pattern that's going to be repeated, should there be
some centralised code for handling it?

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--

Re: [PATCH] font-core: add font-lock-{enable, disable}-global-for functions

[Dmitry Gutov]

On 01/04/2015 05:43 PM, Michal Nazarewicz wrote:

> It doesn't work with derived modes though, does it?

It doesn't. That would be easy to fix, but I'm sure it makes sense for 
font-lock.

> Also, if that's a pattern that's going to be repeated, should there be
> some centralised code for handling it?

M-x customize-variable ?

`turn-on-font-lock-if-desired' is simple enough to be copied and tweaked 
as desired (in some cases to maybe consider derived modes, but not in 
others).

Re: [PATCH] font-core: add font-lock-{enable, disable}-global-for functions

[Stefan Monnier]

> For centralized user customization, I believe.

But the redundancy is rather problematic, not to mention the fact that
the semantics is unclear (e.g. how does it interact with derived modes?
How does it interact with explicit calls in the mode hook? ...).

> It's a nice pattern: you see all the major modes font-lock-mode won't be
> enabled in.

Maybe we should provide something like M-x customize-minor-mode-states RET
which would collect the presence/absence of `foo-mode' or `(lambda ()
(foo-mode N))' in all the hooks and let the user add/remove them from
those hooks.


        Stefan

Re: [PATCH] font-core: add font-lock-{enable, disable}-global-for functions

[Dmitry Gutov]

On 01/04/2015 10:42 PM, Stefan Monnier wrote:

> But the redundancy is rather problematic

As long as one is for `customize-variable' and the other for elisp, I 
think it's good enough.

 > not to mention the fact that
> the semantics is unclear (e.g. how does it interact with derived modes?
> How does it interact with explicit calls in the mode hook? ...).

Right, these two aspects are not 100% transparent to a common user. 
Improving on either would be hard, though, if we want the derived modes 
sometimes to be affected (if we don't, that can just be documented).

> Maybe we should provide something like M-x customize-minor-mode-states RET
> which would collect the presence/absence of `foo-mode' or `(lambda ()
> (foo-mode N))' in all the hooks and let the user add/remove them from
> those hooks.

That might be handy.

But that wouldn't solve the "derived modes problem" any better than a 
docstring would (if it would, how?).

And scanning hooks for enabled/disabled modes would be necessarily 
imprecise: even if we take care of the three main variations 
(`foo-mode', (lambda () (foo-mode 1)), (lambda () (foo-mode -1))), that 
still leaves out random functions (lambdas or named) that include a 
`foo-mode' call among other code, that a more advanced user could have 
in their init file, or some yet-another package might include in their 
"initialize me" function.

If a function has been compiled, can we even scan its body for 
`foo-mode' calls?

Re: [PATCH] font-core: add font-lock-{enable, disable}-global-for functions

[Stefan Monnier]

>> Maybe we should provide something like M-x customize-minor-mode-states RET
>> which would collect the presence/absence of `foo-mode' or `(lambda ()
>> (foo-mode N))' in all the hooks and let the user add/remove them from
>> those hooks.
> That might be handy.
> But that wouldn't solve the "derived modes problem" any better
> than a docstring would (if it would, how?).

Depends what you mean by "derived modes problem".  My version of the
problem is when activating foo-mode in parent-mode and foo-mode fails to
be activated in child-mode.
This problem should not affect M-x customize-minor-mode-states RET.

> And scanning hooks for enabled/disabled modes would be necessarily
> imprecise: even if we take care of the three main variations (`foo-mode',
> (lambda () (foo-mode 1)), (lambda () (foo-mode -1))), that still leaves out
> random functions (lambdas or named) that include a `foo-mode' call among
> other code, that a more advanced user could have in their init file, or some
> yet-another package might include in their "initialize me" function.

That's no worse than the current font-lock-global-modes.


        Stefan

Re: [PATCH] font-core: add font-lock-{enable, disable}-global-for functions

[Dmitry Gutov]

On 01/05/2015 02:44 AM, Stefan Monnier wrote:

> Depends what you mean by "derived modes problem".  My version of the
> problem is when activating foo-mode in parent-mode and foo-mode fails to
> be activated in child-mode.

If you want derived modes to always be affected, you could just as well 
(aside from backward incompatibility) change 
`turn-on-font-lock-if-desired' to use `derived-mode-p'.

My version is more complicated: in some cases we might want derived 
modes to be affected, in others not. A graphical interface for picking 
the modes and modifiers might turn out to be more confusing than editing 
the Lisp form.

> That's no worse than the current font-lock-global-modes.

It makes to pretence of solving that problem, though. Often that's 
better than solving a problem halfway (no misconceptions for the user).

Re: [PATCH] font-core: add font-lock-{enable, disable}-global-for functions

[Dmitry Gutov]

On 01/05/2015 03:02 AM, Dmitry Gutov wrote:

> It makes to pretence of solving that problem, though. Often that's
            ^^
            no

Re: [PATCH] font-core: add font-lock-{enable, disable}-global-for functions

[Stefan Monnier]

> My version is more complicated: in some cases we might want derived modes to
> be affected, in others not.  A graphical interface for picking the modes and
> modifiers might turn out to be more confusing than editing the Lisp form.

I don't think we want that level of control in this kind of simple UI,
because it complexifies it for a very rare case anyway: it's not worth
the trouble.

>> That's no worse than the current font-lock-global-modes.
> It makes no pretence of solving that problem, though.  Often that's
> better than solving a problem halfway (no misconceptions for the
> user).

I didn't mean to claim that it solves this problem.  I only meant it as
a way to add elements to major-mode modes hooks, but seen from the point
of the view of "the element versus all the modes on which it can apply"
(and then remove the things you added this way).

It shouldn't try to recognize elements added via some other way
than Custom.


        Stefan