bug#39597: 27.0.60: M-x occur adds fontification to fundamental-mode

bug#39597: 27.0.60: M-x occur adds fontification to fundamental-mode

[Kévin Le Gouguec]

Hello,

I think commit fb16313025 (2018-12-18T23:10:09Z!juri@linkov.net) had an
unintended side-effect on fundamental-mode.  From emacs -Q:

- C-x b repro RET
- foo RET "bar" RET baz
- M-s o . RET

In emacs 26.3, the Occur buffer pops up and not much else happens;
starting with emacs 27, font-lock-string-face is applied to "bar".

I haven't dug much more than that; FWIW however, emacs 26.3 already
applied font-lock-string-face to double-quoted strings when running
(font-lock-ensure) in a fundamental buffer, so maybe the right fix
belongs e.g. somewhere in font-lock-set-defaults?


Thank you for your time.


In GNU Emacs 28.0.50 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.24.5, cairo version 1.16.0)
 of 2020-01-31 built on hirondell
Repository revision: d3ead375092e2690c1d1d6a5dd82e6e89cdf4f4c
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12004000
System Description: Debian GNU/Linux 10 (buster)

Configured using:
 'configure --with-xwidgets --with-cairo'

Configured features:
XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND GPM DBUS GSETTINGS GLIB NOTIFY
INOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF
ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES THREADS XWIDGETS
LIBSYSTEMD JSON PDUMPER LCMS2 GMP

bug#39597: 27.0.60: M-x occur adds fontification to fundamental-mode

[Juri Linkov]

> I think commit fb16313025 (2018-12-18T23:10:09Z!juri@linkov.net) had an
> unintended side-effect on fundamental-mode.  From emacs -Q:

This commit contains this change:

