hi-lock and overlays (was: Conflicting overlays and the 'priority property)

[Tassilo Horn <tassilo@member.fsf.org>]

Hi all,

I was wrong with my previous mail.  The problem that hi-lock pattern on
the current line are not highlighted, when `hl-line-mode' is enabled is
not due to conflicting overlays.  The problem is that hi-lock uses
overlays only when the undocumented variable `font-lock-fontified' is
nil (see `hi-lock-set-pattern').  Else, it uses normal fontification,
and overlays (like the one for the current line created by
`hl-line-mode') take precedence over that.

So now I have some questions:

  - Why does hi-lock use two different styles of highlighting?  IMO, if
    a user does M-x highlight-regexp RET foo RET hi-yellow RET, he
    expects to get that highlighting even if there are other overlays,
    so creating an overlay (and maybe even setting a priority) is the
    only safe bet.

  - Why is the call to `font-lock-add-keywords' done unconditionally,
    although it's possible that overlays might be used.  Isn't that
    superfluous in the overlay case?

I've changed the definition of `hi-lock-set-pattern' like this, which
looks more correct to me.

--8<---------------cut here---------------start------------->8---
diff -u /home/horn/repos/el/emacs/trunk/lisp/hi-lock.el /tmp/buffer-content-6420a_A
--- /home/horn/repos/el/emacs/trunk/lisp/hi-lock.el	2010-01-17 19:26:05.248012589 +0100
+++ /tmp/buffer-content-6420a_A	2010-01-18 22:24:26.839291297 +0100
@@ -568,19 +568,24 @@
   "Inhibit the action of `hi-lock-font-lock-hook'.
 This is used by `hi-lock-set-pattern'.")
 
-(defun hi-lock-set-pattern (regexp face)
-  "Highlight REGEXP with face FACE."
-  (let ((pattern (list regexp (list 0 (list 'quote face) t)))
-	;; The call to `font-lock-add-keywords' below might disable
-	;; and re-enable font-lock mode.  If so, we don't want
-	;; `hi-lock-font-lock-hook' to run.  This can be removed once
-	;; Bug#635 is fixed. -- cyd
-	(hi-lock--inhibit-font-lock-hook t))
+(defun hi-lock-set-pattern (regexp face &optional permit-font-lock)
+  "Highlight REGEXP with face FACE.
+If PERMIT-FONT-LOCK is non-nil, allow normal font-lock
+fontification instead of creating overlays.  In that case,
+matches of REGEXP won't be highlighted, if they occur inside an
+overlay, because overlays take precedence over normal
+fontification."
+  (let ((pattern (list regexp (list 0 (list 'quote face) t))))
     (unless (member pattern hi-lock-interactive-patterns)
-      (font-lock-add-keywords nil (list pattern) t)
       (push pattern hi-lock-interactive-patterns)
-      (if font-lock-fontified
-          (font-lock-fontify-buffer)
+      (if (and font-lock-fontified permit-font-lock)
+          (let (;; The call to `font-lock-add-keywords' below might disable
+		;; and re-enable font-lock mode.  If so, we don't want
+		;; `hi-lock-font-lock-hook' to run.  This can be removed once
+		;; Bug#635 is fixed. -- cyd
+		(hi-lock--inhibit-font-lock-hook t))
+	    (font-lock-add-keywords nil (list pattern) t)
+	    (font-lock-fontify-buffer))
         (let* ((serial (hi-lock-string-serialize regexp))
                (range-min (- (point) (/ hi-lock-highlight-range 2)))
                (range-max (+ (point) (/ hi-lock-highlight-range 2)))
--8<---------------cut here---------------end--------------->8---

I've tested the function with PERMIT-FONT-LOCK set to t, and then it
acts exactly as it does right now, e.g. it highlights every occurence of
REGEXP that is not inside an overlay.

When called with PERMIT-FONT-LOCK set to nil (or omitted), it always
creates overlays and the highlighting is done even if the REGEXP matches
inside other overlays.

Do you think my change is reasonable and should be committed?  Or is
there a rationale for the current way that I didn't grasp till now?

Bye,
Tassilo