From 864bb1f7a6c7b3aa582578f875d110f1b1e801dd Mon Sep 17 00:00:00 2001
From: Fabian Homborg <FHomborg@gmail.com>
Date: Sat, 12 Jan 2019 20:20:35 +0100
Subject: [PATCH] Add string-replace-fewer-backslashes feature

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 #5474.
---
 src/builtin_string.cpp                                 | 10 ++++++++--
 src/future_feature_flags.cpp                           |  1 +
 src/future_feature_flags.h                             |  3 +++
 .../invocation/features-string-backslashes-off.invoke  |  1 +
 tests/invocation/features-string-backslashes-off.out   |  1 +
 tests/invocation/features-string-backslashes.invoke    |  1 +
 tests/invocation/features-string-backslashes.out       |  1 +
 tests/status.out                                       |  1 +
 8 files changed, 17 insertions(+), 2 deletions(-)
 create mode 100644 tests/invocation/features-string-backslashes-off.invoke
 create mode 100644 tests/invocation/features-string-backslashes-off.out
 create mode 100644 tests/invocation/features-string-backslashes.invoke
 create mode 100644 tests/invocation/features-string-backslashes.out

diff --git a/src/builtin_string.cpp b/src/builtin_string.cpp
index a136b1fb2..201e7ab25 100644
--- a/src/builtin_string.cpp
+++ b/src/builtin_string.cpp
@@ -26,6 +26,7 @@
 #include "builtin.h"
 #include "common.h"
 #include "fallback.h"  // IWYU pragma: keep
+#include "future_feature_flags.h"
 #include "io.h"
 #include "parse_util.h"
 #include "pcre2.h"
@@ -933,8 +934,13 @@ class regex_replacer_t : public string_replacer_t {
     regex_replacer_t(const wchar_t *argv0, const wcstring &pattern, const wcstring &replacement_,
                      const options_t &opts, io_streams_t &streams)
         : string_replacer_t(argv0, opts, streams),
-          regex(argv0, pattern, opts.ignore_case, streams),
-          replacement(interpret_escapes(replacement_)) {}
+          regex(argv0, pattern, opts.ignore_case, streams) {
+        if (feature_test(features_t::string_replace_backslash)) {
+            replacement = replacement_;
+        } else {
+            replacement = interpret_escapes(replacement_);
+        }
+    }
 
     bool replace_matches(const wcstring &arg) override;
 };
diff --git a/src/future_feature_flags.cpp b/src/future_feature_flags.cpp
index 4feca703b..2caf145f5 100644
--- a/src/future_feature_flags.cpp
+++ b/src/future_feature_flags.cpp
@@ -13,6 +13,7 @@ features_t &mutable_fish_features() { return global_features; }
 const features_t::metadata_t features_t::metadata[features_t::flag_count] = {
     {stderr_nocaret, L"stderr-nocaret", L"3.0", L"^ no longer redirects stderr"},
     {qmark_noglob, L"qmark-noglob", L"3.0", L"? no longer globs"},
+    {string_replace_backslash, L"string-replace-fewer-backslashes", L"3.1", L"string replace -r needs fewer backslashes in the replacement"},
 };
 
 const struct features_t::metadata_t *features_t::metadata_for(const wchar_t *name) {
diff --git a/src/future_feature_flags.h b/src/future_feature_flags.h
index 7ccc7f570..1c42cb21a 100644
--- a/src/future_feature_flags.h
+++ b/src/future_feature_flags.h
@@ -17,6 +17,9 @@ class features_t {
      /// Whether ? is supported as a glob.
      qmark_noglob,
 
+     /// Whether string replace -r double-unescapes the replacement.
+     string_replace_backslash,
+
      /// The number of flags.
      flag_count
  };
diff --git a/tests/invocation/features-string-backslashes-off.invoke b/tests/invocation/features-string-backslashes-off.invoke
new file mode 100644
index 000000000..600bde64f
--- /dev/null
+++ b/tests/invocation/features-string-backslashes-off.invoke
@@ -0,0 +1 @@
+--features 'no-string-replace-fewer-backslashes' -C 'string replace -ra "\\\\" "\\\\\\\\" -- "a\b\c"'
diff --git a/tests/invocation/features-string-backslashes-off.out b/tests/invocation/features-string-backslashes-off.out
new file mode 100644
index 000000000..2eba0a277
--- /dev/null
+++ b/tests/invocation/features-string-backslashes-off.out
@@ -0,0 +1 @@
+a\b\c
diff --git a/tests/invocation/features-string-backslashes.invoke b/tests/invocation/features-string-backslashes.invoke
new file mode 100644
index 000000000..e7923c0dc
--- /dev/null
+++ b/tests/invocation/features-string-backslashes.invoke
@@ -0,0 +1 @@
+--features 'string-replace-fewer-backslashes' -C 'string replace -ra "\\\\" "\\\\\\\\" -- "a\b\c"'
diff --git a/tests/invocation/features-string-backslashes.out b/tests/invocation/features-string-backslashes.out
new file mode 100644
index 000000000..e67f30541
--- /dev/null
+++ b/tests/invocation/features-string-backslashes.out
@@ -0,0 +1 @@
+a\\b\\c
diff --git a/tests/status.out b/tests/status.out
index ea2ab205e..283b13030 100644
--- a/tests/status.out
+++ b/tests/status.out
@@ -6,5 +6,6 @@ test_function
 # Future Feature Flags
 stderr-nocaret	off	3.0	^ no longer redirects stderr
 qmark-noglob	off	3.0	? no longer globs
+string-replace-fewer-backslashes	off	3.1	string replace -r needs fewer backslashes in the replacement
 1
 2