From e40eba35856ac6d52611d3fc04ade1d5a1f2391a Mon Sep 17 00:00:00 2001
From: Johannes Altmanninger <aclopte@gmail.com>
Date: Sat, 30 Oct 2021 17:32:48 +0200
Subject: [PATCH] Treat text following quoted command substitution as quoted

Commit ec3d3a481 (Support "$(cmd)" command substitution without line
splitting, 2021-07-02) started treating an input string like
"a$()b" as if it were "a"$()"b". Yet, we do not actually insert the
virtual quotes. Instead we just adapted the definition of when quotes
are closed - hence the changes to quote_end().

parse_util_locate_cmdsubst_range() is aware
of the changes to quote_end() but some of its
callers like parse_util_detect_errors_in_argument() and
highlighter_t::color_as_argument() are not.  They split strings at
command substitution boundaries without handling the special quoting
rules. (Only the expansion logic did it right.)

Fix this by handling the special quoting rules inside
parse_util_locate_cmdsubst_range(). This is a bit hacky since it
makes it harder for callers to process some substrings in between
command substitutions, but that's okay because current callers only
care about what's inside the command substitutions.

Fixes #8394
---
 src/parse_util.cpp       | 19 ++++++++++++++++++-
 tests/checks/cmdsub.fish | 12 ++++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/src/parse_util.cpp b/src/parse_util.cpp
index 49efb4585..bdc72e458 100644
--- a/src/parse_util.cpp
+++ b/src/parse_util.cpp
@@ -230,6 +230,7 @@ int parse_util_locate_cmdsubst_range(const wcstring &str, size_t *inout_cursor_o
     if (out_contents != nullptr) out_contents->clear();
     *out_start = 0;
     *out_end = str.size();
+    bool cmdsub_is_quoted = false;
 
     // Nothing to do if the offset is at or past the end of the string.
     if (*inout_cursor_offset >= str.size()) return 0;
@@ -242,7 +243,7 @@ int parse_util_locate_cmdsubst_range(const wcstring &str, size_t *inout_cursor_o
     const wchar_t *bracket_range_end = nullptr;
 
     int ret = parse_util_locate_cmdsub(valid_range_start, &bracket_range_begin, &bracket_range_end,
-                                       accept_incomplete, out_is_quoted);
+                                       accept_incomplete, &cmdsub_is_quoted);
     if (ret <= 0) {
         return ret;
     }
@@ -263,10 +264,26 @@ int parse_util_locate_cmdsubst_range(const wcstring &str, size_t *inout_cursor_o
     // Return the start and end.
     *out_start = bracket_range_begin - buff;
     *out_end = bracket_range_end - buff;
+    if (out_is_quoted) *out_is_quoted = cmdsub_is_quoted;
 
     // Update the inout_cursor_offset. Note this may cause it to exceed str.size(), though
     // overflow is not likely.
     *inout_cursor_offset = 1 + *out_end;
+
+    // Update the inout_cursor_offset. Note this may cause it to exceed str.size(), though
+    // overflow is not likely.
+    *inout_cursor_offset = 1 + *out_end;
+    if (cmdsub_is_quoted && *bracket_range_end) {
+        // We are usually called in a loop, to process all command substitutions in this string.
+        // If we just located a quoted cmdsub like $(A) inside "$(A)B"(C), the B part is also
+        // quoted but the naïve next caller wouldn't know. Since next caller only cares about
+        // the next command substitution - (C) - and not about the B part, just advance the
+        // cursor to the closing quote.
+        const wchar_t *q_end = quote_end(bracket_range_end, L'"');
+        assert(q_end && "expect balanced quotes");
+        *inout_cursor_offset = 1 + q_end - buff;
+    }
+
     return ret;
 }
 
diff --git a/tests/checks/cmdsub.fish b/tests/checks/cmdsub.fish
index 9c833e2b2..e6d475615 100644
--- a/tests/checks/cmdsub.fish
+++ b/tests/checks/cmdsub.fish
@@ -45,3 +45,15 @@ echo "$(echo '"')"
 
 echo "$(echo $(echo 1) ())"
 #CHECK: 1
+
+echo "$(echo 1))"
+# CHECK: 1)
+
+echo "($(echo 1))"
+# CHECK: (1)
+
+echo "$(echo 1) ( $(echo 2)"
+# CHECK: 1 ( 2
+
+echo "$(echo A)B$(echo C)D"(echo E)
+# CHECK: ABCDE