From 62b3ed17ba42150e1107b87b0e719cf793ae8d0f Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Thu, 27 Mar 2014 11:17:05 -0700 Subject: [PATCH 01/10] Teach parser_t how to parse an argument list that contains newlines, for complete -a support. Fixes #1369 --- parse_constants.h | 6 +++++- parse_productions.cpp | 21 +++++++++++++++++++++ parse_tree.cpp | 2 ++ parse_tree.h | 7 ++++++- parser.cpp | 10 +++++----- 5 files changed, 39 insertions(+), 7 deletions(-) diff --git a/parse_constants.h b/parse_constants.h index 8ed37acdf..ebf6030bd 100644 --- a/parse_constants.h +++ b/parse_constants.h @@ -43,6 +43,10 @@ enum parse_token_type_t symbol_argument_or_redirection, symbol_argument_list, + + // "freestanding" argument lists are parsed from the argument list supplied to 'complete -a' + // They are not generated by parse trees rooted in symbol_job_list + symbol_freestanding_argument_list, symbol_argument, symbol_redirection, @@ -50,7 +54,7 @@ enum parse_token_type_t symbol_optional_background, symbol_end_command, - + // Terminal types parse_token_type_string, parse_token_type_pipe, diff --git a/parse_productions.cpp b/parse_productions.cpp index d559a0d93..3d65e6294 100644 --- a/parse_productions.cpp +++ b/parse_productions.cpp @@ -276,6 +276,26 @@ RESOLVE(argument_list) } } +PRODUCTIONS(freestanding_argument_list) = +{ + {}, + {symbol_argument, symbol_freestanding_argument_list}, + {parse_token_type_end, symbol_freestanding_argument_list}, +}; +RESOLVE(freestanding_argument_list) +{ + switch (token1.type) + { + case parse_token_type_string: + return 1; + case parse_token_type_end: + return 2; + default: + return 0; + } +} + + PRODUCTIONS(block_statement) = { {symbol_block_header, parse_token_type_end, symbol_job_list, symbol_end_command, symbol_arguments_or_redirections_list} @@ -485,6 +505,7 @@ const production_t *parse_productions::production_for_token(parse_token_type_t n TEST(case_item_list) TEST(case_item) TEST(argument_list) + TEST(freestanding_argument_list) TEST(block_header) TEST(for_header) TEST(while_header) diff --git a/parse_tree.cpp b/parse_tree.cpp index ba742bcc4..b080cc435 100644 --- a/parse_tree.cpp +++ b/parse_tree.cpp @@ -186,6 +186,8 @@ wcstring token_type_description(parse_token_type_t type) case symbol_argument_list: return L"argument_list"; + case symbol_freestanding_argument_list: + return L"freestanding_argument_list"; case symbol_boolean_statement: return L"boolean_statement"; diff --git a/parse_tree.h b/parse_tree.h index 5b2f717e3..f9f27bebb 100644 --- a/parse_tree.h +++ b/parse_tree.h @@ -254,7 +254,12 @@ bool parse_tree_from_string(const wcstring &str, parse_tree_flags_t flags, parse optional_background = | end_command = END - + + # A freestanding_argument_list is equivalent to a normal argument list, except it may contain TOK_END (newlines, and even semicolons, for historical reasons: + + freestanding_argument_list = | + argument freestanding_argument_list | + freestanding_argument_list */ #endif diff --git a/parser.cpp b/parser.cpp index 4a7ee602d..e95cb8780 100644 --- a/parser.cpp +++ b/parser.cpp @@ -493,7 +493,7 @@ void parser_t::expand_argument_list(const wcstring &arg_list_src, std::vectortype == symbol_argument_list); + assert(arg_list->type == symbol_freestanding_argument_list); /* Extract arguments from it */ while (arg_list != NULL) @@ -968,18 +968,18 @@ bool parser_t::detect_errors_in_argument_list(const wcstring &arg_list_src, wcst /* Parse the string as an argument list */ parse_node_tree_t tree; - if (! parse_tree_from_string(arg_list_src, parse_flag_none, &tree, &errors, symbol_argument_list)) + if (! parse_tree_from_string(arg_list_src, parse_flag_none, &tree, &errors, symbol_freestanding_argument_list)) { /* Failed to parse. */ errored = true; } - + if (! errored) { /* Get the root argument list */ assert(! tree.empty()); const parse_node_t *arg_list = &tree.at(0); - assert(arg_list->type == symbol_argument_list); + assert(arg_list->type == symbol_freestanding_argument_list); /* Extract arguments from it */ while (arg_list != NULL && ! errored) From 42166be22e5bc2ab16d8e9e895696d325aef8246 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Thu, 27 Mar 2014 11:34:18 -0700 Subject: [PATCH 02/10] Make the argument list parsing in complete -a robust against weird tokens like &. Improve the error message when such tokens are found. --- parse_tree.cpp | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/parse_tree.cpp b/parse_tree.cpp index b080cc435..d8aefaff0 100644 --- a/parse_tree.cpp +++ b/parse_tree.cpp @@ -274,7 +274,7 @@ wcstring keyword_description(parse_keyword_t k) } } -static wcstring token_type_user_presentable_description(parse_token_type_t type, parse_keyword_t keyword) +static wcstring token_type_user_presentable_description(parse_token_type_t type, parse_keyword_t keyword = parse_keyword_none) { if (keyword != parse_keyword_none) { @@ -287,6 +287,9 @@ static wcstring token_type_user_presentable_description(parse_token_type_t type, case symbol_statement: return L"a command"; + case symbol_argument: + return L"an argument"; + case parse_token_type_string: return L"a string"; @@ -301,7 +304,7 @@ static wcstring token_type_user_presentable_description(parse_token_type_t type, case parse_token_type_end: return L"end of the statement"; - + default: return format_string(L"a %ls", token_type_description(type).c_str()); } @@ -754,8 +757,6 @@ void parse_ll_t::parse_error_unbalancing_token(parse_token_t token) this->fatal_errored = true; if (this->should_generate_error_messages) { - assert(token.type == parse_token_type_string); - assert(token.keyword == parse_keyword_end || token.keyword == parse_keyword_else || token.keyword == parse_keyword_case); switch (token.keyword) { case parse_keyword_end: @@ -771,8 +772,17 @@ void parse_ll_t::parse_error_unbalancing_token(parse_token_t token) break; default: - fprintf(stderr, "Unexpected token %ls passed to %s\n", token.describe().c_str(), __FUNCTION__); - PARSER_DIE(); + // At the moment, this case should only be hit if you parse a freestanding_argument_list + // For example, 'complete -c foo -a 'one & three' + // Hackish error message for that case + if (! symbol_stack.empty() && symbol_stack.back().type == symbol_freestanding_argument_list) + { + this->parse_error(token, parse_error_generic, L"Expected %ls, but found %ls", token_type_user_presentable_description(symbol_argument).c_str(), token.user_presentable_description().c_str()); + } + else + { + this->parse_error(token, parse_error_generic, L"Did not expect %ls", token.user_presentable_description().c_str()); + } break; } } From c1f64ba017b3294820ed5381cfd7a6e36501dcc8 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Thu, 27 Mar 2014 13:46:33 -0700 Subject: [PATCH 03/10] Make set_color fail silently if there is no argument (reintroducing 469743c). Fixes #1335 --- builtin_set_color.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/builtin_set_color.cpp b/builtin_set_color.cpp index 0fdc1ac0b..79a81bbb9 100644 --- a/builtin_set_color.cpp +++ b/builtin_set_color.cpp @@ -81,6 +81,12 @@ static int builtin_set_color(parser_t &parser, wchar_t **argv) int argc = builtin_count_args(argv); + /* Some code passes variables to set_color that don't exist, like $fish_user_whatever. As a hack, quietly return failure. */ + if (argc <= 1) + { + return EXIT_FAILURE; + } + const wchar_t *bgcolor = NULL; bool bold = false, underline=false; int errret; From 005edf71a8ed3493b9fbbc1de81d10ed73cdca79 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Fri, 28 Mar 2014 14:39:47 -0700 Subject: [PATCH 04/10] Fix initially backgrounded jobs. Fixes #1373 --- highlight.cpp | 3 ++- parse_execution.cpp | 2 +- parse_productions.cpp | 4 ++-- parse_tree.cpp | 13 +++++++++++++ parse_tree.h | 11 +++++++---- 5 files changed, 25 insertions(+), 8 deletions(-) diff --git a/highlight.cpp b/highlight.cpp index c1c24dd89..d03a60873 100644 --- a/highlight.cpp +++ b/highlight.cpp @@ -960,7 +960,7 @@ public: void highlighter_t::color_node(const parse_node_t &node, highlight_spec_t color) { // Can only color nodes with valid source ranges - if (! node.has_source()) + if (! node.has_source() || node.source_length == 0) return; // Fill the color array with our color in the corresponding range @@ -1356,6 +1356,7 @@ const highlighter_t::color_array_t & highlighter_t::highlight() case parse_token_type_background: case parse_token_type_end: + case symbol_optional_background: { this->color_node(node, highlight_spec_statement_terminator); } diff --git a/parse_execution.cpp b/parse_execution.cpp index 649caaa8e..8a2e9d271 100644 --- a/parse_execution.cpp +++ b/parse_execution.cpp @@ -1371,7 +1371,7 @@ parse_execution_result_t parse_execution_context_t::run_1_job(const parse_node_t (job_control_mode==JOB_CONTROL_ALL) || ((job_control_mode == JOB_CONTROL_INTERACTIVE) && (get_is_interactive()))); - job_set_flag(j, JOB_FOREGROUND, 1); + job_set_flag(j, JOB_FOREGROUND, ! tree.job_should_be_backgrounded(job_node)); job_set_flag(j, JOB_TERMINAL, job_get_flag(j, JOB_CONTROL) \ && (!is_subshell && !is_event)); diff --git a/parse_productions.cpp b/parse_productions.cpp index 3d65e6294..0fd754354 100644 --- a/parse_productions.cpp +++ b/parse_productions.cpp @@ -82,7 +82,7 @@ RESOLVE(job_list) PRODUCTIONS(job) = { - {symbol_statement, symbol_job_continuation} + {symbol_statement, symbol_job_continuation, symbol_optional_background} }; RESOLVE_ONLY(job) @@ -402,7 +402,7 @@ RESOLVE(decorated_statement) PRODUCTIONS(plain_statement) = { - {parse_token_type_string, symbol_arguments_or_redirections_list, symbol_optional_background} + {parse_token_type_string, symbol_arguments_or_redirections_list} }; RESOLVE_ONLY(plain_statement) diff --git a/parse_tree.cpp b/parse_tree.cpp index d8aefaff0..4effea20e 100644 --- a/parse_tree.cpp +++ b/parse_tree.cpp @@ -1496,6 +1496,19 @@ parse_node_tree_t::parse_node_list_t parse_node_tree_t::specific_statements_for_ return result; } +bool parse_node_tree_t::job_should_be_backgrounded(const parse_node_t &job) const +{ + assert(job.type == symbol_job); + assert(job.production_idx == 0); + bool result = false; + const parse_node_t *opt_background = get_child(job, 2, symbol_optional_background); + if (opt_background != NULL) { + assert(opt_background->production_idx <= 1); + result = (opt_background->production_idx == 1); + } + return result; +} + const parse_node_t *parse_node_tree_t::next_node_in_node_list(const parse_node_t &node_list, parse_token_type_t entry_type, const parse_node_t **out_list_tail) const { parse_token_type_t list_type = node_list.type; diff --git a/parse_tree.h b/parse_tree.h index f9f27bebb..c0e24a5ca 100644 --- a/parse_tree.h +++ b/parse_tree.h @@ -184,6 +184,9 @@ public: /* Given a job, return all of its statements. These are 'specific statements' (e.g. symbol_decorated_statement, not symbol_statement) */ parse_node_list_t specific_statements_for_job(const parse_node_t &job) const; + + /* Given a job, return whether it should be backgrounded, because it has a & specifier */ + bool job_should_be_backgrounded(const parse_node_t &job) const; }; @@ -198,9 +201,9 @@ bool parse_tree_from_string(const wcstring &str, parse_tree_flags_t flags, parse job job_list job_list -# A job is a non-empty list of statements, separated by pipes. (Non-empty is useful for cases like if statements, where we require a command). To represent "non-empty", we require a statement, followed by a possibly empty job_continuation +# A job is a non-empty list of statements, separated by pipes. (Non-empty is useful for cases like if statements, where we require a command). To represent "non-empty", we require a statement, followed by a possibly empty job_continuation, and then optionally a background specifier '&' - job = statement job_continuation + job = statement job_continuation optional_background job_continuation = | statement job_continuation @@ -240,7 +243,7 @@ bool parse_tree_from_string(const wcstring &str, parse_tree_flags_t flags, parse # A decorated_statement is a command with a list of arguments_or_redirections, possibly with "builtin" or "command" or "exec" decorated_statement = plain_statement | COMMAND plain_statement | BUILTIN plain_statement | EXEC plain_statement - plain_statement = arguments_or_redirections_list optional_background + plain_statement = arguments_or_redirections_list argument_list = | argument argument_list @@ -255,7 +258,7 @@ bool parse_tree_from_string(const wcstring &str, parse_tree_flags_t flags, parse end_command = END - # A freestanding_argument_list is equivalent to a normal argument list, except it may contain TOK_END (newlines, and even semicolons, for historical reasons: + # A freestanding_argument_list is equivalent to a normal argument list, except it may contain TOK_END (newlines, and even semicolons, for historical reasons freestanding_argument_list = | argument freestanding_argument_list | From 74b28f0a8632b91f99d73f0f613d04a9d26d22e1 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Fri, 28 Mar 2014 16:56:44 -0700 Subject: [PATCH 05/10] Fix for crash with malformed switch statement. Fixes #1376 --- parse_execution.cpp | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/parse_execution.cpp b/parse_execution.cpp index 8a2e9d271..46df733ce 100644 --- a/parse_execution.cpp +++ b/parse_execution.cpp @@ -547,13 +547,15 @@ parse_execution_result_t parse_execution_context_t::run_switch_statement(const p _(L"switch: Expected exactly one argument, got %lu\n"), switch_values_expanded.size()); } - const wcstring &switch_value_expanded = switch_values_expanded.at(0).completion; - - switch_block_t *sb = new switch_block_t(); - parser->push_block(sb); - + if (result == parse_execution_success) { + const wcstring &switch_value_expanded = switch_values_expanded.at(0).completion; + + switch_block_t *sb = new switch_block_t(); + parser->push_block(sb); + + /* Expand case statements */ const parse_node_t *case_item_list = get_child(statement, 3, symbol_case_item_list); @@ -597,16 +599,16 @@ parse_execution_result_t parse_execution_context_t::run_switch_statement(const p } } } - } - if (result == parse_execution_success && matching_case_item != NULL) - { - /* Success, evaluate the job list */ - const parse_node_t *job_list = get_child(*matching_case_item, 3, symbol_job_list); - result = this->run_job_list(*job_list, sb); - } + if (result == parse_execution_success && matching_case_item != NULL) + { + /* Success, evaluate the job list */ + const parse_node_t *job_list = get_child(*matching_case_item, 3, symbol_job_list); + result = this->run_job_list(*job_list, sb); + } - parser->pop_block(sb); + parser->pop_block(sb); + } return result; } From 7248b2213d53b34747b89594f563e1c32dd9a835 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Fri, 28 Mar 2014 17:09:08 -0700 Subject: [PATCH 06/10] Fix switch statement syntax highlighting so that the arguemnt to switch is colored as a parameter, not a command. Promote this from a tok_string to a symbol_argument in the grammar too. --- highlight.cpp | 10 +++++++++- parse_execution.cpp | 2 +- parse_productions.cpp | 2 +- parse_tree.h | 2 +- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/highlight.cpp b/highlight.cpp index d03a60873..eeda753ea 100644 --- a/highlight.cpp +++ b/highlight.cpp @@ -1331,7 +1331,6 @@ const highlighter_t::color_array_t & highlighter_t::highlight() case symbol_if_clause: case symbol_else_clause: case symbol_case_item: - case symbol_switch_statement: case symbol_boolean_statement: case symbol_decorated_statement: case symbol_if_statement: @@ -1340,6 +1339,15 @@ const highlighter_t::color_array_t & highlighter_t::highlight() } break; + case symbol_switch_statement: + { + const parse_node_t *literal_switch = this->parse_tree.get_child(node, 0, parse_token_type_string); + const parse_node_t *switch_arg = this->parse_tree.get_child(node, 1, symbol_argument); + this->color_node(*literal_switch, highlight_spec_command); + this->color_node(*switch_arg, highlight_spec_param); + } + break; + case symbol_for_header: { // Color the 'for' and 'in' as commands diff --git a/parse_execution.cpp b/parse_execution.cpp index 46df733ce..1da9858fe 100644 --- a/parse_execution.cpp +++ b/parse_execution.cpp @@ -509,7 +509,7 @@ parse_execution_result_t parse_execution_context_t::run_switch_statement(const p parse_execution_result_t result = parse_execution_success; /* Get the switch variable */ - const parse_node_t &switch_value_node = *get_child(statement, 1, parse_token_type_string); + const parse_node_t &switch_value_node = *get_child(statement, 1, symbol_argument); const wcstring switch_value = get_source(switch_value_node); /* Expand it. We need to offset any errors by the position of the string */ diff --git a/parse_productions.cpp b/parse_productions.cpp index 0fd754354..221316b38 100644 --- a/parse_productions.cpp +++ b/parse_productions.cpp @@ -237,7 +237,7 @@ RESOLVE(else_continuation) PRODUCTIONS(switch_statement) = { - { KEYWORD(parse_keyword_switch), parse_token_type_string, parse_token_type_end, symbol_case_item_list, symbol_end_command, symbol_arguments_or_redirections_list} + { KEYWORD(parse_keyword_switch), symbol_argument, parse_token_type_end, symbol_case_item_list, symbol_end_command, symbol_arguments_or_redirections_list} }; RESOLVE_ONLY(switch_statement) diff --git a/parse_tree.h b/parse_tree.h index c0e24a5ca..0e1a0b2fa 100644 --- a/parse_tree.h +++ b/parse_tree.h @@ -220,7 +220,7 @@ bool parse_tree_from_string(const wcstring &str, parse_tree_flags_t flags, parse else_continuation = if_clause else_clause | job_list - switch_statement = SWITCH case_item_list end_command arguments_or_redirections_list + switch_statement = SWITCH argument case_item_list end_command arguments_or_redirections_list case_item_list = | case_item case_item_list | case_item_list From aa1b065dd1c03f7c0867d002badcf0dc88feb3c4 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Fri, 28 Mar 2014 23:22:03 -0700 Subject: [PATCH 07/10] Allow appending path hints to history items after they have been added, allowing us to avoid the delay before items appear in history. Should fix #984 --- fish_tests.cpp | 5 +- history.cpp | 189 ++++++++++++++++++++++++++++++++++++------------- history.h | 51 ++++++++----- reader.cpp | 2 +- 4 files changed, 178 insertions(+), 69 deletions(-) diff --git a/fish_tests.cpp b/fish_tests.cpp index 3cca2a0c0..92bd0f717 100644 --- a/fish_tests.cpp +++ b/fish_tests.cpp @@ -2203,7 +2203,7 @@ void history_tests_t::test_history(void) test_history_matches(search3, 0); /* Test history escaping and unescaping, yaml, etc. */ - std::vector before, after; + history_item_list_t before, after; history.clear(); size_t i, max = 100; for (i=1; i <= max; i++) @@ -2225,7 +2225,8 @@ void history_tests_t::test_history(void) } /* Record this item */ - history_item_t item(value, time(NULL), paths); + history_item_t item(value, time(NULL)); + item.required_paths = paths; before.push_back(item); history.add(item); } diff --git a/history.cpp b/history.cpp index f8183f5d8..de4056998 100644 --- a/history.cpp +++ b/history.cpp @@ -22,6 +22,7 @@ #include "sanity.h" #include "tokenizer.h" #include "reader.h" +#include "parse_tree.h" #include "wutil.h" #include "history.h" @@ -235,7 +236,7 @@ static void escape_yaml(std::string &str); /** Undoes escape_yaml */ static void unescape_yaml(std::string &str); -/* We can merge two items if they are the same command. We use the more recent timestamp and the longer list of required paths. */ +/* We can merge two items if they are the same command. We use the more recent timestamp, more recent identifier, and the longer list of required paths. */ bool history_item_t::merge(const history_item_t &item) { bool result = false; @@ -246,16 +247,20 @@ bool history_item_t::merge(const history_item_t &item) { this->required_paths = item.required_paths; } + if (this->identifier < item.identifier) + { + this->identifier = item.identifier; + } result = true; } return result; } -history_item_t::history_item_t(const wcstring &str) : contents(str), creation_timestamp(time(NULL)) +history_item_t::history_item_t(const wcstring &str) : contents(str), creation_timestamp(time(NULL)), identifier(0) { } -history_item_t::history_item_t(const wcstring &str, time_t when, const path_list_t &paths) : contents(str), creation_timestamp(when), required_paths(paths) +history_item_t::history_item_t(const wcstring &str, time_t when, history_identifier_t ident) : contents(str), creation_timestamp(when), identifier(ident) { } @@ -543,6 +548,7 @@ history_t & history_t::history_with_name(const wcstring &name) history_t::history_t(const wcstring &pname) : name(pname), first_unwritten_new_item_index(0), + disable_automatic_save_counter(0), mmap_start(NULL), mmap_length(0), mmap_file_id(kInvalidFileID), @@ -572,8 +578,21 @@ void history_t::add(const history_item_t &item) { /* We have to add a new item */ new_items.push_back(item); + save_internal_unless_disabled(); } +} +void history_t::save_internal_unless_disabled() +{ + /* This must be called while locked */ + ASSERT_IS_LOCKED(lock); + + /* Respect disable_automatic_save_counter */ + if (disable_automatic_save_counter > 0) + { + return; + } + /* We may or may not vacuum. We try to vacuum every kVacuumFrequency items, but start the countdown at a random number so that even if the user never runs more than 25 commands, we'll eventually vacuum. If countdown_to_vacuum is -1, it means we haven't yet picked a value for the counter. */ const int kVacuumFrequency = 25; if (countdown_to_vacuum < 0) @@ -582,7 +601,7 @@ void history_t::add(const history_item_t &item) /* Generate a number in the range [0, kVacuumFrequency) */ countdown_to_vacuum = rand_r(&seed) / (RAND_MAX / kVacuumFrequency + 1); } - + /* Determine if we're going to vacuum */ bool vacuum = false; if (countdown_to_vacuum == 0) @@ -590,18 +609,17 @@ void history_t::add(const history_item_t &item) countdown_to_vacuum = kVacuumFrequency; vacuum = true; } - + /* This might be a good candidate for moving to a background thread */ time_profiler_t profiler(vacuum ? "save_internal vacuum" : "save_internal no vacuum"); this->save_internal(vacuum); - + /* Update our countdown */ assert(countdown_to_vacuum > 0); countdown_to_vacuum--; - } -void history_t::add(const wcstring &str, const path_list_t &valid_paths) +void history_t::add(const wcstring &str, history_identifier_t ident) { time_t when = time(NULL); /* Big hack: do not allow timestamps equal to our birthdate. This is because we include items whose timestamps are equal to our birthdate when reading old history, so we can catch "just closed" items. But this means that we may interpret our own items, that we just wrote, as old items, if we wrote them in the same second as our birthdate. @@ -609,7 +627,7 @@ void history_t::add(const wcstring &str, const path_list_t &valid_paths) if (when == this->birth_timestamp) when++; - this->add(history_item_t(str, when, valid_paths)); + this->add(history_item_t(str, when, ident)); } void history_t::remove(const wcstring &str) @@ -621,7 +639,7 @@ void history_t::remove(const wcstring &str) size_t idx = new_items.size(); while (idx--) { - if (new_items[idx].str() == str) + if (new_items.at(idx).str() == str) { new_items.erase(new_items.begin() + idx); @@ -635,6 +653,28 @@ void history_t::remove(const wcstring &str) assert(first_unwritten_new_item_index <= new_items.size()); } +void history_t::set_valid_file_paths(const wcstring_list_t &valid_file_paths, history_identifier_t ident) +{ + /* 0 identifier is used to mean "not necessary" */ + if (ident == 0) + { + return; + } + + scoped_lock locker(lock); + + /* Look for an item with the given identifier. It is likely to be at the end of new_items */ + for (history_item_list_t::reverse_iterator iter = new_items.rbegin(); iter != new_items.rend(); iter++) + { + if (iter->identifier == ident) + { + /* Found it */ + iter->required_paths = valid_file_paths; + break; + } + } +} + void history_t::get_string_representation(wcstring &result, const wcstring &separator) { scoped_lock locker(lock); @@ -644,7 +684,7 @@ void history_t::get_string_representation(wcstring &result, const wcstring &sepa std::set seen; /* Append new items. Note that in principle we could use const_reverse_iterator, but we do not because reverse_iterator is not convertible to const_reverse_iterator ( http://github.com/fish-shell/fish-shell/issues/431 ) */ - for (std::vector::reverse_iterator iter=new_items.rbegin(); iter < new_items.rend(); ++iter) + for (history_item_list_t::reverse_iterator iter=new_items.rbegin(); iter < new_items.rend(); ++iter) { /* Skip duplicates */ if (! seen.insert(iter->str()).second) @@ -658,7 +698,7 @@ void history_t::get_string_representation(wcstring &result, const wcstring &sepa /* Append old items */ load_old_if_needed(); - for (std::vector::reverse_iterator iter = old_item_offsets.rbegin(); iter != old_item_offsets.rend(); ++iter) + for (std::deque::reverse_iterator iter = old_item_offsets.rbegin(); iter != old_item_offsets.rend(); ++iter) { size_t offset = *iter; const history_item_t item = history_t::decode_item(mmap_start + offset, mmap_length - offset, mmap_type); @@ -825,7 +865,9 @@ history_item_t history_t::decode_item_fish_2_0(const char *base, size_t len) } } done: - return history_item_t(cmd, when, paths); + history_item_t result(cmd, when); + result.required_paths.swap(paths); + return result; } history_item_t history_t::decode_item(const char *base, size_t len, history_file_type_t type) @@ -1280,7 +1322,7 @@ bool history_t::save_internal_via_rewrite() history_lru_cache_t lru(HISTORY_SAVE_MAX); /* Insert old items in, from old to new. Merge them with our new items, inserting items with earlier timestamps first. */ - std::vector::const_iterator new_item_iter = new_items.begin(); + history_item_list_t::const_iterator new_item_iter = new_items.begin(); /* Map in existing items (which may have changed out from underneath us, so don't trust our old mmap'd data) */ const char *local_mmap_start = NULL; @@ -1526,6 +1568,22 @@ void history_t::save_and_vacuum(void) this->save_internal(true); } +void history_t::disable_automatic_saving() +{ + scoped_lock locker(lock); + disable_automatic_save_counter++; + assert(disable_automatic_save_counter != 0); // overflow! +} + +void history_t::enable_automatic_saving() +{ + scoped_lock locker(lock); + assert(disable_automatic_save_counter > 0); //underflow + disable_automatic_save_counter--; + save_internal_unless_disabled(); +} + + void history_t::clear(void) { scoped_lock locker(lock); @@ -1693,11 +1751,10 @@ bool file_detection_context_t::paths_are_valid(const path_list_t &paths) return perform_file_detection(false) > 0; } -file_detection_context_t::file_detection_context_t(history_t *hist, const wcstring &cmd) : +file_detection_context_t::file_detection_context_t(history_t *hist, history_identifier_t ident) : history(hist), - command(cmd), - when(time(NULL)), - working_directory(env_get_pwd_slash()) + working_directory(env_get_pwd_slash()), + history_item_identifier(ident) { } @@ -1710,8 +1767,13 @@ static int threaded_perform_file_detection(file_detection_context_t *ctx) static void perform_file_detection_done(file_detection_context_t *ctx, int success) { - /* Now that file detection is done, create the history item */ - ctx->history->add(ctx->command, ctx->valid_paths); + ASSERT_IS_MAIN_THREAD(); + + /* Now that file detection is done, update the history item with the valid file paths */ + ctx->history->set_valid_file_paths(ctx->valid_paths, ctx->history_item_identifier); + + /* Allow saving again */ + ctx->history->enable_automatic_saving(); /* Done with the context. */ delete ctx; @@ -1721,7 +1783,9 @@ static bool string_could_be_path(const wcstring &potential_path) { // Assume that things with leading dashes aren't paths if (potential_path.empty() || potential_path.at(0) == L'-') + { return false; + } return true; } @@ -1729,44 +1793,73 @@ void history_t::add_with_file_detection(const wcstring &str) { ASSERT_IS_MAIN_THREAD(); path_list_t potential_paths; - - /* Hack hack hack - if the command is likely to trigger an exit, then don't do background file detection, because we won't be able to write it to our history file before we exit. */ + + /* Find all arguments that look like they could be file paths */ bool impending_exit = false; - - tokenizer_t tokenizer(str.c_str(), TOK_SQUASH_ERRORS); - for (; tok_has_next(&tokenizer); tok_next(&tokenizer)) + parse_node_tree_t tree; + parse_tree_from_string(str, parse_flag_none, &tree, NULL); + size_t count = tree.size(); + + for (size_t i=0; i < count; i++) { - int type = tok_last_type(&tokenizer); - if (type == TOK_STRING) + const parse_node_t &node = tree.at(i); + if (! node.has_source()) { - const wchar_t *token_cstr = tok_last(&tokenizer); - if (token_cstr) + continue; + } + + if (node.type == symbol_argument) + { + wcstring potential_path = node.get_source(str); + bool unescaped = unescape_string_in_place(&potential_path, UNESCAPE_DEFAULT); + if (unescaped && string_could_be_path(potential_path)) { - wcstring potential_path; - bool unescaped = unescape_string(token_cstr, &potential_path, UNESCAPE_DEFAULT); - if (unescaped && string_could_be_path(potential_path)) - { - potential_paths.push_back(potential_path); - - /* What a hack! */ - impending_exit = impending_exit || contains(potential_path, L"exec", L"exit", L"reboot"); - } + potential_paths.push_back(potential_path); + } + } + else if (node.type == symbol_plain_statement) + { + /* Hack hack hack - if the command is likely to trigger an exit, then don't do background file detection, because we won't be able to write it to our history file before we exit. */ + if (tree.decoration_for_plain_statement(node) == parse_statement_decoration_exec) + { + impending_exit = true; + } + + wcstring command; + tree.command_for_plain_statement(node, str, &command); + unescape_string_in_place(&command, UNESCAPE_DEFAULT); + if (contains(command, L"exit", L"reboot")) + { + impending_exit = true; } } } - if (potential_paths.empty() || impending_exit) + /* If we got a path, we'll perform file detection for autosuggestion hinting */ + history_identifier_t identifier = 0; + if (! potential_paths.empty() && ! impending_exit) { - this->add(str); - } - else - { - /* We have some paths. Make a context. */ - file_detection_context_t *context = new file_detection_context_t(this, str); - - /* Store the potential paths. Reverse them to put them in the same order as in the command. */ + /* Grab the next identifier */ + static history_identifier_t sLastIdentifier = 0; + identifier = ++sLastIdentifier; + + /* Create a new detection context */ + file_detection_context_t *context = new file_detection_context_t(this, identifier); context->potential_paths.swap(potential_paths); + + /* Prevent saving until we're done, so we have time to get the paths */ + this->disable_automatic_saving(); + + /* Kick it off. Even though we haven't added the item yet, it updates the item on the main thread, so we can't race */ iothread_perform(threaded_perform_file_detection, perform_file_detection_done, context); } -} + /* Actually add the item to the history */ + this->add(str, identifier); + + /* If we think we're about to exit, save immediately, regardless of any disabling. This may cause us to lose file hinting for some commands, but it beats losing history items */ + if (impending_exit) + { + this->save(); + } +} diff --git a/history.h b/history.h index 4b08da678..290d91a89 100644 --- a/history.h +++ b/history.h @@ -9,6 +9,7 @@ #include "common.h" #include "pthread.h" #include "wutil.h" +#include #include #include #include @@ -34,6 +35,8 @@ enum history_search_type_t HISTORY_SEARCH_TYPE_PREFIX }; +typedef uint32_t history_identifier_t; + class history_item_t { friend class history_t; @@ -41,8 +44,8 @@ class history_item_t friend class history_tests_t; private: - explicit history_item_t(const wcstring &); - explicit history_item_t(const wcstring &, time_t, const path_list_t &paths = path_list_t()); + explicit history_item_t(const wcstring &str); + explicit history_item_t(const wcstring &, time_t, history_identifier_t ident = 0); /** Attempts to merge two compatible history items together */ bool merge(const history_item_t &item); @@ -53,9 +56,12 @@ private: /** Original creation time for the entry */ time_t creation_timestamp; + /** Sometimes unique identifier used for hinting */ + history_identifier_t identifier; + /** Paths that we require to be valid for this item to be autosuggested */ path_list_t required_paths; - + public: const wcstring &str() const { @@ -88,6 +94,8 @@ public: } }; +typedef std::deque history_item_list_t; + /* The type of file that we mmap'd */ enum history_file_type_t { @@ -123,10 +131,13 @@ private: const wcstring name; /** New items. Note that these are NOT discarded on save. We need to keep these around so we can distinguish between items in our history and items in the history of other shells that were started after we were started. */ - std::vector new_items; + history_item_list_t new_items; /** The index of the first new item that we have not yet written. */ size_t first_unwritten_new_item_index; + + /** Whether we should disable saving to the file for a time */ + uint32_t disable_automatic_save_counter; /** Deleted item contents. */ std::set deleted_items; @@ -153,7 +164,7 @@ private: void populate_from_mmap(void); /** List of old items, as offsets into out mmap data */ - std::vector old_item_offsets; + std::deque old_item_offsets; /** Whether we've loaded old items */ bool loaded_old; @@ -175,6 +186,9 @@ private: /** Saves history */ void save_internal(bool vacuum); + + /** Saves history, maybe */ + void save_internal_unless_disabled(); /* Do a private, read-only map of the entirety of a history file with the given name. Returns true if successful. Returns the mapped memory region by reference. */ bool map_file(const wcstring &name, const char **out_map_start, size_t *out_map_len, file_id_t *file_id); @@ -195,7 +209,7 @@ public: bool is_empty(void); /** Add a new history item to the end */ - void add(const wcstring &str, const path_list_t &valid_paths = path_list_t()); + void add(const wcstring &str, history_identifier_t ident = 0); /** Remove a history item */ void remove(const wcstring &str); @@ -205,9 +219,13 @@ public: /** Saves history */ void save(); - + /** Performs a full (non-incremental) save */ void save_and_vacuum(); + + /** Enable / disable automatic saving. Main thread only! */ + void disable_automatic_saving(); + void enable_automatic_saving(); /** Irreversibly clears history */ void clear(); @@ -217,6 +235,9 @@ public: /* Gets all the history into a string with ARRAY_SEP_STR. This is intended for the $history environment variable. This may be long! */ void get_string_representation(wcstring &str, const wcstring &separator); + + /** Sets the valid file paths for the history item with the given identifier */ + void set_valid_file_paths(const wcstring_list_t &valid_file_paths, history_identifier_t ident); /** Return the specified history at the specified index. 0 is the index of the current commandline. (So the most recent item is at index 1.) */ history_item_t item_at_index(size_t idx); @@ -295,8 +316,6 @@ public: }; - - /** Init history library. The history file won't actually be loaded until the first time a history search is performed. @@ -316,21 +335,14 @@ void history_sanity_check(); /* A helper class for threaded detection of paths */ struct file_detection_context_t { - /* Constructor */ - file_detection_context_t(history_t *hist, const wcstring &cmd); + file_detection_context_t(history_t *hist, history_identifier_t ident = 0); /* Determine which of potential_paths are valid, and put them in valid_paths */ int perform_file_detection(); /* The history associated with this context */ - history_t *history; - - /* The command */ - wcstring command; - - /* When the command was issued */ - time_t when; + history_t * const history; /* The working directory at the time the command was issued */ wcstring working_directory; @@ -340,6 +352,9 @@ struct file_detection_context_t /* Paths that were found to be valid */ path_list_t valid_paths; + + /* Identifier of the history item to which we are associated */ + const history_identifier_t history_item_identifier; /* Performs file detection. Returns 1 if every path in potential_paths is valid, 0 otherwise. If test_all is true, tests every path; otherwise stops as soon as it reaches an invalid path. */ int perform_file_detection(bool test_all); diff --git a/reader.cpp b/reader.cpp index 3bf970800..69883e16f 100644 --- a/reader.cpp +++ b/reader.cpp @@ -1353,7 +1353,7 @@ struct autosuggestion_context_t search_string(term), cursor_pos(pos), searcher(*history, term, HISTORY_SEARCH_TYPE_PREFIX), - detector(history, term), + detector(history), working_directory(env_get_pwd_slash()), vars(env_vars_snapshot_t::highlighting_keys), generation_count(s_generation_count) From 1270384ede4ec1963c4a45e4083dabcc0cfabd87 Mon Sep 17 00:00:00 2001 From: Knut Ahlers Date: Sat, 29 Mar 2014 11:46:45 +0100 Subject: [PATCH 08/10] Fixed appearance of ssh hostnames with [] in them refs https://github.com/fish-shell/fish-shell/issues/1355 --- share/functions/__fish_print_hostnames.fish | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/share/functions/__fish_print_hostnames.fish b/share/functions/__fish_print_hostnames.fish index dd62b5c87..1e9f81433 100644 --- a/share/functions/__fish_print_hostnames.fish +++ b/share/functions/__fish_print_hostnames.fish @@ -14,7 +14,7 @@ function __fish_print_hostnames -d "Print a list of known hostnames" # Print hosts with known ssh keys # Does not match hostnames with @directives specified - sgrep -Eoh '^[^#@|, ]*' ~/.ssh/known_hosts{,2} ^/dev/null + sgrep -Eoh '^[^#@|, ]*' ~/.ssh/known_hosts{,2} ^/dev/null | sed -E 's/^\[([^]]+)\]:([0-9]+)$/\1/' # Print hosts from system wide ssh configuration file if [ -e /etc/ssh/ssh_config ] From 42813eeb841ea4a5dbb94e35fb481a163b486d77 Mon Sep 17 00:00:00 2001 From: David Adam Date: Sat, 29 Mar 2014 23:28:11 +0800 Subject: [PATCH 09/10] configure/Makefile: remove unused $LIBS and $LDFLAGS complications --- Makefile.in | 19 +++++++++---------- configure.ac | 35 ----------------------------------- 2 files changed, 9 insertions(+), 45 deletions(-) diff --git a/Makefile.in b/Makefile.in index 3063438c2..a39b4bb56 100644 --- a/Makefile.in +++ b/Makefile.in @@ -54,11 +54,10 @@ localedir = @localedir@ MACROS = -DLOCALEDIR=\"$(localedir)\" -DPREFIX=L\"$(prefix)\" -DDATADIR=L\"$(datadir)\" -DSYSCONFDIR=L\"$(sysconfdir)\" -DBINDIR=L\"$(bindir)\" -DDOCDIR=L\"$(docdir)\" CXXFLAGS = @CXXFLAGS@ $(MACROS) $(EXTRA_CXXFLAGS) +CPPFLAGS = @CPPFLAGS@ LDFLAGS = @LDFLAGS@ -LDFLAGS_FISH = ${LDFLAGS} @LIBS_FISH@ @LDFLAGS_FISH@ -LDFLAGS_FISH_INDENT = ${LDFLAGS} @LIBS_FISH_INDENT@ -LDFLAGS_FISHD = ${LDFLAGS} @LIBS_FISHD@ -LDFLAGS_MIMEDB = ${LDFLAGS} @LIBS_MIMEDB@ +LIBS = @LIBS@ +LDFLAGS_FISH = ${LDFLAGS} @LDFLAGS_FISH@ # # Set to 1 if we have gettext @@ -686,7 +685,7 @@ uninstall-translations: # fish: $(FISH_OBJS) fish.o - $(CXX) $(CXXFLAGS) $(FISH_OBJS) fish.o $(LDFLAGS_FISH) -o $@ + $(CXX) $(CXXFLAGS) $(LDFLAGS_FISH) $(FISH_OBJS) fish.o $(LIBS) -o $@ # @@ -694,7 +693,7 @@ fish: $(FISH_OBJS) fish.o # fishd: $(FISHD_OBJS) - $(CXX) $(CXXFLAGS) $(FISHD_OBJS) $(LDFLAGS_FISHD) -o $@ + $(CXX) $(CXXFLAGS) $(LDFLAGS) $(FISHD_OBJS) $(LIBS) -o $@ # @@ -702,7 +701,7 @@ fishd: $(FISHD_OBJS) # fish_tests: $(FISH_TESTS_OBJS) - $(CXX) $(CXXFLAGS) $(FISH_TESTS_OBJS) $(LDFLAGS_FISH) -o $@ + $(CXX) $(CXXFLAGS) $(LDFLAGS_FISH) $(FISH_TESTS_OBJS) $(LIBS) -o $@ # @@ -710,7 +709,7 @@ fish_tests: $(FISH_TESTS_OBJS) # mimedb: $(MIME_OBJS) - $(CXX) $(CXXFLAGS) $(MIME_OBJS) $(LDFLAGS_MIMEDB) -o $@ + $(CXX) $(CXXFLAGS) $(LDFLAGS) $(MIME_OBJS) $(LIBS) -o $@ # @@ -718,7 +717,7 @@ mimedb: $(MIME_OBJS) # fish_indent: $(FISH_INDENT_OBJS) - $(CXX) $(CXXFLAGS) $(FISH_INDENT_OBJS) $(LDFLAGS_FISH_INDENT) -o $@ + $(CXX) $(CXXFLAGS) $(LDFLAGS) $(FISH_INDENT_OBJS) $(LIBS) -o $@ # @@ -726,7 +725,7 @@ fish_indent: $(FISH_INDENT_OBJS) # key_reader: key_reader.o input_common.o common.o env_universal.o env_universal_common.o wutil.o iothread.o - $(CXX) $(CXXFLAGS) key_reader.o input_common.o common.o env_universal.o env_universal_common.o wutil.o iothread.o $(LDFLAGS_FISH) -o $@ + $(CXX) $(CXXFLAGS) $(LDFLAGS_FISH) key_reader.o input_common.o common.o env_universal.o env_universal_common.o wutil.o iothread.o $(LIBS) -o $@ # diff --git a/configure.ac b/configure.ac index 91f4f88cb..ccabf62db 100644 --- a/configure.ac +++ b/configure.ac @@ -24,10 +24,6 @@ conf_arg=$@ AC_SUBST(HAVE_GETTEXT) AC_SUBST(HAVE_DOXYGEN) AC_SUBST(LDFLAGS_FISH) -AC_SUBST(LIBS_FISH) -AC_SUBST(LIBS_FISH_INDENT) -AC_SUBST(LIBS_FISHD) -AC_SUBST(LIBS_MIMEDB) # @@ -414,37 +410,6 @@ if test x$local_gettext != xno; then AC_SEARCH_LIBS( gettext, intl,,) fi -LIBS_SHARED=$LIBS - -# -# Check for libraries needed by fish. -# - -LIBS="$LIBS_SHARED" -LIBS_FISH=$LIBS - -# -# Check for libraries needed by fish_indent. -# - -LIBS="$LIBS_SHARED" -LIBS_FISH_INDENT=$LIBS - -# -# Check for libraries needed by fishd. -# - -LIBS="$LIBS_SHARED" -LIBS_FISHD=$LIBS - -# -# Check for libraries needed by mimedb. -# - -LIBS="$LIBS_SHARED" -LIBS_MIMEDB=$LIBS - - # # Check presense of various header files # From 1177daecded5b8dac03097801d4d6111330e1cff Mon Sep 17 00:00:00 2001 From: David Adam Date: Sun, 30 Mar 2014 13:05:04 +0800 Subject: [PATCH 10/10] configure: turn off automatic searches through non-standard directories configure will no longer check for the existence of extra include, lib and bin directories in /usr/pkg /sw /opt /opt/local /usr/local. The check was not done in a particularly sensible manner and there are now no mandatory dependencies that not shipped in the main system trees on virtually every system in existence. If building with Fink, follow these directions as suggested by the fink project: http://www.finkproject.org/faq/usage-general.php#compile-myself Closes #1185, and closes #1186. --- configure.ac | 37 ------------------------------------- 1 file changed, 37 deletions(-) diff --git a/configure.ac b/configure.ac index ccabf62db..23c5b16a0 100644 --- a/configure.ac +++ b/configure.ac @@ -94,43 +94,6 @@ AC_LANG(C++) echo "CXXFLAGS: $CXXFLAGS" -# -# Detect directories which may contain additional headers, libraries -# and commands. This needs to be done early - before Autoconf starts -# to mess with CXXFLAGS and all the other environemnt variables. -# -# This mostly helps OS X users, since fink usually installs out of -# tree and doesn't update CXXFLAGS. - -for i in /usr/pkg /sw /opt /opt/local /usr/local; do - - AC_MSG_CHECKING([for $i/include include directory]) - if test -d $i/include; then - AC_MSG_RESULT(yes) - CXXFLAGS="$CXXFLAGS -I$i/include/" - else - AC_MSG_RESULT(no) - fi - - AC_MSG_CHECKING([for $i/lib library directory]) - if test -d $i/lib; then - AC_MSG_RESULT(yes) - LDFLAGS="$LDFLAGS -L$i/lib/" - else - AC_MSG_RESULT(no) - fi - - AC_MSG_CHECKING([for $i/bin command directory]) - if test -d $i/bin; then - AC_MSG_RESULT(yes) - optbindirs="$optbindirs $i/bin" - else - AC_MSG_RESULT(no) - fi - -done - - # # Tell autoconf to create config.h header #