bug#43702: Emacs master: Incorrect highlighting in regexp isearch.

[Juri Linkov <juri@linkov.net>]

[-- Attachment #1: Type: text/plain, Size: 640 bytes --]

>> How highlighting the replacement submatches will help the user to decide
>> whether to replace the current match or not?
>
> Whether to replace a given match is not the only thing
> a user can do - not the only thing to decide/consider.
>
> Highlighting groups helps you see _how_ a regexp matches.
>
> And you can always change a regexp and search/replace
> again, if it's either not matching something you want
> to match or matching too many things you don't want to
> match.
>
> It's helpful in pretty much any context where regexps
> are matched against text, IMO.

I think you are right.  So here is a patch that implements this:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: query-replace-highlight-submatches.patch --]
[-- Type: text/x-diff, Size: 3756 bytes --]

diff --git a/lisp/replace.el b/lisp/replace.el
index e363924501..f9ffa36f54 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -126,6 +126,18 @@ query-replace-highlight
   :type 'boolean
   :group 'matching)
 
+(defcustom query-replace-highlight-submatches t
+  "Whether to highlight regexp subexpressions during query replacement.
+The faces used to do the highlights are named `isearch-group-1',
+`isearch-group-2', etc.  (By default, only these 2 are defined.)
+When there are more matches than faces, then faces are reused from the
+beginning, in a cyclical manner, so the `isearch-group-1' face is
+isreused for the third match.  If you want to use more distinctive colors,
+you can define more of these faces using the same numbering scheme."
+  :type 'boolean
+  :group 'matching
+  :version "28.1")
+
 (defcustom query-replace-lazy-highlight t
   "Controls the lazy-highlighting during query replacements.
 When non-nil, all text in the buffer matching the current match
@@ -2403,16 +2427,36 @@ replace-search
     (funcall search-function search-string limit t)))
 
 (defvar replace-overlay nil)
+(defvar replace-submatches-overlays nil)
 
 (defun replace-highlight (match-beg match-end range-beg range-end
 			  search-string regexp-flag delimited-flag
-			  case-fold &optional backward)
+			  case-fold &optional backward match-data)
   (if query-replace-highlight
       (if replace-overlay
 	  (move-overlay replace-overlay match-beg match-end (current-buffer))
 	(setq replace-overlay (make-overlay match-beg match-end))
 	(overlay-put replace-overlay 'priority 1001) ;higher than lazy overlays
 	(overlay-put replace-overlay 'face 'query-replace)))
+
+  (when (and query-replace-highlight-submatches
+	     regexp-flag)
+    (mapc 'delete-overlay replace-submatches-overlays)
+    (setq replace-submatches-overlays nil)
+    (let ((submatch-data (cddr (butlast match-data)))
+          (group 0)
+          ov face)
+      (while submatch-data
+        (setq group (1+ group))
+        (setq ov (make-overlay (pop submatch-data) (pop submatch-data))
+              face (intern-soft (format "isearch-group-%d" group)))
+        ;; Recycle faces from beginning.
+        (unless (facep face)
+          (setq group 1 face 'isearch-group-1))
+        (overlay-put ov 'face face)
+        (overlay-put ov 'priority 1002)
+        (push ov replace-submatches-overlays))))
+
   (if query-replace-lazy-highlight
       (let ((isearch-string search-string)
 	    (isearch-regexp regexp-flag)
@@ -2433,6 +2477,9 @@ replace-highlight
 (defun replace-dehighlight ()
   (when replace-overlay
     (delete-overlay replace-overlay))
+  (when query-replace-highlight-submatches
+    (mapc 'delete-overlay replace-submatches-overlays)
+    (setq replace-submatches-overlays nil))
   (when query-replace-lazy-highlight
     (lazy-highlight-cleanup lazy-highlight-cleanup)
     (setq isearch-lazy-highlight-last-string nil))
@@ -2694,7 +2741,7 @@ perform-replace
 		    (replace-highlight
 		     (nth 0 real-match-data) (nth 1 real-match-data)
 		     start end search-string
-		     regexp-flag delimited-flag case-fold-search backward))
+		     regexp-flag delimited-flag case-fold-search backward real-match-data))
 		  (setq noedit
 			(replace-match-maybe-edit
 			 next-replacement nocasify literal
@@ -2719,7 +2766,7 @@ perform-replace
                   (replace-highlight
 		   (match-beginning 0) (match-end 0)
 		   start end search-string
-		   regexp-flag delimited-flag case-fold-search backward)
+		   regexp-flag delimited-flag case-fold-search backward real-match-data)
                   ;; Obtain the matched groups: needed only when
                   ;; regexp-flag non nil.
                   (when (and last-was-undo regexp-flag)