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

[Stefan Monnier]

>> >> >    ;; `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.
>> 
>> I believe the numerical value of (nth 0 match-data) will be the same at
>> this point as that of (nth 0 (match-data)) because we just passed that
>> very value to `set-match-data`, and that is always equal
>> (numerically) to (match-beginning 0).
>> Since the only thing we do with that value is pass it immediately to
>> `goto-char`, it makes no difference if it's a marker or an integer, no?
>> 
>> What am I missing?
>
> What are you trying to understand?

Why you're saying that in

    ;; `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)))

it is not true that

    and (nth 0 match-data) == (match-beginning 0), no?

Note: this is not directly related to your patch, since your patch did
not touch that line, AFAICT.

> What my kludge did is simply use a marker, so the adjusted position is
> not clobbered.

I don't see that.  E.g. if you change your code from

    (set-match-data (list (car match-data) (nth 1 (match-data))))
to
    (set-match-data (list (car match-data) (nth 1 (match-data t))))

it fixes the problem just as well, AFAICT.  And it could even be
replaced with:

    (set-match-data (cons (car match-data) (cdr (match-data t))))

My understand is that what you patch does is use the match end as
adjusted by `replace-match` rather than the match end as provided by the
`match-end` argument (because that argument contains integers and hence
ends up pointing to the wrong place after the buffer was modified by
`replace-match`).

Basically, the purpose of the above `set-match-data` originally was to
correct the (match-beginning 0) info because `replace-match` adjusted it
incorrectly (i.e. it works around a bug in `update_search_regs`), but
while doing that it ended up messing up the (match-end 0), so your patch
refines that code so as to leave (match-end 0) unchanged while
correcting (match-beginning 0).

"use a marker kludge" would sound like an appropriate description of the
`advice-add` I sent to Grabriel, on the other hand.


        Stefan