Subject: Re: bug#22541: 25.0.50; highlight-regexp from isearch has is case-sensitive even if case-fold is active

From: Juri Linkov <[email-masked]>
Date: Tue, 25 Apr 2017 23:52:06 +0300

> I updated the patch to make work `hi-lock-unface-buffer'.  I added tests
> as well.
>
> Note that in interactive calls the case fold is determined with the
> variables `search-upper-case' and `case-fold-search'.  This way it behaves
> as `isearch-forward-regexp'.
> Before this bug case fold was determined _just_ with `case-fold-search'.
> Do you prefer avoid `search-upper-case' in this case?

Since ‘search-upper-case’ is used by other commands such as ‘occur’ and
‘perform-replace’, I think ‘hi-lock’ should use it as well.

>>>From 7cad27c0fcc39add8679d0893010c4fdb3ed507a Mon Sep 17 00:00:00 2001
> From: Juri Linkov <[email-masked]>
> Date: Tue, 25 Apr 2017 14:17:23 +0900
> Subject: [PATCH] highlight-regexp: Honor case-fold-search
> ...
> Co-authored-by: Tino Calancha <[email-masked]>

I recommend to commit first my old patch, and then later your changes over it.

> -(defun hi-lock-face-buffer (regexp &optional face)
> +(defun hi-lock-face-buffer (regexp &optional face case-fold)
> ...
> +   (let* ((reg
> +           (hi-lock-regexp-okay
> +            (read-regexp "Regexp to highlight" 'regexp-history-last)))
> +          (face (hi-lock-read-face-name))
> +          (fold
> +           (if search-upper-case
> +               (isearch-no-upper-case-p reg t)
> +             case-fold-search)))
> +     (list reg face fold)))

Small thing, but for readability in the interactive spec better to use
the same variable names as argument names:

        (list regexp face case-fold)

> -(defun hi-lock-set-pattern (regexp face)
> -  "Highlight REGEXP with face FACE."
> +(defun hi-lock-set-pattern (regexp face &optional case-fold)
> +  "Highlight REGEXP with face FACE.
> +If optional arg CASE-FOLD is non-nil, then bind `case-fold-search' to it."
>    ;; Hashcons the regexp, so it can be passed to remove-overlays later.
>    (setq regexp (hi-lock--hashcons regexp))
> -  (let ((pattern (list regexp (list 0 (list 'quote face) 'prepend))))
> +  (let ((pattern (list (if (eq case-fold 'undefined)
> +                           regexp
> +                         (cons regexp
> +                               (byte-compile
> +                          `(lambda (limit)
> +                             (let ((case-fold-search ,case-fold))
> +                               (re-search-forward ,regexp limit t))))))
> +                       (list 0 (list 'quote face) 'prepend))))

Do you need to remember also the value of ‘case-fold-search’
(together with ‘regexp’)?

> @@ -1950,7 +1950,12 @@ isearch-highlight-regexp
>  			      (regexp-quote s))))
>  			isearch-string ""))
>  		      (t (regexp-quote isearch-string)))))
> -    (hi-lock-face-buffer regexp (hi-lock-read-face-name)))
> +    (hi-lock-face-buffer regexp (hi-lock-read-face-name)
> +                         (if (and (eq isearch-case-fold-search t)
> +                                  search-upper-case)
> +                             (isearch-no-upper-case-p
> +                              isearch-string isearch-regexp)
> +                           isearch-case-fold-search)))
>    (and isearch-recursive-edit (exit-recursive-edit)))

If this works reliably, then we could remove that ugly hack
from ‘isearch-highlight-regexp’, I mean the one with the comment
“Turn isearch-string into a case-insensitive regexp”.