From 5055621e02d17296d05495ebb3e74520a53b24cb Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Mon, 10 Sep 2018 18:59:57 -0700 Subject: [PATCH] Eliminate env_push and env_pop --- src/builtin_argparse.cpp | 37 ++++++++++++++++++++----------------- src/env.cpp | 16 ++++++---------- src/env.h | 6 ------ src/fish_tests.cpp | 5 +++-- src/parser.cpp | 4 ++-- 5 files changed, 31 insertions(+), 37 deletions(-) diff --git a/src/builtin_argparse.cpp b/src/builtin_argparse.cpp index ae15cfc6e..4fe148032 100644 --- a/src/builtin_argparse.cpp +++ b/src/builtin_argparse.cpp @@ -22,6 +22,7 @@ #include "exec.h" #include "fallback.h" // IWYU pragma: keep #include "io.h" +#include "parser.h" #include "wgetopt.h" // IWYU pragma: keep #include "wutil.h" // IWYU pragma: keep @@ -433,14 +434,14 @@ static void populate_option_strings(const argparse_cmd_opts_t &opts, wcstring *s long_options->push_back({NULL, 0, NULL, 0}); } -static int validate_arg(const argparse_cmd_opts_t &opts, option_spec_t *opt_spec, bool is_long_flag, - const wchar_t *woptarg, io_streams_t &streams) { +static int validate_arg(parser_t &parser, const argparse_cmd_opts_t &opts, option_spec_t *opt_spec, + bool is_long_flag, const wchar_t *woptarg, io_streams_t &streams) { // Obviously if there is no arg validation command we assume the arg is okay. if (opt_spec->validation_command.empty()) return STATUS_CMD_OK; wcstring_list_t cmd_output; - env_push(true); + parser.vars().push(true); env_set_one(L"_argparse_cmd", ENV_LOCAL, opts.name); if (is_long_flag) { env_set_one(var_name_prefix + L"name", ENV_LOCAL, opt_spec->long_flag); @@ -455,7 +456,7 @@ static int validate_arg(const argparse_cmd_opts_t &opts, option_spec_t *opt_spec streams.err.append(output); streams.err.push_back(L'\n'); } - env_pop(); + parser.vars().pop(); return retval; } @@ -473,13 +474,14 @@ static bool is_implicit_int(const argparse_cmd_opts_t &opts, const wchar_t *val) } // Store this value under the implicit int option. -static int validate_and_store_implicit_int(const argparse_cmd_opts_t &opts, const wchar_t *val, - wgetopter_t &w, int long_idx, io_streams_t &streams) { +static int validate_and_store_implicit_int(parser_t &parser, const argparse_cmd_opts_t &opts, + const wchar_t *val, wgetopter_t &w, int long_idx, + io_streams_t &streams) { // See if this option passes the validation checks. auto found = opts.options.find(opts.implicit_int_flag); assert(found != opts.options.end()); const auto &opt_spec = found->second; - int retval = validate_arg(opts, opt_spec.get(), long_idx != -1, val, streams); + int retval = validate_arg(parser, opts, opt_spec.get(), long_idx != -1, val, streams); if (retval != STATUS_CMD_OK) return retval; // It's a valid integer so store it and return success. @@ -490,8 +492,8 @@ static int validate_and_store_implicit_int(const argparse_cmd_opts_t &opts, cons return STATUS_CMD_OK; } -static int handle_flag(const argparse_cmd_opts_t &opts, option_spec_t *opt_spec, int long_idx, - const wchar_t *woptarg, io_streams_t &streams) { +static int handle_flag(parser_t &parser, const argparse_cmd_opts_t &opts, option_spec_t *opt_spec, + int long_idx, const wchar_t *woptarg, io_streams_t &streams) { opt_spec->num_seen++; if (opt_spec->num_allowed == 0) { // It's a boolean flag. Save the flag we saw since it might be useful to know if the @@ -506,7 +508,7 @@ static int handle_flag(const argparse_cmd_opts_t &opts, option_spec_t *opt_spec, } if (woptarg) { - int retval = validate_arg(opts, opt_spec, long_idx != -1, woptarg, streams); + int retval = validate_arg(parser, opts, opt_spec, long_idx != -1, woptarg, streams); if (retval != STATUS_CMD_OK) return retval; } @@ -526,9 +528,9 @@ static int handle_flag(const argparse_cmd_opts_t &opts, option_spec_t *opt_spec, return STATUS_CMD_OK; } -static int argparse_parse_flags(const argparse_cmd_opts_t &opts, const wchar_t *short_options, - const woption *long_options, const wchar_t *cmd, int argc, - wchar_t **argv, int *optind, parser_t &parser, +static int argparse_parse_flags(parser_t &parser, const argparse_cmd_opts_t &opts, + const wchar_t *short_options, const woption *long_options, + const wchar_t *cmd, int argc, wchar_t **argv, int *optind, io_streams_t &streams) { int opt; int long_idx = -1; @@ -544,7 +546,8 @@ static int argparse_parse_flags(const argparse_cmd_opts_t &opts, const wchar_t * const wchar_t *arg_contents = argv[w.woptind - 1] + 1; int retval = STATUS_CMD_OK; if (is_implicit_int(opts, arg_contents)) { - retval = validate_and_store_implicit_int(opts, arg_contents, w, long_idx, streams); + retval = validate_and_store_implicit_int(parser, opts, arg_contents, w, long_idx, + streams); } else { builtin_unknown_option(parser, streams, cmd, argv[w.woptind - 1]); retval = STATUS_INVALID_ARGS; @@ -558,7 +561,7 @@ static int argparse_parse_flags(const argparse_cmd_opts_t &opts, const wchar_t * auto found = opts.options.find(opt); assert(found != opts.options.end()); - int retval = handle_flag(opts, found->second.get(), long_idx, w.woptarg, streams); + int retval = handle_flag(parser, opts, found->second.get(), long_idx, w.woptarg, streams); if (retval != STATUS_CMD_OK) return retval; long_idx = -1; } @@ -590,8 +593,8 @@ static int argparse_parse_args(argparse_cmd_opts_t &opts, const wcstring_list_t auto argv = argv_container.get(); int optind; - int retval = argparse_parse_flags(opts, short_options.c_str(), long_options.data(), cmd, argc, - argv, &optind, parser, streams); + int retval = argparse_parse_flags(parser, opts, short_options.c_str(), long_options.data(), cmd, + argc, argv, &optind, streams); if (retval != STATUS_CMD_OK) return retval; retval = check_for_mutually_exclusive_flags(opts, streams); diff --git a/src/env.cpp b/src/env.cpp index 50341b890..df7282a72 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -161,7 +161,12 @@ struct var_stack_t { void mark_changed_exported() { has_changed_exported = true; } void update_export_array_if_necessary(); - var_stack_t() : top(globals()), global_env(globals()) {} + var_stack_t() : top(globals()), global_env(globals()) { + // Add a toplevel local scope on top of the global scope. This local scope will persist + // throughout the lifetime of the fish process, and it will ensure that `set -l` commands + // run at the command-line don't affect the global scope. + push(false); + } // Pushes a new node onto our stack // Optionally creates a new scope for the node @@ -999,11 +1004,6 @@ void env_init(const struct config_paths_t *paths /* or NULL */) { callback_data_list_t callbacks; s_universal_variables->initialize(callbacks); env_universal_callbacks(&env_stack_t::principal(), callbacks); - - // Now that the global scope is fully initialized, add a toplevel local scope. This same local - // scope will persist throughout the lifetime of the fish process, and it will ensure that `set - // -l` commands run at the command-line don't affect the global scope. - env_push(false); } /// Search all visible scopes in order for the specified key. Return the first scope in which it was @@ -1438,10 +1438,6 @@ int env_set_empty(const wcstring &key, env_mode_flags_t mode) { int env_remove(const wcstring &key, int mode) { return env_stack_t::principal().remove(key, mode); } -void env_push(bool new_scope) { env_stack_t::principal().push(new_scope); } - -void env_pop() { env_stack_t::principal().pop(); } - void env_universal_barrier() { env_stack_t::principal().universal_barrier(); } void env_set_argv(const wchar_t *const *argv) { return env_stack_t::principal().set_argv(argv); } diff --git a/src/env.h b/src/env.h index 379175880..22606c45c 100644 --- a/src/env.h +++ b/src/env.h @@ -165,12 +165,6 @@ int env_set_empty(const wcstring &key, env_mode_flags_t mode); /// \return zero if the variable existed, and non-zero if the variable did not exist int env_remove(const wcstring &key, int mode); -/// Push the variable stack. Used for implementing local variables for functions and for-loops. -void env_push(bool new_scope); - -/// Pop the variable stack. Used for implementing local variables for functions and for-loops. -void env_pop(); - /// Synchronizes all universal variable changes: writes everything out, reads stuff in. void env_universal_barrier(); diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 24decdb44..6b6c8b6eb 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -1755,7 +1755,8 @@ static void test_ifind_fuzzy() { static void test_abbreviations() { say(L"Testing abbreviations"); - env_push(true); + auto &vars = parser_t::principal_parser().vars(); + vars.push(true); const std::vector> abbreviations = { {L"gc", L"git checkout"}, @@ -1822,7 +1823,7 @@ static void test_abbreviations() { expanded = reader_expand_abbreviation_in_command(L"command gc", wcslen(L"command gc"), &result); if (expanded) err(L"gc incorrectly expanded on line %ld", (long)__LINE__); - env_pop(); + vars.pop(); } /// Test path functions. diff --git a/src/parser.cpp b/src/parser.cpp index ce79a0c04..65fcffcd4 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -154,7 +154,7 @@ void parser_t::push_block_int(block_t *new_current) { } if (new_current->type() != TOP) { - env_push(type == FUNCTION_CALL); + vars().push(type == FUNCTION_CALL); new_current->wants_pop_env = true; } } @@ -172,7 +172,7 @@ void parser_t::pop_block(const block_t *expected) { std::unique_ptr old = std::move(block_stack.back()); block_stack.pop_back(); - if (old->wants_pop_env) env_pop(); + if (old->wants_pop_env) vars().pop(); // Figure out if `status is-block` should consider us to be in a block now. bool new_is_block = false;