Re: master 5c70ff9: New user option 'font-lock-ignore'

[Augusto Stoffel]

Hi Eli,

thanks for your feedback.  I've responded to your points below.

On Sat,  2 Apr 2022 at 09:36, Eli Zaretskii <eliz@gnu.org> wrote:

> Thanks for developing this feature.
>
> Reading its documentation, I think it should be clarified to be more
> useful.  See the questions/comments below:
>
>> +@example
>> +(@var{mode} @var{rule} @dots{})
>> +@end example
>> +
>> +Here, @var{mode} is a symbol, say a major or minor mode.
>
> Why "say"?  Can it usefully be something else? if so, what can it be?

This is easiest to explain with code.  A rule for a given xyz-mode
applies if

    (or (bound-and-true-p xyz-mode)
        (derived-mode-p xyz-mode))

Do you think it's better to just say “Here, @var{mode} a major or minor
mode”?  This would be slightly imprecise, because technically 'mode' can
be any variable.

>>                                                            The
>> +subsequent rules apply if the current major mode is derived from
>> +@var{mode} or @var{mode} is bound and true as a variable.  Each
>> +@var{rule} can be one of the following:
>> +
>> +@table @code
>> +@cindex @var{font-lock-ignore} rules
>> +@item @var{symbol}
>> +A symbol, say a face name, matches any Font Lock keyword containing
>> +the symbol in its definition.
>
> I don't think I understand how can a keyword "contain" a symbol.
> Please elaborate or suggest a different text that clarifies this.

Okay, as a general remark, this feature has been bolted on the existing
font lock mechanism, and it shows.  So the user has to interact to some
degree with the font-lock internals to make any meaningful use of it.

Specifically regarding your question, the user has to be aware that
'font-lock-keywords' is a list where (quoting the docstring):

    Each element [...] should have one of these forms:

     MATCHER
     (MATCHER . SUBEXP)
     (MATCHER . FACENAME)
     (MATCHER . HIGHLIGHT)
     (MATCHER HIGHLIGHT ...)
     (eval . FORM)

    where MATCHER can be either the regexp to search for, or the
    function name to call to make the search (called with one

So here I mean that 'symbol' equals the MATCHER, or the FACENAME, or is
a member of (flatten-tree FORM), and so on.

Unfortunately, I can't think of a better way to explain this.  Or, at
least, not in an annoyingly long-winded way.

(Coming back to the "bolted on" remark: If this feature had been planned
from the beginning, perhaps one would have forced the font-lock keyword
definitions to be named and/or include other metadata like "importance"
or "precedence".  But that is not the case, and the filtering options
provided here are IMO the best one can do at this point.)

>> The symbol is interpreted as a glob
>> +pattern; in particular, @code{*} matches everything.
>
> Do you mean shell glob patterns?  AFAIK, we don't use them in Emacs
> except for shell wildcards, so why are they used here? why not
> regexps?  And in any case, those patterns need a thorough description,
> since we don't use them elsewhere.  For example, the way to quote
> meta-characters such as '*' must be documented, because it isn't
> self-evident.

The intention is that the user can write, for instance

    (setq font-lock-ignore '((prog-mode font-lock-*-face)))

and this will be equivalent to

    (setq font-lock-ignore '((prog-mode (pred (lambda (obj)
                                                 (and (symbolp obj)
                                                      (string-match-p
                                                       "\\`font-lock-.*-face\\'"
                                                       (symbol-name obj))))))))

Do you agree this is a sensible and convenient mini-language?  I guess
Stefan liked the idea, at least :-).

>> +@item @var{string}
>> +A string matches any font-lock keyword defined by a regexp that
>> +matches the string.
>
> I don't think I understand this sentence.  In particular, saying "a
> regexp that matches the string" is at least unusual: we usually say it
> the other way around: "a string that matches the regexp".  And what
> does it mean for a keyword to be "defined by a regexp"?

Just to make sure we're on the same page, this refers, for instance, to
the rule

    (emacs-lisp-mode ";;;###autoload")

which would deactivate the highlighting of autoload cookies.

So, we could say "A string matches any font-lock rule which would
highlight that string".  It would be a slight lie, because this doesn't
work if the MATCHER is a function.  Should we trade precision for
clarity here?  I think we probably should.

>> +In each buffer, Font Lock keywords that match at least one applicable
>> +rule are disabled.
>
> This should be _before_ the list of rules, not after.  And given the
> fact that actions of the rules are implicit (i.e. they aren't present
> in the rules themselves), I would suggest to call them "conditions",
> not "rules".  A rule includes the action, not just the condition for
> that action, and those "rules" don't.

Okay, I'll move the text.  About rule vs. condition, let me think about
it.

>> +@smallexample
>> +(setq font-lock-ignore
>> +      '((prog-mode font-lock-*-face
>> +                   (except help-echo))
>> +        (emacs-lisp-mode (except ";;;###autoload)")
>> +        (whitespace-mode whitespace-empty-at-bob-regexp)
>> +        (makefile-mode (except *))))
>> +@end smallexample
>> +
>> +Line by line, this does the following:
>> +
>> +@enumerate
>> +@item
>> +In all programming modes, disable all font-lock keywords that apply
>> +one of the standard font-lock faces (excluding strings and comments,
>> +which are covered by syntactic Font Lock).
>> +
>> +@item
>> +However, keep any keywords that add a @code{help-echo} text property.
>> +
>> +@item
>> +In Emacs Lisp mode, also keep the highlighting of autoload cookies,
>> +which would have been excluded by rule 1.
>> +
>> +@item
>> +In @code{whitespace-mode} (a minor mode), don't highlight an empty
>> +line at beginning of buffer.
>
> Shouldn't you have "also" in this last description, like you did for
> Emacs Lisp mode?

