From b25f72f39140dffef4b3fc0986e557ac4fdf3db5 Mon Sep 17 00:00:00 2001
From: Fabian Homborg <FHomborg@gmail.com>
Date: Tue, 11 Jun 2019 19:35:49 +0200
Subject: [PATCH] Teach fish_indent to remove useless quotes

This tries to see if quotes guard some expansion from happening. If it
detects a "weird" character it'll leave the quotes in place, even in
some cases where it might not trigger.

So

    for i in 'c' 'color'

turns into

    for i in c color

The rationale here is that these quotes are useless, wasting
space (and line length), but more importantly that they are
superstitions. They don't do anything, but look like they do.

The counter argument is that they can be kept in case of later
changes, or that they make the intent clear - "this is supposed to be
a string we pass".
---
 src/fish_indent.cpp | 21 ++++++++++++++++++++-
 tests/indent.out    |  7 +++----
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/src/fish_indent.cpp b/src/fish_indent.cpp
index dbf496cc7..0de0cb98d 100644
--- a/src/fish_indent.cpp
+++ b/src/fish_indent.cpp
@@ -36,6 +36,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
 #include "color.h"
 #include "common.h"
 #include "env.h"
+#include "expand.h"
 #include "fish_version.h"
 #include "flog.h"
 #include "highlight.h"
@@ -251,7 +252,25 @@ void prettifier_t::prettify_node(const parse_node_tree_t &tree, node_offset_t no
                 if (prev_node_type != parse_token_type_redirection) {
                     append_whitespace(node_indent);
                 }
-                output.append(source, node.source_start, node.source_length);
+                wcstring unescaped{source, node.source_start, node.source_length};
+                // Unescape the string - this leaves special markers around if there are any expansions or anything.
+                // TODO: This also already computes backslash-escapes like \u or \x.
+                // We probably don't want that - if someone picked `\x20` to spell space, they have a reason.
+                unescape_string_in_place(&unescaped, UNESCAPE_SPECIAL);
+
+                // Remove INTERNAL_SEPARATOR because that's a quote.
+                auto quote = [](wchar_t ch) { return ch == INTERNAL_SEPARATOR; };
+                unescaped.erase(std::remove_if(unescaped.begin(), unescaped.end(), quote),
+                                unescaped.end());
+
+                // If no non-alphanumeric char is left, use the unescaped version.
+                // This can be extended to other characters, but giving the precise list is tough,
+                // can change over time (see "^", "%" and "?", in some cases "{}") and it just makes people feel more at ease.
+                if (std::find_if_not(unescaped.begin(), unescaped.end(), fish_iswalnum) == unescaped.end() && !unescaped.empty()) {
+                    output.append(unescaped);
+                } else {
+                    output.append(source, node.source_start, node.source_length);
+                }
                 has_new_line = false;
             }
         }
diff --git a/tests/indent.out b/tests/indent.out
index 8003da678..204b42521 100644
--- a/tests/indent.out
+++ b/tests/indent.out
@@ -103,10 +103,9 @@ f true
 en\
 d
 
-"whil\
-e" true
-    "builtin" yes
-en"d"
+while true
+    builtin yes
+end
 
 alpha | \
     beta