bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.

[Juri Linkov <juri@linkov.net>]

> Sorry for getting back to this after such a long time.  I've attached a
> new patch that hopefully is good to merge, except for adding some NEWS
> entry.  Let me know what you think.

Please see comments for your latest patch below:

> @@ -1812,6 +1812,8 @@ isearch-edit-string
>  	  (minibuffer-history-symbol)
>  	  ;; Search string might have meta information on text properties.
>  	  (minibuffer-allow-text-properties t))
> +     (when isearch-lazy-highlight
> +       (add-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup))

Since this does both highlighting and counting, shouldn't the condition be

        (when (and isearch-lazy-highlight isearch-lazy-count))

Or maybe new separate customizable options are needed, e.g.
'minibuffer-lazy-highlight' and 'minibuffer-lazy-count'?

> @@ -2350,7 +2352,9 @@ isearch-query-replace
>  	(isearch-recursive-edit nil)
>  	(isearch-string-propertized
>           (isearch-string-propertize isearch-string)))
> -    (isearch-done nil t)
> +    (let ((lazy-highlight-cleanup (and lazy-highlight-cleanup
> +                                       (not query-replace-lazy-highlight))))
> +      (isearch-done nil t))

Is this some optimization?  It seems it's intended to leave
some existing highlighting?  Is this to avoid double highlighting?

Also maybe this condition could use a new variable as well.

> @@ -4048,7 +4056,7 @@ isearch-lazy-highlight-new-loop
>  			         isearch-lazy-highlight-window-end))))))
>      ;; something important did indeed change
>      (lazy-highlight-cleanup t (not (equal isearch-string ""))) ;stop old timer
> -    (when (and isearch-lazy-count isearch-mode (null isearch-message-function))
> +    (when isearch-lazy-count
> ...
> @@ -4067,7 +4075,10 @@ isearch-lazy-highlight-new-loop
>          (setq isearch-lazy-count-current nil
>                isearch-lazy-count-total nil)
>          ;; Delay updating the message if possible, to avoid flicker
> -        (when (string-equal isearch-string "") (isearch-message))))
> +        (when (string-equal isearch-string "")
> +          (when (and isearch-mode (null isearch-message-function))
> +            (isearch-message))
> ...
> @@ -4120,13 +4131,15 @@ isearch-lazy-highlight-new-loop
>                                   'isearch-lazy-highlight-start))))
>    ;; Update the current match number only in isearch-mode and
>    ;; unless isearch-mode is used specially with isearch-message-function
> -  (when (and isearch-lazy-count isearch-mode (null isearch-message-function))
> +  (when isearch-lazy-count

The problem is that when these conditions 'isearch-mode (null isearch-message-function)'
are removed, now this shows wrong counts in the minibuffer history search
(e.g. 'M-! C-r s C-r C-r ...') and the shell history search
(e.g. 'M-x shell RET M-r s C-r C-r ...').  Before this change
counting was disabled in the history search because it shows wrong numbers.

> +(defun minibuffer-lazy-highlight--count ()
> +  "Display total match count in the minibuffer prompt."
> +  (when minibuffer-lazy-highlight--overlay
> +    (overlay-put minibuffer-lazy-highlight--overlay
> +                 'after-string
> +                 (and isearch-lazy-count-total
> +                      (not isearch-error)
> +                      (format minibuffer-lazy-count-format
> +                              isearch-lazy-count-total)))))
> ...
> +  (setq minibuffer-lazy-highlight--overlay
> +        (and minibuffer-lazy-count-format
> +             (make-overlay (point-min) (point-min) (current-buffer) t)))

For some reasons the package lisp/mb-depth.el uses 'after-string'
instead of 'before-string', and (make-overlay (point-min) (1+ (point-min)))
instead of (make-overlay (point-min) (point-min)),
so maybe better to do the same?

> @@ -365,14 +372,44 @@ query-replace-read-args
> +    (condition-case error
> +        (let (;; Variables controlling lazy highlighting while reading
> +              ;; FROM and TO.
> +              (lazy-highlight-cleanup nil)
> +              (isearch-lazy-highlight query-replace-lazy-highlight)
> +              (isearch-regexp regexp-flag)
> +              (isearch-regexp-function nil)

Highlighting is still incorrect for word replacement ('C-u M-%')
and for non-nil 'replace-char-fold'.  To handle these cases correctly,
'replace-highlight' uses:

	    (isearch-regexp-function (or replace-regexp-function
					 delimited-flag
					 (and replace-char-fold
					      (not regexp-flag)
					      #'char-fold-to-regexp)))

> @@ -2857,22 +2914,8 @@ perform-replace
>      (when region-noncontiguous-p
> -      (let ((region-bounds
> -             (mapcar (lambda (position)
> -                       (cons (copy-marker (car position))
> -                             (copy-marker (cdr position))))
> -                     (funcall region-extract-function 'bounds))))
> -        (setq region-filter
> -              (lambda (start end)
> -                (delq nil (mapcar
> -                           (lambda (bounds)
> -                             (and
> -                              (>= start (car bounds))
> -                              (<= start (cdr bounds))
> -                              (>= end   (car bounds))
> -                              (<= end   (cdr bounds))))
> -                           region-bounds))))
> -        (add-function :after-while isearch-filter-predicate region-filter)))
> +      (setq region-filter (replace--region-filter
> +                           (funcall region-extract-function 'bounds))))

I wonder why (add-function :after-while isearch-filter-predicate region-filter)
is removed?