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

[Eli Zaretskii <eliz@gnu.org>]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Gabriele Nicolardi <gabriele@medialab.sissa.it>,  67124@debbugs.gnu.org
> Date: Sun, 12 Nov 2023 22:56:16 -0500
> 
> (nth 1 (match-data)) == (match-end 0), no?

No, because the former normally returns a marker, whereas the latter
returns a number.  And here the difference is crucial.

> Hmm... so here we're throwing away all the subgroup info and keeping
> only the start/end, right?

Yes.

> It's probably OK, indeed, but I think the comment should clarify
> that (and should clarify that we (well, presumably the undo feature)
> need the match end in addition to the match beginning).

The comment before the patched part (you can see its end in the patch)
says so, no?

> Also here it's not obvious which match-data is returned by (match-data).
> IIUC it's the match data as adjusted by `replace-match`.

Yes, and that's the root cause here: replace-match updates match-data,
but the original code then clobbered it by overwriting it with the
match-data _before_ the replace-match call.

> Which makes me wonder why we don't change `replace-match` so it's also
> careful to preserve the match beginning just like it preserves the match
> end.

AFAIU, it's a general issue with markers: when you have both
match-beginning and match-end at the same buffer position (because the
matched text is an empty string, as when the search regexp is \b or
similar, then replace-match moves them both to the end of the match,
instead of leaving one of them at the beginning of the match.

> >    ;; `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)))
> 
> and (nth 0 match-data) == (match-beginning 0), no?

See above: not exactly.

> So, I tried the patch below, which makes sense to my superficial
> understanding of the problem, but it apparently doesn't fix the problem
> in the OP's recipe, so I'm clearly missing something.

I don't understand the fix, so cannot help you here ;-)