Yes, or maybe "Additionally, if whitespace-mode (a minor-mode) is active,
don't highlight and empty line at the beginning of buffer."

> And why doesn't the example show all the possible forms of a "rule",
> but just one of them?

Well, I just wanted to keep true to this being a "@smallexample".  I
think it will be exceedingly rare that anyone needs any of the other
rules (and, pred, not, etc.).  It's so easy to implement those things
that it doesn't make sense not to, but mentioning them in the example
would perhaps distract from the main point.

>> +*** New user option 'font-lock-ignore'.
>> +This variable provides a mechanism to selectively disable font-lock
>> +keywords.
>
> I don't think it disables keywords, I think it disables
> fontifications.  (How can one disable a keyword?)

Hum, I'm not sure I understand the fine-grained distinction your are
making here.  font-lock.el speaks of "font lock keywords"; the thing
that fontifies variable names with font-lock-variable-face would be a
"font lock keyword" in that terminology.  I'm just following along the
terminology.

>> +
>> ++++
>>  *** New package vtable.el for formatting tabular data.
>>  This package allows formatting data using variable-pitch fonts.
>>  The resulting tables can display text in variable pitch fonts, text
>> diff --git a/lisp/font-lock.el b/lisp/font-lock.el
>> index d8a1fe3..8af3c30 100644
>> --- a/lisp/font-lock.el
>> +++ b/lisp/font-lock.el
>> @@ -208,6 +208,7 @@
>>  
>>  (require 'syntax)
>>  (eval-when-compile (require 'cl-lib))
>> +(eval-when-compile (require 'subr-x))
>>  
>>  ;; Define core `font-lock' group.
>>  (defgroup font-lock '((jit-lock custom-group))
>> @@ -279,6 +280,42 @@ font-lock-maximum-decoration
>>                                    (integer :tag "level" 1)))))
>>    :group 'font-lock)
>>  
>> +(defcustom font-lock-ignore nil
>> +  "Rules to selectively disable font-lock keywords.
>> +This is a list of rule sets of the form
>> +
>> +  (MODE RULE ...)
>> +
>> +where:
>> +
>> + - MODE is a symbol, say a major or minor mode.  The subsequent
>> +   rules apply if the current major mode is derived from MODE or
>> +   MODE is bound and true as a variable.
>> +
>> + - Each RULE can be one of the following:
>> +   - A symbol, say a face name.  It matches any font-lock keyword
>> +     containing the symbol in its definition.  The symbol is
>> +     interpreted as a glob pattern; in particular, `*' matches
>> +     everything.
>> +   - A string.  It matches any font-lock keyword defined by a regexp
>> +     that matches the string.
>> +   - A form (pred FUNCTION).  It matches if FUNCTION, which is called
>> +     with the font-lock keyword as argument, returns non-nil.
>> +   - A form (not RULE).  It matches if RULE doesn't.
>> +   - A form (and RULE ...).  It matches if all the provided rules
>> +     match.
>> +   - A form (or RULE ...).  It matches if any of the provided rules
>> +     match.
>> +   - A form (except RULE ...).  This can be used only at top level or
>> +     inside an `or' clause.  It undoes the effect of a previous
>> +     matching rule.
>> +
>> +In each buffer, font lock keywords that match at least one
>> +applicable rule are disabled."
>
> Same comments apply to this doc string.

Sure.

>> +  :type '(alist :key-type symbol :value-type sexp)
>> +  :group 'font-lock
>> +  :version "29.1")
>> +
>>  (defcustom font-lock-verbose nil
>>    "If non-nil, means show status messages for buffer fontification.
>>  If a number, only buffers greater than this size have fontification 
>> messages."
>> @@ -1810,9 +1847,8 @@ font-lock-compile-keywords
>>        (error "Font-lock trying to use keywords before setting them up"))
>>    (if (eq (car-safe keywords) t)
>>        keywords
>> -    (setq keywords
>> -      (cons t (cons keywords
>> -                    (mapcar #'font-lock-compile-keyword keywords))))
>> +    (let ((compiled (mapcar #'font-lock-compile-keyword keywords)))
>> +      (setq keywords `(t ,keywords ,@(font-lock--filter-keywords 
>> compiled))))
>>      (if (and (not syntactic-keywords)
>>           (let ((beg-function (with-no-warnings syntax-begin-function)))
>>             (or (eq beg-function #'beginning-of-defun)
>> @@ -1883,6 +1919,50 @@ font-lock-choose-keywords
>>      (t
>>       (car keywords))))
>>  
>> +(defun font-lock--match-keyword (rule keyword)
>> +  "Return non-nil if font-lock KEYWORD matches RULE.
>> +See `font-lock-ignore' for the possible rules."
>> +  (pcase-exhaustive rule
>> +    ('* t)
>> +    ((pred symbolp)
>> +     (let ((regexp (when (string-match-p "[*?]" (symbol-name rule))
>> +                     (wildcard-to-regexp (symbol-name rule)))))
>
> So "[abcd]" isn't supported by these "glob patterns"?  This should be
> documented.

Ah, okay, I guess we should use the regexp "[][*?]" in that test.

> I'm okay with clarifying the docs myself if you explain enough for me
> to understand how to do that.

I guess yes, it would be great if we could have the perspective of a
different person incorporated in the documentation, so please go ahead!
I'll be glad to provide further clarifications.

> TIA