Fight: font-lock.el 1.289 vs. cc-defs.el

Fight: font-lock.el 1.289 vs. cc-defs.el

[Alan Mackenzie]

Hi, Emacs!

In font-lock.el V1.289, the following check was introduced into
font-lock-compile-keywords:

--- emacs/lisp/font-lock.el     2005/12/22 16:09:32     1.288
+++ emacs/lisp/font-lock.el     2005/12/30 04:38:52     1.289
@@ -1507,6 +1507,13 @@
 `font-lock-keywords' doc string.
 If REGEXP is non-nil, it means these keywords are used for
 `font-lock-keywords' rather than for `font-lock-syntactic-keywords'."
+  (if (not font-lock-set-defaults)
+      ;; This should never happen.  But some external packages sometimes
+      ;; call font-lock in unexpected and incorrect ways.  It's important to
+      ;; stop processing at this point, otherwise we may end up changing the
+      ;; global value of font-lock-keywords and break highlighting in many
+      ;; other buffers.
+      (error "Font-lock trying to use keywords before setting them up"))
   (if (eq (car-safe keywords) t)
       keywords
     (setq keywords

This inconveniences CC Mode, thusly:

CC Mode calls (font-lock-compile-keywords "\\<\\>") at initialisation, to
test for a bug in middle aged XEmacsen which splatted font-lock-keywords.
The new check in V1.289 causes CC Mode to bail out with an error.

Andreas Schwab patched a work-around into cc-defs.el V1.38, placing a
condition-case around the call.  This seems suboptimal: If some error
occurs whilst calling some Font Lock function, we definitely want to know
about it.

A better(??) kludge would be for cc-defs to bind font-lock-set-defaults
to non-nil before calling (font-lock-compile-keywords "\\<\\>"), but this
is really ghastly coding practise.

I'm asking that this change to font-lock.el be reconsidered, though I
admit I don't know the problem it fixes.  Might a better solution be
(make-variable-buffer-local 'font-lock-keywords)?  That variable surely
_cannot_ have a meaningful global value - or can it?

--
Alan Mackenzie (Munich, Germany)

Re: Fight: font-lock.el 1.289 vs. cc-defs.el

[Stefan Monnier]

> CC Mode calls (font-lock-compile-keywords "\\<\\>") at initialisation, to
> test for a bug in middle aged XEmacsen which splatted font-lock-keywords.
> The new check in V1.289 causes CC Mode to bail out with an error.

I must admit I have very little sympathy for ugly code that tries to work
around old bugs that've been fixed a long time ago.

> Andreas Schwab patched a work-around into cc-defs.el V1.38, placing a
> condition-case around the call.  This seems suboptimal: If some error
> occurs whilst calling some Font Lock function, we definitely want to know
> about it.

> A better(??) kludge would be for cc-defs to bind font-lock-set-defaults
> to non-nil before calling (font-lock-compile-keywords "\\<\\>"), but this
> is really ghastly coding practise.

So a better/real fix would be to remove this workaround altogether and if
someone complains, tell him to fix it himself or upgrade his old XEmacs or
stick to an old cc-mode.  Plenty of choice left.

> I'm asking that this change to font-lock.el be reconsidered, though I
> admit I don't know the problem it fixes.

It help(s|ed) detect faulty code, more precisely code that used font-lock
code without first setting up font-lock variables.  I guess with the recent
changes that force font-lock-fontify-buffer and font-lock-fontify-region to
setup the variables, this is not needed any more, since those were the
main culprits.

> Might a better solution be (make-variable-buffer-local
> 'font-lock-keywords)?

That would only hide the problem.  Calling font-lock-set-defaults does a lot
more than make this variable local to a buffer.

> That variable surely _cannot_ have a meaningful global value - or can it?

Indeed.


        Stefan

Re: Fight: font-lock.el 1.289 vs. cc-defs.el

[Alan Mackenzie]

On Sun, 29 Jan 2006, Stefan Monnier wrote:

>> CC Mode calls (font-lock-compile-keywords "\\<\\>") at initialisation,
>> to test for a bug in middle aged XEmacsen which splatted
>> font-lock-keywords.  The new check in V1.289 causes CC Mode to bail
>> out with an error.

>I must admit I have very little sympathy for ugly code that tries to
>work around old bugs that've been fixed a long time ago.

The alternative is CC Mode not working properly (or at all) in some
versions.  For example, but for a similar test and correction, it would
fail in Emacs 21.[1234] because of a bug in regexp-opt-depth.  These old
bugs in the XEmacs and Emacs cores have only been fixed in recent
versions, not in all the versions currently in the field.  It's a core
objective of CC Mode to be compatible with all recent versions of both
Emacsen.

>> Andreas Schwab patched a work-around into cc-defs.el V1.38, placing a
>> condition-case around the call.  This seems suboptimal: If some error
>> occurs whilst calling some Font Lock function, we definitely want to
>> know about it.

>> A better(??) kludge would be for cc-defs to bind font-lock-set-defaults
>> to non-nil before calling (font-lock-compile-keywords "\\<\\>"), but this
>> is really ghastly coding practise.

>So a better/real fix would be to remove this workaround altogether and
>if someone complains, tell him to fix it himself or upgrade his old
>XEmacs or stick to an old cc-mode.  Plenty of choice left.

There is indeed!  There's vim, Eclipse, CodeWrite, MS Visual Studio, ....

Actually, thinking about it, it would be better to test explicitly for
XEmacs in this case.  That's what I'll do.

>> I'm asking that this change to font-lock.el be reconsidered, though I
>> admit I don't know the problem it fixes.

>It help(s|ed) detect faulty code, more precisely code that used
>font-lock code without first setting up font-lock variables.  I guess
>with the recent changes that force font-lock-fontify-buffer and
>font-lock-fontify-region to setup the variables, this is not needed any
>more, since those were the main culprits.

OK.  Presumably that code has since been corrected.

But as a general point, CC Mode (and presumably other packages) is going
to be calling random functions within the (X)Emacs core to detect the
inevitable buggy versions.  There is a danger that other packages also
test for this bug in font-lock-compile-keywords, causing these packages
to fail.

>> Might a better solution be (make-variable-buffer-local
>> 'font-lock-keywords)?

>That would only hide the problem.  Calling font-lock-set-defaults does a lot
>more than make this variable local to a buffer.

>> That variable surely _cannot_ have a meaningful global value - or can
>> it?

>Indeed.

>        Stefan

-- 
Alan.

Re: Fight: font-lock.el 1.289 vs. cc-defs.el

[Richard M. Stallman]

    I'm asking that this change to font-lock.el be reconsidered, though I
    admit I don't know the problem it fixes.

It detects a kind of problem that was hard to debug, when it occurred.
I decided we needed to make such problems show themselves.  I don't
remember the details, but there was probably another change around the
same time which fixed the specific problem.

    CC Mode calls (font-lock-compile-keywords "\\<\\>") at initialisation, to
    test for a bug in middle aged XEmacsen which splatted font-lock-keywords.
    The new check in V1.289 causes CC Mode to bail out with an error.

I suggest you test that condition only on XEmacs.

Re: Fight: font-lock.el 1.289 vs. cc-defs.el

[Alan Mackenzie]

On Mon, 30 Jan 2006, Richard M. Stallman wrote:

>    I'm asking that this change to font-lock.el be reconsidered, though I
>    admit I don't know the problem it fixes.
>
>It detects a kind of problem that was hard to debug, when it occurred.
>I decided we needed to make such problems show themselves.  I don't
>remember the details, but there was probably another change around the
>same time which fixed the specific problem.
>
>    CC Mode calls (font-lock-compile-keywords "\\<\\>") at initialisation, to
>    test for a bug in middle aged XEmacsen which splatted font-lock-keywords.
>    The new check in V1.289 causes CC Mode to bail out with an error.
>
>I suggest you test that condition only on XEmacs.

Will do!

As a matter of interest, to anybody who's interested, (featurep 'xemacs)
is a good way of distinguishing the two Emacsen.

-- 
Alan.