bug#81583: 32.0.50; Optional preview of the replacement text while typing it

> Cc: Karthik Chikmagalur <[email protected]>, [email protected],
>  Rahul Martim Juliato <[email protected]>,
>  Stéphane Marks <[email protected]>
> From: Rahul Martim Juliato <[email protected]>
> Date: Thu, 20 Aug 2026 21:38:32 -0300
> 
> So this is ready from my side.  Attached is a formatted patch, the same
> as the previous diff plus the etc/NEWS entry, just rebased on current
> master.  If nobody else has comments, it could go in.

Thanks, a few minor comments:

> -+++
>  ** New user option 'query-replace-show-preview'.
> -When set to t, the replacement commands preview the replacement while
> -you type it: the matches visible in the window are shown as they would
> -look after the replacement, using the new face 'query-replace-preview'.
> -This tells you what back-references like '\1' expand to before you
> -commit to the edit.  The preview is off by default.
> +When non-nil, the replacement commands preview the replacement while you

Please avoid using "when" when you really mean "if".  "When" has a
time-wise interpretation, as in "do something when this-and-that
happens", and this is not what you mean here.  Please use "if" in
these cases.

> +With a value of t, each match is shown as the replacement text, in the
   ^^^^^^^^^^^^^^^
"If the value is ..."

> +new face 'query-replace-preview'.  With a value of 'both', the match and
                                      ^^^^^^^^^^^^^^^
Same here.

> +If `both', the match and the replacement appear side by side, separated
   ^^^^^^^^^
"If the value is `both', ..."

> +The value can also be a function of two arguments, the match and the
> +replacement, returning the string to show in place of the match, or nil
> +to leave it alone.  It must propertize the string itself, be fast and
                       ^^^^^^^^^^^^^^^^^^
"Must"? why "must"?  Cannot the function returns a string that is not
prpertized at all?

> +free of side effects, and cope with a replacement still being typed:
   ^^^^^^^^^^^^^^^^^^^^
Why "free of side effects"?  I'm guessing you have some specific side
effect in mind, like moving point or changing the text of the buffer
being searched, but if that is the case, please document the real
restrictions, so that people don't unnecessarily restrict their code.

> +  (lambda (match replacement)
> +    (concat \"[\" match \"]~>{\" replacement \"}\"))"
> +  :type '(choice (const    :tag "No preview" nil)
> +                 (const    :tag "Show the replacement" t)
> +                 (const    :tag "Show the match and the replacement" both)

Suggest to make the tag "Show both match and replacement"

> +                 (function :tag "Function"))

Suggest to tell more: "Function returning the preview string"

>  (defface query-replace-preview
> -  '((t (:inherit query-replace)))
> +  '((t (:inherit diff-added)))
>    "Face for the preview of the replacement text.
>  Used while reading the replacement string of `query-replace' and
>  friends when `query-replace-show-preview' is non-nil."
>    :group 'matching
>    :version "32.1")
>  
> +(defface query-replace-preview-match
> +  '((t (:inherit diff-removed)))
> +  "Face for the matched text in the preview.
> +Used for the left half of the preview when `query-replace-show-preview'
> +is `both'."
> +  :group 'matching
> +  :version "32.1")

The default value of these two faces sounds strange to me.  In
particular, why shouldn't query-replace-preview-match inherit from
query-replace? it shows the same string, no?

> +(defun replace-preview--propertize (text face)
> +  "Return a copy of TEXT with all its text properties replaced by FACE.

Removing all text properties is a bit too radical, no?  There could be
properties which have no effect on display whatsoever.  How about
removing only the properties that can get in the way?