Fix bug in SubstitutionString replace

[sam0410]

fixes #27125

[nalimilan]

Thanks. I think the following comment by @digital-carver at #27125 needs to be taken into account, though:

There's a change in behaviour due to this though: invalid single letter sequences like \m, \j, etc. are silently replaced by their corresponding character (m, j, ...) by unescape_string, whereas currently _replace emits the "Bad replacement string" error for those. Perhaps unescape_string needs another optional argument to enable a strict mode, or... is there a reason unescape_string is currently not strict about invalid C sequences, while it errors out on invalid Unicode sequences?

[sam0410]

Thanks for your review @nalimilan. I thought the following comment has already been merged in pull request #27238 ?

There's a change in behaviour due to this though: invalid single letter sequences like \m, \j, etc. are silently replaced by their corresponding character (m, j, ...) by unescape_string, whereas currently _replace emits the "Bad replacement string" error for those. Perhaps unescape_string needs another optional argument to enable a strict mode, or... is there a reason unescape_string is currently not strict about invalid C sequences, while it errors out on invalid Unicode sequences?

[nalimilan]

Thanks!

I realize that if performance really matters it could be more efficient to pass a function instead of an array of characters: c -> c == '\\' || c == 'g' || '0' <= c <= '9' would need fewer operations. But that probably doesn't matter a lot.

[sam0410]

Thanks for approving this PR @nalimilan. I think it is better to allow array of characters for making the usage of this function easier.

[sam0410]

Hi @StefanKarpinski @nalimilan , just came in to see if this could be merged.

[JeffBezanson]

Bump. @StefanKarpinski ok to merge? (needs rebase tho)

[StefanKarpinski]

Can be squash-merged if CI passes.