Autosuggestions to validate the first command, not the last command

When considering an autosuggestion from history, we attempt to validate the
command to ensure that we don't suggest invalid (e.g. path-dependent)
commands. Prior to this fix, we would validate the last command in the
command line (e.g. in `cd /bin && ./stuff` we would validate "./stuff".
This doesn't really make sense; we should be validating the first command
because it has the potential to change the PWD. Switch to validating the
first command.

Also remove some helper functions that became dead through this change.
This commit is contained in:
ridiculousfish 2019-11-02 13:40:31 -07:00
parent 4dbb209421
commit 151e75d141
4 changed files with 18 additions and 42 deletions

View file

@ -395,23 +395,31 @@ static bool has_expand_reserved(const wcstring &str) {
return result;
}
// Parse a command line. Return by reference the last command, and the last argument to that command
// (as a string), if any. This is used by autosuggestions.
// Parse a command line. Return by reference the first command, and the first argument to that
// command (as a string), if any. This is used to validate autosuggestions.
static bool autosuggest_parse_command(const wcstring &buff, const environment_t &vars,
wcstring *out_expanded_command, wcstring *out_last_arg) {
wcstring *out_expanded_command, wcstring *out_arg) {
// Parse the buffer.
parse_node_tree_t parse_tree;
parse_tree_from_string(buff,
parse_flag_continue_after_error | parse_flag_accept_incomplete_tokens,
&parse_tree, NULL);
// Find the last statement.
auto last_statement = parse_tree.find_last_node<g::plain_statement>();
if (last_statement &&
plain_statement_get_expanded_command(buff, last_statement, vars, out_expanded_command)) {
// Find the last argument. If we don't get one, return an invalid node.
if (auto last_arg = parse_tree.find_last_node<g::argument>(last_statement)) {
*out_last_arg = last_arg.get_source(buff);
// Find the first statement.
tnode_t<g::plain_statement> first_statement{};
for (const auto &node : parse_tree) {
if (node.type == symbol_plain_statement) {
first_statement = tnode_t<g::plain_statement>(&parse_tree, &node);
break;
}
}
if (first_statement &&
plain_statement_get_expanded_command(buff, first_statement, vars, out_expanded_command)) {
// Find the first argument.
auto args_and_redirs = first_statement.child<1>();
if (auto arg = args_and_redirs.next_in_list<grammar::argument>()) {
*out_arg = arg.get_source(buff);
}
return true;
}

View file

@ -1212,20 +1212,3 @@ const parse_node_t *parse_node_tree_t::find_node_matching_source_location(
return result;
}
const parse_node_t *parse_node_tree_t::find_last_node_of_type(parse_token_type_t type,
const parse_node_t *parent) const {
const parse_node_t *result = NULL;
// Find nodes of the given type in the tree, working backwards.
size_t idx = this->size();
while (idx--) {
const parse_node_t &node = this->at(idx);
bool expected_type = (node.type == type);
if (expected_type && (parent == NULL || node_has_ancestor(*this, node, *parent))) {
// The types match and it has the right parent.
result = &node;
break;
}
}
return result;
}

View file

@ -181,11 +181,6 @@ class parse_node_tree_t : public std::vector<parse_node_t> {
const parse_node_t *get_parent(const parse_node_t &node,
parse_token_type_t expected_type = token_type_invalid) const;
// Finds the last node of a given type, or empty if it could not be found. If parent is NULL,
// this finds the last node in the tree of that type.
template <typename Type>
tnode_t<Type> find_last_node(const parse_node_t *parent = NULL) const;
// Finds a node containing the given source location. If 'parent' is not NULL, it must be an
// ancestor.
const parse_node_t *find_node_matching_source_location(parse_token_type_t type,
@ -205,11 +200,6 @@ class parse_node_tree_t : public std::vector<parse_node_t> {
const parse_node_t *next_node_in_node_list(const parse_node_t &node_list,
parse_token_type_t item_type,
const parse_node_t **list_tail) const;
// Finds the last node of a given type underneath a given node, or NULL if it could not be
// found. If parent is NULL, this finds the last node in the tree of that type.
const parse_node_t *find_last_node_of_type(parse_token_type_t type,
const parse_node_t *parent) const;
};
/// The big entry point. Parse a string, attempting to produce a tree for the given goal type.

View file

@ -223,11 +223,6 @@ tnode_t<Type> parse_node_tree_t::find_child(const parse_node_t &parent) const {
return tnode_t<Type>(this, &this->find_child(parent, Type::token));
}
template <typename Type>
tnode_t<Type> parse_node_tree_t::find_last_node(const parse_node_t *parent) const {
return tnode_t<Type>(this, this->find_last_node_of_type(Type::token, parent));
}
/// Given a plain statement, get the command from the child node. Returns the command string on
/// success, none on failure.
maybe_t<wcstring> command_for_plain_statement(tnode_t<grammar::plain_statement> stmt,