Fix for issue where comments are lost in function definitions

Fixes #1710
This commit is contained in:
ridiculousfish 2014-09-27 17:32:54 -07:00
parent 4f4734fc9b
commit b00cbae4b5
2 changed files with 34 additions and 8 deletions

View file

@ -363,11 +363,11 @@ parse_execution_result_t parse_execution_context_t::run_begin_statement(const pa
} }
/* Define a function */ /* Define a function */
parse_execution_result_t parse_execution_context_t::run_function_statement(const parse_node_t &header, const parse_node_t &contents) parse_execution_result_t parse_execution_context_t::run_function_statement(const parse_node_t &header, const parse_node_t &block_end_command)
{ {
assert(header.type == symbol_function_header); assert(header.type == symbol_function_header);
assert(contents.type == symbol_job_list); assert(block_end_command.type == symbol_end_command);
/* Get arguments */ /* Get arguments */
const parse_node_t *unmatched_wildcard = NULL; const parse_node_t *unmatched_wildcard = NULL;
wcstring_list_t argument_list; wcstring_list_t argument_list;
@ -382,8 +382,25 @@ parse_execution_result_t parse_execution_context_t::run_function_statement(const
if (result == parse_execution_success) if (result == parse_execution_success)
{ {
const wcstring contents_str = get_source(contents); /* The function definition extends from the end of the header to the function end. It's not just the range of the contents because that loses comments - see issue #1710 */
int definition_line_offset = this->line_offset_of_node_at_offset(this->get_offset(contents)); assert(block_end_command.has_source());
size_t contents_start = header.source_start + header.source_length;
size_t contents_end = block_end_command.source_start; // 1 past the last character in the function definition
assert(contents_end >= contents_start);
// Swallow whitespace at both ends
while (contents_start < contents_end && iswspace(this->src.at(contents_start)))
{
contents_start++;
}
while (contents_start < contents_end && iswspace(this->src.at(contents_end - 1)))
{
contents_end--;
}
assert(contents_end >= contents_start);
const wcstring contents_str = wcstring(this->src, contents_start, contents_end - contents_start);
int definition_line_offset = this->line_offset_of_character_at_offset(contents_start);
wcstring error_str; wcstring error_str;
int err = define_function(*parser, argument_list, contents_str, definition_line_offset, &error_str); int err = define_function(*parser, argument_list, contents_str, definition_line_offset, &error_str);
proc_set_last_status(err); proc_set_last_status(err);
@ -418,8 +435,11 @@ parse_execution_result_t parse_execution_context_t::run_block_statement(const pa
break; break;
case symbol_function_header: case symbol_function_header:
ret = run_function_statement(header, contents); {
const parse_node_t &function_end = *get_child(statement, 3, symbol_end_command); //the 'end' associated with the block
ret = run_function_statement(header, function_end);
break; break;
}
case symbol_begin_header: case symbol_begin_header:
ret = run_begin_statement(header, contents); ret = run_begin_statement(header, contents);
@ -1575,9 +1595,14 @@ int parse_execution_context_t::line_offset_of_node_at_offset(node_offset_t reque
{ {
return -1; return -1;
} }
size_t char_offset = tree.at(requested_index).source_start;
return this->line_offset_of_character_at_offset(char_offset);
}
int parse_execution_context_t::line_offset_of_character_at_offset(size_t offset)
{
/* Count the number of newlines, leveraging our cache */ /* Count the number of newlines, leveraging our cache */
const size_t offset = tree.at(requested_index).source_start;
assert(offset <= src.size()); assert(offset <= src.size());
/* Easy hack to handle 0 */ /* Easy hack to handle 0 */

View file

@ -97,7 +97,7 @@ private:
parse_execution_result_t run_if_statement(const parse_node_t &statement); parse_execution_result_t run_if_statement(const parse_node_t &statement);
parse_execution_result_t run_switch_statement(const parse_node_t &statement); parse_execution_result_t run_switch_statement(const parse_node_t &statement);
parse_execution_result_t run_while_statement(const parse_node_t &header, const parse_node_t &contents); parse_execution_result_t run_while_statement(const parse_node_t &header, const parse_node_t &contents);
parse_execution_result_t run_function_statement(const parse_node_t &header, const parse_node_t &contents); parse_execution_result_t run_function_statement(const parse_node_t &header, const parse_node_t &block_end_command);
parse_execution_result_t run_begin_statement(const parse_node_t &header, const parse_node_t &contents); parse_execution_result_t run_begin_statement(const parse_node_t &header, const parse_node_t &contents);
parse_execution_result_t determine_arguments(const parse_node_t &parent, wcstring_list_t *out_arguments, const parse_node_t **out_unmatched_wildcard_node); parse_execution_result_t determine_arguments(const parse_node_t &parent, wcstring_list_t *out_arguments, const parse_node_t **out_unmatched_wildcard_node);
@ -111,6 +111,7 @@ private:
/* Returns the line number of the node at the given index, indexed from 0. Not const since it touches cached_lineno_offset */ /* Returns the line number of the node at the given index, indexed from 0. Not const since it touches cached_lineno_offset */
int line_offset_of_node_at_offset(node_offset_t idx); int line_offset_of_node_at_offset(node_offset_t idx);
int line_offset_of_character_at_offset(size_t char_idx);
public: public:
parse_execution_context_t(const parse_node_tree_t &t, const wcstring &s, parser_t *p, int initial_eval_level); parse_execution_context_t(const parse_node_tree_t &t, const wcstring &s, parser_t *p, int initial_eval_level);