From 4ff8e6e7818747d805ac44be0a36aedef4deaa5c Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Tue, 5 Apr 2016 15:43:24 -0700 Subject: change how redirections are formatted Modify `fish_indent` to emit redirections without a space before the target of the redirection; e.g., "2>&1" rather than "2>& 1" as the former is clearer to humans. Fixes #2899 --- doc_src/fish_indent.txt | 2 + src/fish_indent.cpp | 105 +++++++++++++++++++++++++++++++++++------------- src/parse_constants.h | 6 ++- src/parse_tree.cpp | 43 ++++++++++++++++++++ tests/indent.in | 6 +++ tests/indent.out | 4 ++ 6 files changed, 136 insertions(+), 30 deletions(-) diff --git a/doc_src/fish_indent.txt b/doc_src/fish_indent.txt index 95b3c1ec..b4cf13c0 100644 --- a/doc_src/fish_indent.txt +++ b/doc_src/fish_indent.txt @@ -11,6 +11,8 @@ fish_indent [OPTIONS] The following options are available: +- `-d` or `--dump` dumps information about the parsed fish commands to stderr + - `-i` or `--no-indent` do not indent commands; only reformat to one job per line - `-v` or `--version` displays the current fish version and then exits diff --git a/src/fish_indent.cpp b/src/fish_indent.cpp index 037d9b77..0e31f28e 100644 --- a/src/fish_indent.cpp +++ b/src/fish_indent.cpp @@ -15,10 +15,7 @@ along with this program; if not, write to the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA */ -/** \file fish_indent.cpp - The fish_indent proegram. -*/ - +// The fish_indent proegram. #include "config.h" #include @@ -45,10 +42,11 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA #define SPACES_PER_INDENT 4 -/* An indent_t represents an abstract indent depth. 2 means we are in a doubly-nested block, etc. */ +// An indent_t represents an abstract indent depth. 2 means we are in a doubly-nested block, etc. typedef unsigned int indent_t; +static bool dump_parse_tree = false; -/* Read the entire contents of a file into the specified string */ +// Read the entire contents of a file into the specified string. static wcstring read_file(FILE *f) { wcstring result; @@ -69,10 +67,12 @@ static wcstring read_file(FILE *f) return result; } -/* Append whitespace as necessary. If we have a newline, append the appropriate indent. Otherwise, append a space. */ -static void append_whitespace(indent_t node_indent, bool do_indent, bool has_new_line, wcstring *out_result) +// Append whitespace as necessary. If we have a newline, append the appropriate indent. Otherwise, +// append a space. +static void append_whitespace(indent_t node_indent, bool do_indent, bool has_new_line, + wcstring *out_result) { - if (! has_new_line) + if (!has_new_line) { out_result->push_back(L' '); } @@ -82,26 +82,62 @@ static void append_whitespace(indent_t node_indent, bool do_indent, bool has_new } } -static void prettify_node_recursive(const wcstring &source, const parse_node_tree_t &tree, node_offset_t node_idx, indent_t node_indent, parse_token_type_t parent_type, bool *has_new_line, wcstring *out_result, bool do_indent) +// Dump a parse tree node in a form helpful to someone debugging the behavior of this program. +static void dump_node(indent_t node_indent, const parse_node_t &node, const wcstring &source) +{ + int nextc_idx = node.source_start + node.source_length; + wchar_t prevc = node.source_start > 0 ? source[node.source_start - 1] : L' '; + wchar_t nextc = nextc_idx < source.size() ? source[nextc_idx] : L' '; + wchar_t prevc_str[4] = {prevc, 0, 0, 0}; + wchar_t nextc_str[4] = {nextc, 0, 0, 0}; + if (prevc < L' ') + { + prevc_str[0] = L'\\'; + prevc_str[1] = L'c'; + prevc_str[2] = prevc + '@'; + } + if (nextc < L' ') + { + nextc_str[0] = L'\\'; + nextc_str[1] = L'c'; + nextc_str[2] = nextc + '@'; + } + fwprintf(stderr, L"{off %4d, len %4d, indent %2u, %ls} [%ls|%ls|%ls]\n", + node.source_start, node.source_length, node_indent, + parser_token_types[node.type].c_str(), prevc_str, source.substr(node.source_start, + node.source_length).c_str(), nextc_str); +} + +static void prettify_node_recursive(const wcstring &source, const parse_node_tree_t &tree, + node_offset_t node_idx, indent_t node_indent, parse_token_type_t parent_type, + bool *has_new_line, wcstring *out_result, bool do_indent) { const parse_node_t &node = tree.at(node_idx); const parse_token_type_t node_type = node.type; - - /* Increment the indent if we are either a root job_list, or root case_item_list, or in an if or while header (#1665) */ - const bool is_root_job_list = (node_type == symbol_job_list && parent_type != symbol_job_list); - const bool is_root_case_item_list = (node_type == symbol_case_item_list && parent_type != symbol_case_item_list); - const bool is_if_while_header = ((node_type == symbol_job || node_type == symbol_andor_job_list) && - (parent_type == symbol_if_clause || parent_type == symbol_while_header)); - if (is_root_job_list || is_root_case_item_list || is_if_while_header) + const parse_token_type_t prev_node_type = node_idx > 0 ? + tree.at(node_idx - 1).type : token_type_invalid; + + // Increment the indent if we are either a root job_list, or root case_item_list, or in an if or + // while header (#1665). + const bool is_root_job_list = node_type == symbol_job_list && parent_type != symbol_job_list; + const bool is_root_case_list = node_type == symbol_case_item_list && + parent_type != symbol_case_item_list; + const bool is_if_while_header = \ + (node_type == symbol_job || node_type == symbol_andor_job_list) && + (parent_type == symbol_if_clause || parent_type == symbol_while_header); + + if (is_root_job_list || is_root_case_list || is_if_while_header) { node_indent += 1; } - /* Handle comments, which come before the text */ - if (node.has_comments()) + if (dump_parse_tree) dump_node(node_indent, node, source); + + if (node.has_comments()) // handle comments, which come before the text { - const parse_node_tree_t::parse_node_list_t comment_nodes = tree.comment_nodes_for_node(node); - for (size_t i=0; i < comment_nodes.size(); i++) + const parse_node_tree_t::parse_node_list_t comment_nodes = ( + tree.comment_nodes_for_node(node)); + for (size_t i = 0; i < comment_nodes.size(); i++) { const parse_node_t &comment_node = *comment_nodes.at(i); append_whitespace(node_indent, do_indent, *has_new_line, out_result); @@ -111,26 +147,30 @@ static void prettify_node_recursive(const wcstring &source, const parse_node_tre if (node_type == parse_token_type_end) { - /* Newline */ out_result->push_back(L'\n'); *has_new_line = true; } - else if ((node_type >= FIRST_PARSE_TOKEN_TYPE && node_type <= LAST_PARSE_TOKEN_TYPE) || node_type == parse_special_type_parse_error) + else if ((node_type >= FIRST_PARSE_TOKEN_TYPE && node_type <= LAST_PARSE_TOKEN_TYPE) || + node_type == parse_special_type_parse_error) { if (node.has_source()) { - /* Some type representing a particular token */ - append_whitespace(node_indent, do_indent, *has_new_line, out_result); + // Some type representing a particular token. + if (prev_node_type != parse_token_type_redirection) + { + append_whitespace(node_indent, do_indent, *has_new_line, out_result); + } out_result->append(source, node.source_start, node.source_length); *has_new_line = false; } } - /* Recurse to all our children */ + // Recurse to all our children. for (node_offset_t idx = 0; idx < node.child_count; idx++) { - /* Note we pass our type to our child, which becomes its parent node type */ - prettify_node_recursive(source, tree, node.child_start + idx, node_indent, node_type, has_new_line, out_result, do_indent); + // Note we pass our type to our child, which becomes its parent node type. + prettify_node_recursive(source, tree, node.child_start + idx, node_indent, node_type, + has_new_line, out_result, do_indent); } } @@ -297,9 +337,10 @@ int main(int argc, char *argv[]) } output_type = output_type_plain_text; bool do_indent = true; - const char *short_opts = "+hvi"; + const char *short_opts = "+dhvi"; const struct option long_opts[] = { + { "dump", no_argument, NULL, 'd' }, { "no-indent", no_argument, NULL, 'i' }, { "help", no_argument, NULL, 'h' }, { "version", no_argument, NULL, 'v' }, @@ -319,6 +360,12 @@ int main(int argc, char *argv[]) exit_without_destructors(127); } + case 'd': + { + dump_parse_tree = true; + break; + } + case 'h': { print_help("fish_indent", 1); diff --git a/src/parse_constants.h b/src/parse_constants.h index e7c6399f..afc88c6c 100644 --- a/src/parse_constants.h +++ b/src/parse_constants.h @@ -11,6 +11,8 @@ #define PARSE_ASSERT(a) assert(a) #define PARSER_DIE() do { fprintf(stderr, "Parser dying!\n"); exit_without_destructors(-1); } while (0) +// IMPORTANT: If the following enum is modified you must update the corresponding parser_token_types +// array in parse_tree.cpp. enum parse_token_type_t { token_type_invalid, @@ -41,7 +43,7 @@ enum parse_token_type_t symbol_plain_statement, symbol_arguments_or_redirections_list, symbol_argument_or_redirection, - + symbol_andor_job_list, symbol_argument_list, @@ -80,6 +82,8 @@ enum parse_token_type_t FIRST_PARSE_TOKEN_TYPE = parse_token_type_string, LAST_PARSE_TOKEN_TYPE = parse_token_type_end } __packed; +// Array of strings corresponding to the enums above instantiated in parse_tree.cpp. +extern wcstring parser_token_types[]; /* These must be maintained in sorted order (except for none, which isn't a keyword). This enables us to do binary search. */ enum parse_keyword_t diff --git a/src/parse_tree.cpp b/src/parse_tree.cpp index 51ae7720..3d2f7b8c 100644 --- a/src/parse_tree.cpp +++ b/src/parse_tree.cpp @@ -16,6 +16,49 @@ #include #include +// This array provides strings for each symbol in enum parse_token_type_t in parse_constants.h. +wcstring parser_token_types[] = { + L"token_type_invalid", + L"symbol_job_list", + L"symbol_job", + L"symbol_job_continuation", + L"symbol_statement", + L"symbol_block_statement", + L"symbol_block_header", + L"symbol_for_header", + L"symbol_while_header", + L"symbol_begin_header", + L"symbol_function_header", + L"symbol_if_statement", + L"symbol_if_clause", + L"symbol_else_clause", + L"symbol_else_continuation", + L"symbol_switch_statement", + L"symbol_case_item_list", + L"symbol_case_item", + L"symbol_boolean_statement", + L"symbol_decorated_statement", + L"symbol_plain_statement", + L"symbol_arguments_or_redirections_list", + L"symbol_argument_or_redirection", + L"symbol_andor_job_list", + L"symbol_argument_list", + L"symbol_freestanding_argument_list", + L"symbol_argument", + L"symbol_redirection", + L"symbol_optional_background", + L"symbol_end_command", + L"parse_token_type_string", + L"parse_token_type_pipe", + L"parse_token_type_redirection", + L"parse_token_type_background", + L"parse_token_type_end", + L"parse_token_type_terminate", + L"parse_special_type_parse_error", + L"parse_special_type_tokenizer_error", + L"parse_special_type_comment", + }; + using namespace parse_productions; static bool production_is_empty(const production_t *production) diff --git a/tests/indent.in b/tests/indent.in index 83ef915b..a1143c67 100644 --- a/tests/indent.in +++ b/tests/indent.in @@ -83,3 +83,9 @@ echo -n ' if begin ; false; end; echo hi ; end while begin ; false; end; echo hi ; end ' | ../test/root/bin/fish_indent + +echo \nTest redir formatting +# issue 2899 +echo -n ' +echo < stdin >>appended yes 2>&1 no > stdout maybe 2>& 4 | cat 2>| cat +' | ../test/root/bin/fish_indent diff --git a/tests/indent.out b/tests/indent.out index b2eaf08a..6b38a72f 100644 --- a/tests/indent.out +++ b/tests/indent.out @@ -91,3 +91,7 @@ while begin end echo hi end + +Test redir formatting + +echo >appended yes 2>&1 no >stdout maybe 2>&4 | cat 2>| cat -- cgit v1.2.3