diff --git a/CHANGELOG.rst b/CHANGELOG.rst index a4efa172e..567038555 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -25,6 +25,9 @@ Notable improvements and fixes for a,b in y 1 z 3 ^~^ - A new helper function ``fish_delta`` can be used to show the difference to fish's stock configuration (:issue:`9255`). +- It is now possible to specify multiple scopes for ``set -e`` and all of the named variables present in any of the specified scopes will be erased. This makes it possible to remove all instances of a variable in all scopes (``set -efglU foo``) in one go (:issue:`7711`). + +======= Deprecations and removed features --------------------------------- diff --git a/doc_src/cmds/set.rst b/doc_src/cmds/set.rst index 96a52e179..780274d51 100644 --- a/doc_src/cmds/set.rst +++ b/doc_src/cmds/set.rst @@ -68,7 +68,7 @@ The following other options are available: Causes the values to be prepended to the current set of values for the variable. This can be used with **--append** to both append and prepend at the same time. This cannot be used when assigning to a variable slice. **-e** or **--erase** - Causes the specified shell variables to be erased + Causes the specified shell variables to be erased. Supports erasing from multiple scopes at once. **-q** or **--query** Test if the specified variable names are defined. Does not output anything, but the builtins exit status is the number of variables specified that were not defined, up to a maximum of 255. If no variable was given, it also returns 255. diff --git a/src/builtins/set.cpp b/src/builtins/set.cpp index 69993d7a6..fe2fc7469 100644 --- a/src/builtins/set.cpp +++ b/src/builtins/set.cpp @@ -204,11 +204,14 @@ static int validate_cmd_opts(const wchar_t *cmd, const set_cmd_opts_t &opts, int return STATUS_INVALID_ARGS; } - // Variables can only have one scope. + // Variables can only have one scope... if (opts.local + opts.function + opts.global + opts.universal > 1) { - streams.err.append_format(BUILTIN_ERR_GLOCAL, cmd); - builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + // ..unless we are erasing a variable, in which case we can erase from several in one go. + if (!opts.erase) { + streams.err.append_format(BUILTIN_ERR_GLOCAL, cmd); + builtin_print_error_trailer(parser, streams.err, cmd); + return STATUS_INVALID_ARGS; + } } // Variables can only have one export status. @@ -391,7 +394,7 @@ static maybe_t<split_var_t> split_var_and_indexes(const wchar_t *arg, env_mode_f return res; } -/// Given a list of values and 1-based indexes, return a new list, with those elements removed. +/// Given a list of values and 1-based indexes, return a new list with those elements removed. /// Note this deliberately accepts both args by value, as it modifies them both. static wcstring_list_t erased_at_indexes(wcstring_list_t input, std::vector<long> indexes) { // Sort our indexes into *descending* order. @@ -620,45 +623,50 @@ static int builtin_set_show(const wchar_t *cmd, const set_cmd_opts_t &opts, int static int builtin_set_erase(const wchar_t *cmd, set_cmd_opts_t &opts, int argc, const wchar_t **argv, parser_t &parser, io_streams_t &streams) { int ret = STATUS_CMD_OK; - env_mode_flags_t scope = compute_scope(opts); - for (int i = 0; i < argc; i++) { - auto split = split_var_and_indexes(argv[i], scope, parser.vars(), streams); - if (!split) { - builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_CMD_ERROR; - } - - if (!valid_var_name(split->varname)) { - streams.err.append_format(BUILTIN_ERR_VARNAME, cmd, split->varname.c_str()); - builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; - } - - int retval = STATUS_CMD_OK; - if (split->indexes.empty()) { // unset the var - retval = parser.vars().remove(split->varname, scope); - // When a non-existent-variable is unset, return ENV_NOT_FOUND as $status - // but do not emit any errors at the console as a compromise between user - // friendliness and correctness. - if (retval != ENV_NOT_FOUND) { - handle_env_return(retval, cmd, split->varname, streams); + env_mode_flags_t scopes = compute_scope(opts); + // `set -e` is allowed to be called with multiple scopes. + for (int bit = 0; 1<<bit <= ENV_USER; ++bit) { + int scope = scopes & 1<<bit; + if (scope == 0 || (scope == ENV_USER && scopes != ENV_USER)) continue; + for (int i = 0; i < argc; i++) { + auto split = split_var_and_indexes(argv[i], scope, parser.vars(), streams); + if (!split) { + builtin_print_error_trailer(parser, streams.err, cmd); + return STATUS_CMD_ERROR; } - if (retval == ENV_OK) { - event_fire(parser, event_t::variable_erase(split->varname)); - } - } else { // remove just the specified indexes of the var - if (!split->var) return STATUS_CMD_ERROR; - wcstring_list_t result = erased_at_indexes(split->var->as_list(), split->indexes); - retval = env_set_reporting_errors(cmd, split->varname, scope, std::move(result), - streams, parser); - } - // Set $status to the last error value. - // This is cheesy, but I don't expect this to be checked often. - if (retval != STATUS_CMD_OK) { - ret = retval; - } else { - warn_if_uvar_shadows_global(cmd, opts, split->varname, streams, parser); + if (!valid_var_name(split->varname)) { + streams.err.append_format(BUILTIN_ERR_VARNAME, cmd, split->varname.c_str()); + builtin_print_error_trailer(parser, streams.err, cmd); + return STATUS_INVALID_ARGS; + } + + int retval = STATUS_CMD_OK; + if (split->indexes.empty()) { // unset the var + retval = parser.vars().remove(split->varname, scope); + // When a non-existent-variable is unset, return ENV_NOT_FOUND as $status + // but do not emit any errors at the console as a compromise between user + // friendliness and correctness. + if (retval != ENV_NOT_FOUND) { + handle_env_return(retval, cmd, split->varname, streams); + } + if (retval == ENV_OK) { + event_fire(parser, event_t::variable_erase(split->varname)); + } + } else { // remove just the specified indexes of the var + if (!split->var) return STATUS_CMD_ERROR; + wcstring_list_t result = erased_at_indexes(split->var->as_list(), split->indexes); + retval = env_set_reporting_errors(cmd, split->varname, scope, std::move(result), + streams, parser); + } + + // Set $status to the last error value. + // This is cheesy, but I don't expect this to be checked often. + if (retval != STATUS_CMD_OK) { + ret = retval; + } else { + warn_if_uvar_shadows_global(cmd, opts, split->varname, streams, parser); + } } } return ret; diff --git a/tests/checks/set.fish b/tests/checks/set.fish index 1fbc671f1..db936a54d 100644 --- a/tests/checks/set.fish +++ b/tests/checks/set.fish @@ -921,3 +921,23 @@ foo=bar $FISH -c 'set foo 1 2 3; set --show foo' # CHECK: $foo[2]: |2| # CHECK: $foo[3]: |3| # CHECK: $foo: originally inherited as |bar| + +# Verify behavior of erasing in multiple scopes simultaneously +set -U marbles global +set -g marbles global +set -l marbles local + +set -eUg marbles +set -ql marbles || echo "erased in more scopes than it should!" +set -qg marbles && echo "didn't erase from global scope!" +set -qU marbles && echo "didn't erase from universal scope!" + +begin + set -l secret local 4 8 15 16 23 42 + set -g secret global 4 8 15 16 23 42 + set -egl secret[3..5] + echo $secret + # CHECK: local 4 23 42 +end +echo $secret +# CHECK: global 4 23 42