Add string-replace-fewer-backslashes feature #5556
Conversation
This disables an extra round of escaping in the `string replace -r`
replacement string.
Currently, to add a backslash to an a or b (to "escape" it):
string replace -ra '([ab])' '\\\\\\\$1' a
7 backslashes!
This removes one of the layers, so now 3 or 4 works (each one escaped
for the single-quotes, so pcre receives two, which it reads as one literal):
string replace -ra '([ab])' '\\\\$1' a
This is backwards-incompatible as replacement strings will change
meaning, so we put it behind a feature flag.
The name is kinda crappy, though.
Fixes fish-shell#5474.
|
I'm kind of opposed to adding more feature flags. I don't think a meaningful number of people are going to opt-in to test features against their scripts; I think we should just |
The main problem is that there is no way to write cross-compatible expressions. If you need a backslash in the replacement, you'll need to write two different ones. So without a feature flag, which allows testing via But I admit there is this question how far we want to take this system - do we want to put everything even slightly backwards-incompatible through it? Or do we only do it for huge stuff (like Personally, I'd like to see a better deprecation cycle! |
|
Could the feature flags be used to test for the new behavior without actually requiring users to configure something to get the saner |
We could default it to on. Also, as a note to myself:
Do that! |
This is the one place in fish where we use a `\` in the replacement of a `string replace -r`, so we'll have to check the feature.
|
@floam yes, imho the feature flags should all default to on in fish 3.0 and users should be able to temporarily opt in to the legacy behavior while updating their scripts by disabling the features in question. The discoverability otherwise approaches zero. |
|
I've just merged this as-is. It's not a huge deal if you do use backslashes in the replacement expression (and none if you don't), but this way people at least get a fighting chance to handle it, and defaulting it to on seems like a bigger precedent that I don't want to set on something this small. |
This disables an extra round of escaping in the
string replace -rreplacement string.
Currently, to add a backslash to an a or b (to "escape" it):
7 backslashes!
This removes one of the layers, so now 3 or 4 works (each one escaped
for the single-quotes, so pcre receives two, which it reads as one literal):
This is backwards-incompatible as replacement strings will change
meaning, so we put it behind a feature flag.
The name is kinda crappy, though.
Fixes #5474.
TODOs:
We really should think about documenting feature flags better!