bug#67124: 26.3; query-replace Arg out of range with comma option (at end-buffer)

[Eli Zaretskii <eliz@gnu.org>]

> Date: Sat, 11 Nov 2023 20:40:27 +0100
> From: Gabriele Nicolardi <gabriele@medialab.sissa.it>
> 
> Hi, I'm resending the bug report trying to do better than the previous one.

Thanks.  (In the future, please respond to the original bug number,
instead of creating a new bug report; I've merged them now.)

> I often type `,` (`comma`) to check the replacement. It happens that, 
> some times, I get the error (e.g.):
> 
> match-substitute-replacement: Args out of range: #<buffer *scratch*>, 
> 1667, 1679
> 
> The error doesn't happen if I type `y` (or `n`)
> 
> Try this:
> 
> (query-replace ",.\\footnote{" ".\\footnote{" nil)
> 
> With the string ",.\footnote{" at the end of the buffer. To see the 
> error "{" must be the last char in the buffer.
> 
> (I see often this bug because I use "narrowing" a lot)
> 
> Is it a known bug?

The kludgey solution in the patch below should fix this.  Please see
if it indeed fixes your real-life use cases.

Stefan, any ideas for a better fix, or other comments?  AFAIU, the
root cause of the bug was that we are using integers instead of
markers there, and that is because bug#31492 needs to preserve the
original match-start position when the search string is a regexp that
matches an empty string (in which case the match-start marker moves
together with the match-end marker instead of staying put).

diff --git a/lisp/replace.el b/lisp/replace.el
index eeac734..6a3f223 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -2642,8 +2642,11 @@ replace-match-maybe-edit
   (replace-match newtext fixedcase literal)
   ;; `query-replace' undo feature needs the beginning of the match position,
   ;; but `replace-match' may change it, for instance, with a regexp like "^".
-  ;; Ensure that this function preserves the match data (Bug#31492).
-  (set-match-data match-data)
+  ;; Ensure that this function preserves the beginning of the match position
+  ;; (bug#31492).  But we need to avoid clobbering the end of the match with
+  ;; the original match-end position, since `replace-match' could have made
+  ;; that incorrect or even invalid (bug#67124).
+  (set-match-data (list (car match-data) (nth 1 (match-data))))
   ;; `replace-match' leaves point at the end of the replacement text,
   ;; so move point to the beginning when replacing backward.
   (when backward (goto-char (nth 0 match-data)))