diff --git a/lisp/replace.el b/lisp/replace.el
index dcae12e9b7..b8f231eb55 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -1907,10 +1907,8 @@ occur-engine
       global-matches)))
 
 (defun occur-engine-line (beg end &optional keep-props)
-  (if (and keep-props (if (boundp 'jit-lock-mode) jit-lock-mode)
-	   (text-property-not-all beg end 'fontified t))
-      (if (fboundp 'jit-lock-fontify-now)
-	  (jit-lock-fontify-now beg end)))
+  (if (and keep-props font-lock-mode)
+      (font-lock-ensure beg end))
   (if (and keep-props (not (eq occur-excluded-properties t)))
       (let ((str (buffer-substring beg end)))
 	(remove-list-of-text-properties

> - C-x b repro RET
> - foo RET "bar" RET baz
> - M-s o . RET
>
> In emacs 26.3, the Occur buffer pops up and not much else happens;
> starting with emacs 27, font-lock-string-face is applied to "bar".
>
> I haven't dug much more than that; FWIW however, emacs 26.3 already
> applied font-lock-string-face to double-quoted strings when running
> (font-lock-ensure) in a fundamental buffer, so maybe the right fix
> belongs e.g. somewhere in font-lock-set-defaults?

Stefan, could you suggest what to do with font-lock-ensure
to not highlight double-quoted strings in fundamental-mode?

bug#39597: 27.0.60: M-x occur adds fontification to fundamental-mode

[Stefan Monnier]

Juri Linkov [2020-02-18 02:12:32] wrote:

>> I think commit fb16313025 (2018-12-18T23:10:09Z!juri@linkov.net) had an
>> unintended side-effect on fundamental-mode.  From emacs -Q:
>
> This commit contains this change:
>
> diff --git a/lisp/replace.el b/lisp/replace.el
> index dcae12e9b7..b8f231eb55 100644
> --- a/lisp/replace.el
> +++ b/lisp/replace.el
> @@ -1907,10 +1907,8 @@ occur-engine
>        global-matches)))
>  
>  (defun occur-engine-line (beg end &optional keep-props)
> -  (if (and keep-props (if (boundp 'jit-lock-mode) jit-lock-mode)
> -	   (text-property-not-all beg end 'fontified t))
> -      (if (fboundp 'jit-lock-fontify-now)
> -	  (jit-lock-fontify-now beg end)))
> +  (if (and keep-props font-lock-mode)
> +      (font-lock-ensure beg end))
>    (if (and keep-props (not (eq occur-excluded-properties t)))
>        (let ((str (buffer-substring beg end)))
>  	(remove-list-of-text-properties
>
>> - C-x b repro RET
>> - foo RET "bar" RET baz
>> - M-s o . RET
>>
>> In emacs 26.3, the Occur buffer pops up and not much else happens;
>> starting with emacs 27, font-lock-string-face is applied to "bar".
>>
>> I haven't dug much more than that; FWIW however, emacs 26.3 already
>> applied font-lock-string-face to double-quoted strings when running
>> (font-lock-ensure) in a fundamental buffer, so maybe the right fix
>> belongs e.g. somewhere in font-lock-set-defaults?
>
> Stefan, could you suggest what to do with font-lock-ensure
> to not highlight double-quoted strings in fundamental-mode?

Maybe something like the patch below?
I'm not sufficiently familiar with the design of `font-lock-specified-p`
to be completely sure where the check should be placed; this is my best guess.


        Stefan


diff --git a/lisp/font-lock.el b/lisp/font-lock.el
index 438511b647..bf045d9d7e 100644
--- a/lisp/font-lock.el
+++ b/lisp/font-lock.el
@@ -1139,9 +1139,10 @@ font-lock-ensure
   "Make sure the region BEG...END has been fontified.
 If the region is not specified, it defaults to the entire accessible
 portion of the buffer."
-  (font-lock-set-defaults)
-  (funcall font-lock-ensure-function
-           (or beg (point-min)) (or end (point-max))))
+  (when (font-lock-specified-p t)
+    (font-lock-set-defaults)
+    (funcall font-lock-ensure-function
+             (or beg (point-min)) (or end (point-max)))))
 
 (defun font-lock-default-fontify-buffer ()
   "Fontify the whole buffer using `font-lock-fontify-region-function'."

bug#39597: 27.0.60: M-x occur adds fontification to fundamental-mode

[Juri Linkov]

> Maybe something like the patch below?
> I'm not sufficiently familiar with the design of `font-lock-specified-p`
> to be completely sure where the check should be placed; this is my best guess.

> diff --git a/lisp/font-lock.el b/lisp/font-lock.el
> index 438511b647..bf045d9d7e 100644
> --- a/lisp/font-lock.el
> +++ b/lisp/font-lock.el
> @@ -1139,9 +1139,10 @@ font-lock-ensure
>    "Make sure the region BEG...END has been fontified.
>  If the region is not specified, it defaults to the entire accessible
>  portion of the buffer."
> -  (font-lock-set-defaults)
> -  (funcall font-lock-ensure-function
> -           (or beg (point-min)) (or end (point-max))))
> +  (when (font-lock-specified-p t)
> +    (font-lock-set-defaults)
> +    (funcall font-lock-ensure-function
> +             (or beg (point-min)) (or end (point-max)))))
>  
>  (defun font-lock-default-fontify-buffer ()
>    "Fontify the whole buffer using `font-lock-fontify-region-function'."

I confirm this fixes the problem reported by Kévin,
so maybe this should be installed to emacs-27.

bug#39597: 27.0.60: M-x occur adds fontification to fundamental-mode

[Kévin Le Gouguec]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Maybe something like the patch below?

I can confirm that this fixes the issue.

> I'm not sufficiently familiar with the design of `font-lock-specified-p`
> to be completely sure where the check should be placed; this is my best guess.

I tried to read as much as I could about font-lock in the Emacs and
Elisp manuals, but I'm still overwhelmed by all the variables and
functions involved, e.g.

- font-lock-mode
- font-lock-specified-p
- font-lock-ensure
- font-lock-set-defaults
- font-lock-fontified
- font-lock-defaults
- font-lock-major-mode

M-? and M-. make it trivial to say which function sets which variables,
which predicate depends on which variables, which variable influences
which functions… but I can't figure out what the overall design is,
i.e. which symbols are font-lock plumbing, which ones are meant to be
used by major modes, which ones should be used by minor modes…

IOW I have no idea where the check should be place either :/

bug#39597: 27.0.60: M-x occur adds fontification to fundamental-mode

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Wed, 19 Feb 2020 00:49:12 +0200
> Cc: 39597@debbugs.gnu.org,
>  Kévin Le Gouguec <kevin.legouguec@gmail.com>
> 
> > diff --git a/lisp/font-lock.el b/lisp/font-lock.el
> > index 438511b647..bf045d9d7e 100644
> > --- a/lisp/font-lock.el
> > +++ b/lisp/font-lock.el
> > @@ -1139,9 +1139,10 @@ font-lock-ensure
> >    "Make sure the region BEG...END has been fontified.
> >  If the region is not specified, it defaults to the entire accessible
> >  portion of the buffer."
> > -  (font-lock-set-defaults)
> > -  (funcall font-lock-ensure-function
> > -           (or beg (point-min)) (or end (point-max))))
> > +  (when (font-lock-specified-p t)
> > +    (font-lock-set-defaults)
> > +    (funcall font-lock-ensure-function
> > +             (or beg (point-min)) (or end (point-max)))))
> >  
> >  (defun font-lock-default-fontify-buffer ()
> >    "Fontify the whole buffer using `font-lock-fontify-region-function'."
> 
> I confirm this fixes the problem reported by Kévin,
> so maybe this should be installed to emacs-27.

Doesn't sound serious enough to do this on the release branch.  We've
had this issue for quite some time, no?

bug#39597: 27.0.60: M-x occur adds fontification to fundamental-mode

[Kévin Le Gouguec]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Juri Linkov <juri@linkov.net>
>> Date: Wed, 19 Feb 2020 00:49:12 +0200
>> Cc: 39597@debbugs.gnu.org,
>> 
>> I confirm this fixes the problem reported by Kévin,
>> so maybe this should be installed to emacs-27.
>
> Doesn't sound serious enough to do this on the release branch.  We've
> had this issue for quite some time, no?

Unless I'm getting mixed up between the bug reports I've made recently,
this issue has actually been introduced in emacs 27.  Quoting my initial
message (and hoping I'm not confused):

> In emacs 26.3, the Occur buffer pops up and not much else happens;
> starting with emacs 27, font-lock-string-face is applied to "bar".

bug#39597: 27.0.60: M-x occur adds fontification to fundamental-mode

[Eli Zaretskii]

> From: Kévin Le Gouguec <kevin.legouguec@gmail.com>
> Cc: Juri Linkov <juri@linkov.net>,  monnier@iro.umontreal.ca,
>   39597@debbugs.gnu.org
> Date: Thu, 20 Feb 2020 20:10:25 +0100
> 
> > Doesn't sound serious enough to do this on the release branch.  We've
> > had this issue for quite some time, no?
> 
> Unless I'm getting mixed up between the bug reports I've made recently,
> this issue has actually been introduced in emacs 27.  Quoting my initial
> message (and hoping I'm not confused):
> 
> > In emacs 26.3, the Occur buffer pops up and not much else happens;
> > starting with emacs 27, font-lock-string-face is applied to "bar".

Can you show what commit introduced that?

Thanks.

bug#39597: 27.0.60: M-x occur adds fontification to fundamental-mode

[Kévin Le Gouguec]

Eli Zaretskii <eliz@gnu.org> writes:

> Can you show what commit introduced that?

Yup; quoting Juri above:

>> I think commit fb16313025 (2018-12-18T23:10:09Z!juri <at> linkov.net) had an
>> unintended side-effect on fundamental-mode.  From emacs -Q:
> 
> This commit contains this change:
> 
> diff --git a/lisp/replace.el b/lisp/replace.el
> index dcae12e9b7..b8f231eb55 100644
> --- a/lisp/replace.el
> +++ b/lisp/replace.el
> @@ -1907,10 +1907,8 @@ occur-engine
>        global-matches)))
>  
>  (defun occur-engine-line (beg end &optional keep-props)
> -  (if (and keep-props (if (boundp 'jit-lock-mode) jit-lock-mode)
> -	   (text-property-not-all beg end 'fontified t))
> -      (if (fboundp 'jit-lock-fontify-now)
> -	  (jit-lock-fontify-now beg end)))
> +  (if (and keep-props font-lock-mode)
> +      (font-lock-ensure beg end))
>    (if (and keep-props (not (eq occur-excluded-properties t)))
>        (let ((str (buffer-substring beg end)))
>  	(remove-list-of-text-properties

$ git branch --remote --contains fb16313025 | grep origin/emacs-
  origin/emacs-27

bug#39597: 27.0.60: M-x occur adds fontification to fundamental-mode

[Juri Linkov]

>> I confirm this fixes the problem reported by Kévin,
>> so maybe this should be installed to emacs-27.
>
> Doesn't sound serious enough to do this on the release branch.  We've
> had this issue for quite some time, no?

This bug is a regression introduced in Emacs 27.

bug#39597: 27.0.60: M-x occur adds fontification to fundamental-mode

[Eli Zaretskii]

> From: Kévin Le Gouguec <kevin.legouguec@gmail.com>
> Date: Thu, 20 Feb 2020 23:27:41 +0100
> 
> >> I think commit fb16313025 (2018-12-18T23:10:09Z!juri <at> linkov.net) had an
> >> unintended side-effect on fundamental-mode.  From emacs -Q:
> > 
> > This commit contains this change:
> > 
> > diff --git a/lisp/replace.el b/lisp/replace.el
> > index dcae12e9b7..b8f231eb55 100644
> > --- a/lisp/replace.el
> > +++ b/lisp/replace.el
> > @@ -1907,10 +1907,8 @@ occur-engine
> >        global-matches)))
> >  
> >  (defun occur-engine-line (beg end &optional keep-props)
> > -  (if (and keep-props (if (boundp 'jit-lock-mode) jit-lock-mode)
> > -	   (text-property-not-all beg end 'fontified t))
> > -      (if (fboundp 'jit-lock-fontify-now)
> > -	  (jit-lock-fontify-now beg end)))
> > +  (if (and keep-props font-lock-mode)
> > +      (font-lock-ensure beg end))
> >    (if (and keep-props (not (eq occur-excluded-properties t)))
> >        (let ((str (buffer-substring beg end)))
> >  	(remove-list-of-text-properties

Thanks.

The fix installed on master is in a much more general place, and will
affect much more than just occur-engine.  I'm okay with a bug-fix
change on the release branch which is local to this place in
replace.el, but I don't want to risk breaking unrelated code by
changing font-lock.el.

Another alternative is to revert the offending changes in fb16313025
on the release branch.  Since it is an enhancement, perhaps we could
live without it in Emacs 27.

bug#39597: 27.0.60: M-x occur adds fontification to fundamental-mode

[Kévin Le Gouguec]

Eli Zaretskii <eliz@gnu.org> writes:

> The fix installed on master

(If you are referring to Stefan's patch, AFAICT it has not been
committed to master yet.)

>                             is in a much more general place, and will
> affect much more than just occur-engine.  I'm okay with a bug-fix
> change on the release branch which is local to this place in
> replace.el, but I don't want to risk breaking unrelated code by
> changing font-lock.el.

Idle musing: I wonder if something could be done inside
fundamental-mode.  It's not the only fontification issue I've noticed
there[1].

> Another alternative is to revert the offending changes in fb16313025
> on the release branch.  Since it is an enhancement, perhaps we could
> live without it in Emacs 27.

I don't have a preference about that.  Juri?


[1] E.g. (I was about to report this as a distinct issue… someday…)

    C-x b foo
    foo RET bar RET baz RET
    M-s h r a RET RET
    aaaa

    ⇒ In all modes *but* fundamental-mode, hi-lock-mode dynamically
    highlights new matches (e.g. the added "aaaa" string here).

    This bug is not strictly related to the current one, since the
    former has been with us since at least 26.3, but that does make me
    wonder whether both issues could be solved by turning some font-lock
    knob in fundamental-mode…

bug#39597: 27.0.60: M-x occur adds fontification to fundamental-mode

[Juri Linkov]

> Idle musing: I wonder if something could be done inside
> fundamental-mode.  It's not the only fontification issue I've noticed
> there[1].
>
> [1] E.g. (I was about to report this as a distinct issue… someday…)
>
>     C-x b foo
>     foo RET bar RET baz RET
>     M-s h r a RET RET
>     aaaa
>
>     ⇒ In all modes *but* fundamental-mode, hi-lock-mode dynamically
>     highlights new matches (e.g. the added "aaaa" string here).
>
>     This bug is not strictly related to the current one, since the
>     former has been with us since at least 26.3, but that does make me
>     wonder whether both issues could be solved by turning some font-lock
>     knob in fundamental-mode…

Searching for "font-lock-specified-p" in hi-lock.el
shows these fragments of code.  This first condition decides
whether to use font-lock for highlighting matches
that returns nil in fundamental-mode:

      (if (and font-lock-mode (font-lock-specified-p major-mode))

      ;; FIXME: Calling `font-lock-remove-keywords' causes
      ;; `font-lock-specified-p' to go from nil to non-nil (because it
      ;; calls font-lock-set-defaults).  This is yet-another bug in
      ;; font-lock-add/remove-keywords, which we circumvent here by
      ;; testing `font-lock-fontified' (bug#19796).

bug#39597: 27.0.60: M-x occur adds fontification to fundamental-mode

[Juri Linkov]

tags 39597 fixed
close 39597 28.0.50
quit

> The fix installed on master

Now the more fundamental fix is installed on master.

> Another alternative is to revert the offending changes in fb16313025
> on the release branch.  Since it is an enhancement, perhaps we could
> live without it in Emacs 27.

Yes, better to revert part of fb16313025 on the release branch
since that change was just an "optimization".
Now reverted on the release branch.

bug#39597: 27.0.60: M-x occur adds fontification to fundamental-mode

[Kévin Le Gouguec]

Juri Linkov <juri@linkov.net> writes:

>> [1] E.g. (I was about to report this as a distinct issue… someday…)
>>
>>     C-x b foo
>>     foo RET bar RET baz RET
>>     M-s h r a RET RET
>>     aaaa
>>
>>     ⇒ In all modes *but* fundamental-mode, hi-lock-mode dynamically
>>     highlights new matches (e.g. the added "aaaa" string here).
>>
>>     This bug is not strictly related to the current one, since the
>>     former has been with us since at least 26.3, but that does make me
>>     wonder whether both issues could be solved by turning some font-lock
>>     knob in fundamental-mode…
>
> Searching for "font-lock-specified-p" in hi-lock.el
> shows these fragments of code.  This first condition decides
> whether to use font-lock for highlighting matches
> that returns nil in fundamental-mode:
>
>       (if (and font-lock-mode (font-lock-specified-p major-mode))
>
>       ;; FIXME: Calling `font-lock-remove-keywords' causes
>       ;; `font-lock-specified-p' to go from nil to non-nil (because it
>       ;; calls font-lock-set-defaults).  This is yet-another bug in
>       ;; font-lock-add/remove-keywords, which we circumvent here by
>       ;; testing `font-lock-fontified' (bug#19796).

Thanks, I'll look into this SomeDay™ before I submit a bug report then.

And thank you for fixing the other bug on emacs-27 and master!