Mirrors/fish-shell
2
0
Fork 0
mirror of https://github.com/fish-shell/fish-shell synced 2024-12-25 12:23:09 +00:00
Code Issues Projects Releases Packages Wiki Activity

fix issues with builtin_function()

Browse source 
This does several things. It fixes `builtin_function()` so that errors it
emits are displayed. As part of doing that I've removed the unnecessary
`out_err` parameter to make the interface like every other builtin.

This also fixes a regression introduced by #4000 which was attempting to
fix a bug introduced by #3649.

Fixes #4139
This commit is contained in:
Kurtis Rader 2017-06-17 22:36:56 -07:00
parent 0e954e4764
commit 385e40540c
7 changed files with 38 additions and 45 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

45
src/builtin_function.cpp
View file

@ -7,6 +7,7 @@
#include <unistd.h>
#include <memory>
#include <string>
#include <vector>
#include "builtin.h"
@ -51,9 +52,8 @@ static const struct woption long_options[] = {{L"description", required_argument
{NULL, 0, NULL, 0}};
static int parse_cmd_opts(function_cmd_opts_t &opts, int *optind, //!OCLINT(high ncss method)
int argc, wchar_t **argv, parser_t &parser, io_streams_t &streams,
wcstring *out_err) {
wchar_t *cmd = argv[0];
int argc, wchar_t **argv, parser_t &parser, io_streams_t &streams) {
const wchar_t *cmd = L"function";
int opt;
wgetopter_t w;
while ((opt = w.wgetopt_long(argc, argv, short_options, long_options, NULL)) != -1) {
@ -65,7 +65,7 @@ static int parse_cmd_opts(function_cmd_opts_t &opts, int *optind, //!OCLINT(hig
case 's': {
int sig = wcs2sig(w.woptarg);
if (sig == -1) {
append_format(*out_err, _(L"%ls: Unknown signal '%ls'"), cmd, w.woptarg);
streams.err.append_format(_(L"%ls: Unknown signal '%ls'"), cmd, w.woptarg);
return STATUS_INVALID_ARGS;
}
opts.events.push_back(event_t::signal_event(sig));
@ -73,7 +73,7 @@ static int parse_cmd_opts(function_cmd_opts_t &opts, int *optind, //!OCLINT(hig
}
case 'v': {
if (!valid_var_name(w.woptarg)) {
append_format(*out_err, BUILTIN_ERR_VARNAME, cmd, w.woptarg);
streams.err.append_format(BUILTIN_ERR_VARNAME, cmd, w.woptarg);
return STATUS_INVALID_ARGS;
}
@ -109,8 +109,7 @@ static int parse_cmd_opts(function_cmd_opts_t &opts, int *optind, //!OCLINT(hig
}
if (job_id == -1) {
append_format(*out_err,
_(L"%ls: Cannot find calling job for event handler"), cmd);
streams.err.append_format(_(L"%ls: Cannot find calling job for event handler"), cmd);
return STATUS_INVALID_ARGS;
}
e.type = EVENT_JOB_ID;
@ -118,7 +117,7 @@ static int parse_cmd_opts(function_cmd_opts_t &opts, int *optind, //!OCLINT(hig
} else {
pid = fish_wcstoi(w.woptarg);
if (errno || pid < 0) {
append_format(*out_err, _(L"%ls: Invalid process id '%ls'"), cmd,
streams.err.append_format(_(L"%ls: Invalid process id '%ls'"), cmd,
w.woptarg);
return STATUS_INVALID_ARGS;
}
@ -143,7 +142,7 @@ static int parse_cmd_opts(function_cmd_opts_t &opts, int *optind, //!OCLINT(hig
}
case 'V': {
if (!valid_var_name(w.woptarg)) {
append_format(*out_err, BUILTIN_ERR_VARNAME, cmd, w.woptarg);
streams.err.append_format(BUILTIN_ERR_VARNAME, cmd, w.woptarg);
return STATUS_INVALID_ARGS;
}
opts.inherit_vars.push_back(w.woptarg);
@ -173,22 +172,21 @@ static int parse_cmd_opts(function_cmd_opts_t &opts, int *optind, //!OCLINT(hig
}
static int validate_function_name(int argc, const wchar_t *const *argv, wcstring &function_name,
const wchar_t *cmd, wcstring *out_err) {
const wchar_t *cmd, io_streams_t &streams) {
if (argc < 2) {
// This is currently impossible but let's be paranoid.
append_format(*out_err, _(L"%ls: Expected function name"), cmd);
streams.err.append_format(_(L"%ls: Expected function name"), cmd);
return STATUS_INVALID_ARGS;
}
function_name = argv[1];
if (!valid_func_name(function_name)) {
append_format(*out_err, _(L"%ls: Illegal function name '%ls'"), cmd, function_name.c_str());
streams.err.append_format(_(L"%ls: Illegal function name '%ls'"), cmd, function_name.c_str());
return STATUS_INVALID_ARGS;
}
if (parser_keywords_is_reserved(function_name)) {
append_format(
*out_err,
streams.err.append_format(
_(L"%ls: The name '%ls' is reserved,\nand can not be used as a function name"), cmd,
function_name.c_str());
return STATUS_INVALID_ARGS;
@ -200,14 +198,11 @@ static int validate_function_name(int argc, const wchar_t *const *argv, wcstring
/// Define a function. Calls into `function.cpp` to perform the heavy lifting of defining a
/// function.
int builtin_function(parser_t &parser, io_streams_t &streams, const wcstring_list_t &c_args,
const wcstring &contents, int definition_line_offset, wcstring *out_err) {
assert(out_err != NULL);
const wcstring &contents, int definition_line_offset) {
// The wgetopt function expects 'function' as the first argument. Make a new wcstring_list with
// that property. This is needed because this builtin has a different signature than the other
// builtins.
wcstring_list_t args;
args.push_back(L"function");
wcstring_list_t args = {L"function"};
args.insert(args.end(), c_args.begin(), c_args.end());
// Hackish const_cast matches the one in builtin_run.
@ -215,21 +210,21 @@ int builtin_function(parser_t &parser, io_streams_t &streams, const wcstring_lis
wchar_t **argv = const_cast<wchar_t **>(argv_array.get());
wchar_t *cmd = argv[0];
int argc = builtin_count_args(argv);
wcstring function_name;
function_cmd_opts_t opts;
// A valid function name has to be the first argument.
int retval = validate_function_name(argc, argv, function_name, cmd, out_err);
wcstring function_name;
int retval = validate_function_name(argc, argv, function_name, cmd, streams);
if (retval != STATUS_CMD_OK) return retval;
argv++;
argc--;
function_cmd_opts_t opts;
int optind;
retval = parse_cmd_opts(opts, &optind, argc, argv, parser, streams, out_err);
retval = parse_cmd_opts(opts, &optind, argc, argv, parser, streams);
if (retval != STATUS_CMD_OK) return retval;
if (opts.print_help) {
builtin_print_help(parser, streams, cmd, streams.out);
builtin_print_help(parser, streams, cmd, streams.err);
return STATUS_CMD_OK;
}
@ -239,7 +234,7 @@ int builtin_function(parser_t &parser, io_streams_t &streams, const wcstring_lis
opts.named_arguments.push_back(argv[i]);
}
} else {
append_format(*out_err, _(L"%ls: Unexpected positional argument '%ls'"), cmd,
streams.err.append_format(_(L"%ls: Unexpected positional argument '%ls'"), cmd,
argv[optind]);
return STATUS_INVALID_ARGS;
}

2
src/builtin_function.h
View file

@ -8,5 +8,5 @@ class parser_t;
struct io_streams_t;
int builtin_function(parser_t &parser, io_streams_t &streams, const wcstring_list_t &c_args,
const wcstring &contents, int definition_line_offset, wcstring *out_err);
const wcstring &contents, int definition_line_offset);
#endif

9
src/parse_execution.cpp
View file

@ -388,14 +388,13 @@ parse_execution_result_t parse_execution_context_t::run_function_statement(
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;
io_streams_t streams;
int err = builtin_function(*parser, streams, argument_list, contents_str,
definition_line_offset, &error_str);
definition_line_offset);
proc_set_last_status(err);
if (!error_str.empty()) {
this->report_error(header, L"%ls", error_str.c_str());
if (!streams.err.empty()) {
this->report_error(header, L"%ls", streams.err.buffer().c_str());
result = parse_execution_errored;
}
@ -690,7 +689,7 @@ parse_execution_result_t parse_execution_context_t::report_errors(
// Get a backtrace.
wcstring backtrace_and_desc;
parser->get_backtrace(src, error_list, &backtrace_and_desc);
parser->get_backtrace(src, error_list, backtrace_and_desc);
// Print it.
if (!should_suppress_stderr_for_tests()) {

10
src/parse_tree.cpp
View file

@ -29,13 +29,13 @@ static bool production_is_empty(const production_element_t *production) {
/// Returns a string description of this parse error.
wcstring parse_error_t::describe_with_prefix(const wcstring &src, const wcstring &prefix,
bool is_interactive, bool skip_caret) const {
if (skip_caret || source_start >= src.size() || source_start + source_length > src.size()) {
return L"";
}
wcstring result = prefix;
result.append(this->text);
if (skip_caret || source_start >= src.size() || source_start + source_length > src.size()) {
return result;
}
// Locate the beginning of this line of source.
size_t line_start = 0;
@ -69,7 +69,7 @@ wcstring parse_error_t::describe_with_prefix(const wcstring &src, const wcstring
}
// Append the line of text.
result.push_back(L'\n');
if (!result.empty()) result.push_back(L'\n');
result.append(src, line_start, line_end - line_start);
// Append the caret line. The input source may include tabs; for that reason we

11
src/parser.cpp
View file

@ -602,7 +602,7 @@ int parser_t::eval(const wcstring &cmd, const io_chain_t &io, enum block_type_t
if (!parse_tree_from_string(cmd, parse_flag_none, &tree, &error_list)) {
// Get a backtrace. This includes the message.
wcstring backtrace_and_desc;
this->get_backtrace(cmd, error_list, &backtrace_and_desc);
this->get_backtrace(cmd, error_list, backtrace_and_desc);
// Print it.
fwprintf(stderr, L"%ls\n", backtrace_and_desc.c_str());
@ -725,8 +725,7 @@ bool parser_t::detect_errors_in_argument_list(const wcstring &arg_list_src, wcst
}
void parser_t::get_backtrace(const wcstring &src, const parse_error_list_t &errors,
wcstring *output) const {
assert(output != NULL);
wcstring &output) const {
if (!errors.empty()) {
const parse_error_t &err = errors.at(0);
@ -762,10 +761,10 @@ void parser_t::get_backtrace(const wcstring &src, const parse_error_list_t &erro
const wcstring description =
err.describe_with_prefix(src, prefix, is_interactive, skip_caret);
if (!description.empty()) {
output->append(description);
output->push_back(L'\n');
output.append(description);
output.push_back(L'\n');
}
output->append(this->stack_trace());
output.append(this->stack_trace());
}
}

2
src/parser.h
View file

@ -327,7 +327,7 @@ class parser_t {
profile_item_t *create_profile_item();
void get_backtrace(const wcstring &src, const parse_error_list_t &errors,
wcstring *output) const;
wcstring &output) const;
/// Detect errors in the specified string when parsed as an argument list. Returns true if an
/// error occurred.

4
src/reader.cpp
View file

@ -1966,7 +1966,7 @@ parser_test_error_bits_t reader_shell_test(const wchar_t *b) {
if (res & PARSER_TEST_ERROR) {
wcstring error_desc;
parser_t::principal_parser().get_backtrace(bstr, errors, &error_desc);
parser_t::principal_parser().get_backtrace(bstr, errors, error_desc);
// Ensure we end with a newline. Also add an initial newline, because it's likely the user
// just hit enter and so there's junk on the current line.
@ -3315,7 +3315,7 @@ static int read_ni(int fd, const io_chain_t &io) {
parser.eval(str, io, TOP, std::move(tree));
} else {
wcstring sb;
parser.get_backtrace(str, errors, &sb);
parser.get_backtrace(str, errors, sb);
fwprintf(stderr, L"%ls", sb.c_str());
res = 1;
}