SillyTavern / SillyTavern

LLM Frontend for Power Users.
https://sillytavern.app
GNU Affero General Public License v3.0
8.49k stars 2.45k forks source link

[BUG] Substitute regex bug #3104

Closed soboruB closed 3 days ago

soboruB commented 3 days ago

Environment

🪟 Windows

System

Firefox

Version

1.12.7 staging

Desktop Information

No response

Describe the problem

When using substitute within regex, it matches the literal characters.

Example QR:

/setvar key=regex .* |
/setvar key=regexflags ims |
/setvar key=toreplace TEST |

Regex: 1 2

Additional info

I noticed a pull requests from 3 weeks ago titled 'Fix substitute regex'. This seems to be the cause of the issue. Please revert this change. It's an unwanted feature…

Please tick the boxes

Cohee1207 commented 3 days ago

@honey-tree fight

Cohee1207 commented 3 days ago

Alternatively, "substitute regex" could be made a three-state select: no substitutions, substitute (raw), substitute (escaped).

honey-tree commented 3 days ago

I maintain that my implementation is objectively correct because it bare minimum ensures the regexes outputted are valid for arbitrary {{char}} and generic macro outputs etc.

I am surprised that this actually breaks things for someone. Radio with 3 options sounds good yeah.

honey-tree commented 3 days ago

Actually DOES IT break things for someone? Can you give us the broad strokes on the actual usecase that led you to notice the change in behavior?

EDIT: on my end, the issue was primarily that variables with newlines produce incoherent regexes, and secondarily "/{{char}}/ should reliably match {{char}}", hence the PR.

Cohee1207 commented 3 days ago

Check this one #3105

honey-tree commented 3 days ago

Away from my computer and can't test it but seems reasonable to me.

Cohee1207 commented 3 days ago

I'll wait for someone to test it then.

soboruB commented 3 days ago

I tested substitute(raw) and everything worked well as before. Thanks!

Cohee1207 commented 3 days ago

@honey-tree lemme know if escaped substitution broke. Closing for now.