On Tue,  8 Mar 2022 at 10:18, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

>>> Nice.  I'd suggest you merge `font-lock-ignore--test` and
>>> `font-lock-ignore--test-one` into a single (recursive) function.
>> The exclamation marks are not a recursive thing.
>> But I added `and' and `not' rules that use recursion.
>
> Ah, I see I had misunderstood the syntax and semantics of your toplevel,
> so indeed to bring it into the recursion, you'd need a new thingy which
> might be called maybe `or` with a syntax like (or RULES .. [!] RULE ..).

It's the semantics of a gitignore file.  It think it's handy.

Now, in the syntax you suggest,

  (or RULE1 ! RULE2 RULE3)

means

  (or (and RULE1 (not RULE2)) RULE3)

Maybe this is a potentially confusing extension of such a familiar
construct like `or'?  This is why I originally allowed ! at top level
only (and didn't provide a recursive "or" operation, which seems a bit
of overkill).

But then I implemented what you suggest to see how it looks like.  I'll
let you emit an opinion.

> See some nitpicks below.
>
>
>         Stefan
>
>
>> +   - A symbol, say a face name.  It matches any font-lock rule
>> +     mentioning that symbol anywhere.  Asterisks are treated as
>> +     wildcards.
>
> I suggest we call it a glob pattern so it's a known pattern
> matching thingy and we don't need to reinvent and re-document how to
> make it match an actual `*`.

Great, that sounds better.  But just to make sure, do you like the basic
API here:

- A symbol (possibly wildcard characters) defines a rule that matches
  symbols in the font-lock keyword

- A string defines a rule that matches any font-lock keyword which
  matches the said string

>> @@ -1810,9 +1842,11 @@ 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 ,@(if (and font-lock-ignore
>> +                                              (not syntactic-keywords))
>> +                                         (font-lock--filter-keywords compiled)
>> +                                       compiled))))
>
> I think I'd move the test of `font-lock-ignore` into
> `font-lock--filter-keywords` so it's the only function which consults it.

All right.  I've also made the ignore rules apply to syntactic-keywords
as well, which I believe is harmless -- right?

>> @@ -1883,6 +1917,56 @@ font-lock-choose-keywords
>>  	(t
>>  	 (car keywords))))
>>  
>> +(defun font-lock--test-keyword (rule keyword)
>
> That sadly doesn't say whether it return nil when it matches or whether
> it returns non-nil when it matches.  I suggest to rename it to something
> like `font-lock--matches-keyword` so the name clearly say when we return
> nil and when we return non-nil.

Done.

>> +  "Test whether font-lock KEYWORD matches a RULE.
>> +See `font-lock-ignore' for the possible rules."
>
> Same comment ;-)

Done.  The docstring for `font-lock-ignore' can still use some
polishing.

>> +  (pcase-exhaustive rule
>> +    ('* t)
>> +    ((pred symbolp)
>> +     (let* ((name (symbol-name rule))
>> +            (regexp (when (string-match-p "\\*" name)
>> +                      (let* ((words (mapcar #'regexp-quote
>> +                                            (split-string name "\\*")))
>> +                             (joined (string-join words ".*")))
>> +                      (concat "\\`" joined "\\'")))))
>
> We can use `wildcard-to-regexp` here.

Ah, much better!  I swear I did M-x apropos \bglob\b.

>> +       (if regexp
>> +           (seq-some (lambda (obj)
>> +                       (when (symbolp obj)
>> +                         (string-match-p regexp (symbol-name obj))))
>> +                     (flatten-tree keyword))
>> +         (memq rule (flatten-tree keyword)))))
>
> Performance likely doesn't matter, but I suspect it'd be faster if we
> recursed over the data-structure rather than flattening it.
> E.g. something like
>
>     (named-let search ((obj keyword))
>       (cond
>        ((consp obj) (or (search (car obj)) (search (cdr obj))))
>        ((and obj (symbolp obj))
>         (string-match-p regexp (symbol-name obj)))))

Aesthetically, calling `flatten-tree' there bothers me too. But your
algorithm uses recursion even for a flat list...  And a `find-in-tree'
function is not available even in dash, so I guess adding one to subr.el
is out of the question.  (Also I'm already doing a bunch of extra work
by calling `wildcard-to-regexp' on each use of a rule, so it seems weird
to optimize this without also adding a compiler for the rules, which is
way over the top.)