diff --git a/CHANGELOG.md b/CHANGELOG.md index a4d23105e..06396c2a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ This section is for changes merged to the `major` branch that are not also merge ## Notable fixes and improvements - `read` now requires at least one var name (#4220). - Local exported (`set -lx`) vars are now visible to functions (#1091). +- `set x[1] x[2] a b` is no longer valid syntax (#4236). ## Other significant changes diff --git a/doc_src/set.txt b/doc_src/set.txt index 859a0d899..ce35f2253 100644 --- a/doc_src/set.txt +++ b/doc_src/set.txt @@ -44,7 +44,7 @@ The following options are available: If a variable is set to more than one value, the variable will be an array with the specified elements. If a variable is set to zero elements, it will become an array with zero elements. -If the variable name is one or more array elements, such as `PATH[1 3 7]`, only those array elements specified will be changed. When array indices are specified to `set`, multiple arguments may be used to specify additional indexes, e.g. `set PATH[1] PATH[4] /bin /sbin`. If you specify a negative index when expanding or assigning to an array variable, the index will be calculated from the end of the array. For example, the index -1 means the last index of an array. +If the variable name is one or more array elements, such as `PATH[1 3 7]`, only those array elements specified will be changed. If you specify a negative index when expanding or assigning to an array variable, the index will be calculated from the end of the array. For example, the index -1 means the last index of an array. The scoping rules when creating or updating a variable are: @@ -92,3 +92,7 @@ if set python_path (type -p python) end # Outputs the path to Python if `type -p` returns true. \endfish + +\subsection set-notes Notes + +Fish versions prior to 3.0 allowed you to write `set PATH[1 4] /bin /sbin` as `set PATH[1] PATH[4] /bin /sbin`. That alternative syntax is ambiguous and inconsistent. Also, no fish script in the fish project used it. Nor could we find any 3rd-party scripts in the Oh-My-Fish or Fisherman package managers that used that alternative syntax. So it was removed in the 3.0 release. diff --git a/src/builtin_set.cpp b/src/builtin_set.cpp index ba97cc532..51e103b11 100644 --- a/src/builtin_set.cpp +++ b/src/builtin_set.cpp @@ -8,7 +8,6 @@ #include #include #include -#include #include #include @@ -60,14 +59,12 @@ static const struct woption long_options[] = {{L"export", no_argument, NULL, 'x' {NULL, 0, NULL, 0}}; // Error message for invalid path operations. -#define BUILTIN_SET_PATH_ERROR L"%ls: Warning: $%ls entry \"%ls\" is not valid (%s)\n" - +#define BUILTIN_SET_PATH_ERROR _(L"%ls: Warning: $%ls entry \"%ls\" is not valid (%s)\n") // Hint for invalid path operation with a colon. -#define BUILTIN_SET_PATH_HINT L"%ls: Did you mean 'set %ls $%ls %ls'?\n" - -// Error for mismatch between index count and elements. -#define BUILTIN_SET_ARG_COUNT \ - L"%ls: The number of variable indexes does not match the number of values\n" +#define BUILTIN_SET_PATH_HINT _(L"%ls: Did you mean 'set %ls $%ls %ls'?\n") +#define BUILTIN_SET_MISMATCHED_ARGS _(L"%ls: You provided %d indexes but %d values\n") +#define BUILTIN_SET_ERASE_NO_VAR _(L"%ls: Erase needs a variable name\n") +#define BUILTIN_SET_ARRAY_BOUNDS_ERR _(L"%ls: Array index out of bounds\n") // Test if the specified variable should be subject to path validation. static const wcstring_list_t path_variables({L"PATH", L"CDPATH"}); @@ -143,7 +140,7 @@ static int parse_cmd_opts(set_cmd_opts_t &opts, int *optind, //!OCLINT(high ncs return STATUS_CMD_OK; } -static int validate_cmd_opts(const wchar_t *cmd, set_cmd_opts_t &opts, int optind, int argc, +static int validate_cmd_opts(const wchar_t *cmd, set_cmd_opts_t &opts, int argc, parser_t &parser, io_streams_t &streams) { // Can't query and erase or list. if (opts.query && (opts.erase || opts.list)) { @@ -173,8 +170,8 @@ static int validate_cmd_opts(const wchar_t *cmd, set_cmd_opts_t &opts, int optin return STATUS_INVALID_ARGS; } - if (optind == argc && opts.erase) { - streams.err.append_format(_(L"%ls: Erase needs a variable name\n"), cmd); + if (argc == 0 && opts.erase) { + streams.err.append_format(BUILTIN_SET_ERASE_NO_VAR, cmd); builtin_print_help(parser, streams, cmd, streams.err); return STATUS_INVALID_ARGS; } @@ -182,100 +179,119 @@ static int validate_cmd_opts(const wchar_t *cmd, set_cmd_opts_t &opts, int optin return STATUS_CMD_OK; } +// Check if we are setting a uvar and a global of the same name exists. See +// https://github.com/fish-shell/fish-shell/issues/806 +static int check_global_scope_exists(const wchar_t *cmd, set_cmd_opts_t &opts, const wchar_t *dest, + io_streams_t &streams) { + if (opts.universal) { + env_var_t global_dest = env_get_string(dest, ENV_GLOBAL); + if (!global_dest.missing()) { + streams.err.append_format( + _(L"%ls: Warning: universal scope selected, but a global variable '%ls' exists.\n"), + cmd, dest); + } + } + + return STATUS_CMD_OK; +} + +// Fix for https://github.com/fish-shell/fish-shell/issues/199 . Return success if any path setting +// succeeds. +static int my_env_path_setup(const wchar_t *cmd, const wchar_t *key, //!OCLINT(npath complexity) + const wcstring_list_t &list, io_streams_t &streams) { + bool any_success = false; + + // Don't bother validating (or complaining about) values that are already present. When + // determining already-present values, use ENV_DEFAULT instead of the passed-in scope because + // in: + // + // set -l PATH stuff $PATH + // + // where we are temporarily shadowing a variable, we want to compare against the shadowed value, + // not the (missing) local value. Also don't bother to complain about relative paths, which + // don't start with /. + wcstring_list_t existing_values; + const env_var_t existing_variable = env_get_string(key, ENV_DEFAULT); + if (!existing_variable.missing_or_empty()) + tokenize_variable_array(existing_variable, existing_values); + + for (size_t i = 0; i < list.size(); i++) { + const wcstring &dir = list.at(i); + if (!string_prefixes_string(L"/", dir) || contains(existing_values, dir)) { + any_success = true; + continue; + } + + bool show_hint = false; + bool error = false; + struct stat buff; + if (wstat(dir, &buff) == -1) { + error = true; + } else if (!S_ISDIR(buff.st_mode)) { + error = true; + errno = ENOTDIR; + } else if (waccess(dir, X_OK) == -1) { + error = true; + } + + if (!error) { + any_success = true; + } else { + streams.err.append_format(BUILTIN_SET_PATH_ERROR, cmd, key, dir.c_str(), + strerror(errno)); + const wchar_t *colon = wcschr(dir.c_str(), L':'); + if (colon && *(colon + 1)) show_hint = true; + } + + if (show_hint) { + streams.err.append_format(BUILTIN_SET_PATH_HINT, cmd, key, key, + wcschr(dir.c_str(), L':') + 1); + } + } + + // Fail at setting the path if we tried to set it to something non-empty, but it wound up + // empty. + if (!list.empty() && !any_success) return STATUS_CMD_ERROR; + return STATUS_CMD_OK; +} + /// Call env_set. If this is a path variable, e.g. PATH, validate the elements. On error, print a /// description of the problem to stderr. -static int my_env_set(const wchar_t *key, const wcstring_list_t &list, int scope, - io_streams_t &streams) { +static int my_env_set(const wchar_t *cmd, const wchar_t *key, const wcstring_list_t &list, + int scope, io_streams_t &streams) { + int retval; + if (is_path_variable(key)) { - // Fix for https://github.com/fish-shell/fish-shell/issues/199 . Return success if any path - // setting succeeds. - bool any_success = false; - - // Don't bother validating (or complaining about) values that are already present. When - // determining already-present values, use ENV_DEFAULT instead of the passed-in scope - // because in: - // - // set -l PATH stuff $PATH - // - // where we are temporarily shadowing a variable, we want to compare against the shadowed - // value, not the (missing) local value. Also don't bother to complain about relative paths, - // which don't start with /. - wcstring_list_t existing_values; - const env_var_t existing_variable = env_get_string(key, ENV_DEFAULT); - if (!existing_variable.missing_or_empty()) - tokenize_variable_array(existing_variable, existing_values); - - for (size_t i = 0; i < list.size(); i++) { - const wcstring &dir = list.at(i); - if (!string_prefixes_string(L"/", dir) || contains(existing_values, dir)) { - any_success = true; - continue; - } - - int show_hint = 0; - bool error = false; - struct stat buff; - if (wstat(dir, &buff) == -1) { - error = true; - } else if (!S_ISDIR(buff.st_mode)) { - error = true; - errno = ENOTDIR; - } else if (waccess(dir, X_OK) == -1) { - error = true; - } - - if (!error) { - any_success = true; - } else { - streams.err.append_format(_(BUILTIN_SET_PATH_ERROR), L"set", key, dir.c_str(), - strerror(errno)); - const wchar_t *colon = wcschr(dir.c_str(), L':'); - - if (colon && *(colon + 1)) { - show_hint = 1; - } - } - - if (show_hint) { - streams.err.append_format(_(BUILTIN_SET_PATH_HINT), L"set", key, key, - wcschr(dir.c_str(), L':') + 1); - } - } - - // Fail at setting the path if we tried to set it to something non-empty, but it wound up - // empty. - if (!list.empty() && !any_success) { - return STATUS_CMD_ERROR; - } + retval = my_env_path_setup(cmd, key, list, streams); + if (retval != STATUS_CMD_OK) return retval; } // We don't check `val->empty()` because an array var with a single empty string will be // "empty". A truly empty array var is set to the special value `ENV_NULL`. auto val = list_to_array_val(list); - int retcode = env_set(key, *val == ENV_NULL ? NULL : val->c_str(), scope | ENV_USER); - switch (retcode) { + retval = env_set(key, *val == ENV_NULL ? NULL : val->c_str(), scope | ENV_USER); + switch (retval) { case ENV_OK: { - retcode = STATUS_CMD_OK; + retval = STATUS_CMD_OK; break; } case ENV_PERM: { streams.err.append_format(_(L"%ls: Tried to change the read-only variable '%ls'\n"), - L"set", key); - retcode = STATUS_CMD_ERROR; + cmd, key); + retval = STATUS_CMD_ERROR; break; } case ENV_SCOPE: { streams.err.append_format( - _(L"%ls: Tried to set the special variable '%ls' with the wrong scope\n"), L"set", + _(L"%ls: Tried to set the special variable '%ls' with the wrong scope\n"), cmd, key); - retcode = STATUS_CMD_ERROR; + retval = STATUS_CMD_ERROR; break; } case ENV_INVALID: { streams.err.append_format( - _(L"%ls: Tried to set the special variable '%ls' to an invalid value\n"), L"set", - key); - retcode = STATUS_CMD_ERROR; + _(L"%ls: Tried to set the special variable '%ls' to an invalid value\n"), cmd, key); + retval = STATUS_CMD_ERROR; break; } default: { @@ -284,70 +300,55 @@ static int my_env_set(const wchar_t *key, const wcstring_list_t &list, int scope } } - return retcode; + return retval; } -/// Extract indexes from a destination argument of the form name[index1 index2...] +/// Extract indexes from an argument of the form `var_name[index1 index2...]`. /// -/// \param indexes the list to insert the new indexes into -/// \param src the source string to parse -/// \param name the name of the element. Return null if the name in \c src does not match this name -/// \param var_count the number of elements in the array to parse. +/// Inputs: +/// indexes: the list to insert the new indexes into +/// src: The source string to parse. This must be a var spec of the form "var[...]" /// -/// \return the total number of indexes parsed, or -1 on error -static int parse_index(std::vector &indexes, const wchar_t *src, const wchar_t *name, - size_t var_count, io_streams_t &streams) { - size_t len; +/// Returns: +/// The total number of indexes parsed, or -1 on error. If any indexes were found the `src` string +/// is modified to omit the index expression leaving just the var name. +static int parse_index(std::vector &indexes, wchar_t *src, int scope, io_streams_t &streams) { + wchar_t *p = wcschr(src, L'['); + if (!p) return 0; // no slices so nothing for us to do + *p = L'\0'; // split the var name from the indexes/slices + p++; + + env_var_t var_str = env_get_string(src, scope); + wcstring_list_t var; + if (!var_str.missing()) tokenize_variable_array(var_str, var); + int count = 0; - const wchar_t *src_orig = src; - if (src == 0) { - return 0; - } - - while (*src != L'\0' && (iswalnum(*src) || *src == L'_')) src++; - - if (*src != L'[') { - streams.err.append_format(_(BUILTIN_SET_ARG_COUNT), L"set"); - return 0; - } - - len = src - src_orig; - if ((wcsncmp(src_orig, name, len) != 0) || (wcslen(name) != (len))) { - streams.err.append_format( - _(L"%ls: Multiple variable names specified in single call (%ls and %.*ls)\n"), L"set", - name, len, src_orig); - return 0; - } - - src++; - while (iswspace(*src)) src++; - - while (*src != L']') { + while (*p != L']') { const wchar_t *end; - long l_ind = fish_wcstol(src, &end); + long l_ind = fish_wcstol(p, &end); if (errno > 0) { // ignore errno == -1 meaning the int did not end with a '\0' streams.err.append_format(_(L"%ls: Invalid index starting at '%ls'\n"), L"set", src); - return 0; + return -1; } + p = (wchar_t *)end; - if (l_ind < 0) l_ind = var_count + l_ind + 1; + // Convert negative index to a positive index. + if (l_ind < 0) l_ind = var.size() + l_ind + 1; - src = end; //!OCLINT(parameter reassignment) - if (*src == L'.' && *(src + 1) == L'.') { - src += 2; - long l_ind2 = fish_wcstol(src, &end); + if (*p == L'.' && *(p + 1) == L'.') { + p += 2; + long l_ind2 = fish_wcstol(p, &end); if (errno > 0) { // ignore errno == -1 meaning the int did not end with a '\0' - return 1; + return -1; } - src = end; //!OCLINT(parameter reassignment) + p = (wchar_t *)end; + + // Convert negative index to a positive index. + if (l_ind2 < 0) l_ind2 = var.size() + l_ind2 + 1; - if (l_ind2 < 0) { - l_ind2 = var_count + l_ind2 + 1; - } int direction = l_ind2 < l_ind ? -1 : 1; for (long jjj = l_ind; jjj * direction <= l_ind2 * direction; jjj += direction) { - // debug(0, L"Expand range [set]: %i\n", jjj); indexes.push_back(jjj); count++; } @@ -355,8 +356,6 @@ static int parse_index(std::vector &indexes, const wchar_t *src, const wch indexes.push_back(l_ind); count++; } - - while (iswspace(*src)) src++; } return count; @@ -364,10 +363,8 @@ static int parse_index(std::vector &indexes, const wchar_t *src, const wch static int update_values(wcstring_list_t &list, std::vector &indexes, wcstring_list_t &values) { - size_t i; - // Replace values where needed. - for (i = 0; i < indexes.size(); i++) { + for (size_t i = 0; i < indexes.size(); i++) { // The '- 1' below is because the indices in fish are one-based, but the vector uses // zero-based indices. long ind = indexes[i] - 1; @@ -379,7 +376,6 @@ static int update_values(wcstring_list_t &list, std::vector &indexes, list.resize(ind + 1); } - // free((void *) al_get(list, ind)); list[ind] = newv; } @@ -414,10 +410,15 @@ static int compute_scope(set_cmd_opts_t &opts) { return scope; } -/// Print the names of all environment variables in the scope, with or without shortening, with or -/// without values, with or without escaping -static int builtin_set_list(const wchar_t *cmd, set_cmd_opts_t &opts, int optind, int argc, +/// Print the names of all environment variables in the scope. It will include the values unless the +/// `set --list` flag was used. +static int builtin_set_list(const wchar_t *cmd, set_cmd_opts_t &opts, int argc, wchar_t **argv, parser_t &parser, io_streams_t &streams) { + UNUSED(cmd); + UNUSED(argc); + UNUSED(argv); + UNUSED(parser); + bool names_only = opts.list; wcstring_list_t names = env_get_names(compute_scope(opts)); sort(names.begin(), names.end()); @@ -451,42 +452,33 @@ static int builtin_set_list(const wchar_t *cmd, set_cmd_opts_t &opts, int optind } // Query mode. Return the number of variables that do not exist out of the specified variables. -static int builtin_set_query(const wchar_t *cmd, set_cmd_opts_t &opts, int optind, int argc, +static int builtin_set_query(const wchar_t *cmd, set_cmd_opts_t &opts, int argc, wchar_t **argv, parser_t &parser, io_streams_t &streams) { int retval = 0; + int scope = compute_scope(opts); - for (int i = optind; i < argc; i++) { + for (int i = 0; i < argc; i++) { wchar_t *arg = argv[i]; - bool slice = false; - wchar_t *dest = wcsdup(arg); assert(dest); - if (wchar_t *p = wcschr(dest, L'[')) { - *p = L'\0'; - slice = true; + std::vector indexes; + int idx_count = parse_index(indexes, dest, scope, streams); + if (idx_count == -1) { + free(dest); + builtin_print_help(parser, streams, cmd, streams.err); + return STATUS_CMD_ERROR; } - if (slice) { - std::vector indexes; + if (idx_count) { wcstring_list_t result; - size_t j; - - env_var_t dest_str = env_get_string(dest, compute_scope(opts)); + env_var_t dest_str = env_get_string(dest, scope); if (!dest_str.missing()) tokenize_variable_array(dest_str, result); - if (!parse_index(indexes, arg, dest, result.size(), streams)) { - builtin_print_help(parser, streams, cmd, streams.err); - return STATUS_CMD_ERROR; + for (auto idx : indexes) { + if (idx < 1 || (size_t)idx > result.size()) retval++; } - - for (j = 0; j < indexes.size(); j++) { - long idx = indexes[j]; - if (idx < 1 || (size_t)idx > result.size()) { - retval++; - } - } - } else if (!env_exist(arg, compute_scope(opts))) { + } else if (!env_exist(arg, scope)) { retval++; } @@ -496,6 +488,118 @@ static int builtin_set_query(const wchar_t *cmd, set_cmd_opts_t &opts, int optin return retval; } +/// Erase a variable. +static int builtin_set_erase(const wchar_t *cmd, set_cmd_opts_t &opts, int argc, + wchar_t **argv, parser_t &parser, io_streams_t &streams) { + if (argc != 1) { + streams.err.append_format(BUILTIN_ERR_ARG_COUNT2, cmd, L"--erase", 1, argc); + builtin_print_help(parser, streams, cmd, streams.err); + return STATUS_CMD_ERROR; + } + + int scope = compute_scope(opts); // calculate the variable scope based on the provided options + wchar_t *dest = argv[0]; + + std::vector indexes; + int idx_count = parse_index(indexes, dest, scope, streams); + if (idx_count == -1) { + builtin_print_help(parser, streams, cmd, streams.err); + return STATUS_CMD_ERROR; + } + + int retval; + if (!valid_var_name(dest)) { + streams.err.append_format(BUILTIN_ERR_VARNAME, cmd, dest); + builtin_print_help(parser, streams, cmd, streams.err); + return STATUS_INVALID_ARGS; + } + + if (idx_count == 0) { // unset the var + retval = env_remove(dest, scope); + } else { // remove just the specified indexes of the var + const env_var_t dest_var = env_get_string(dest, scope); + if (dest_var.missing()) return STATUS_CMD_ERROR; + wcstring_list_t result; + tokenize_variable_array(dest_var, result); + erase_values(result, indexes); + retval = my_env_set(cmd, dest, result, scope, streams); + } + + if (retval != STATUS_CMD_OK) return retval; + return check_global_scope_exists(cmd, opts, dest, streams); +} + +/// This handles the more difficult case of setting individual slices of a var. +static int set_slices(const wchar_t *cmd, set_cmd_opts_t &opts, const wchar_t *dest, + std::vector &indexes, int argc, wchar_t **argv, parser_t &parser, + io_streams_t &streams) { + UNUSED(parser); + + if (indexes.size() != static_cast(argc)) { + streams.err.append_format(BUILTIN_SET_MISMATCHED_ARGS, cmd, indexes.size(), argc); + } + + int scope = compute_scope(opts); // calculate the variable scope based on the provided options + wcstring_list_t result; + const env_var_t dest_str = env_get_string(dest, scope); + if (!dest_str.missing()) tokenize_variable_array(dest_str, result); + + // Slice indexes have been calculated, do the actual work. + wcstring_list_t new_values; + for (int i = 0; i < argc; i++) new_values.push_back(argv[i]); + + int retval = update_values(result, indexes, new_values); + if (retval != STATUS_CMD_OK) { + streams.err.append_format(BUILTIN_SET_ARRAY_BOUNDS_ERR, cmd); + return retval; + } + + retval = my_env_set(cmd, dest, result, scope, streams); + if (retval != STATUS_CMD_OK) return retval; + return check_global_scope_exists(cmd, opts, dest, streams); +} + +/// Set a variable. +static int builtin_set_set(const wchar_t *cmd, set_cmd_opts_t &opts, int argc, + wchar_t **argv, parser_t &parser, io_streams_t &streams) { + if (argc == 0) { + streams.err.append_format(BUILTIN_ERR_MIN_ARG_COUNT1, cmd, 1); + builtin_print_help(parser, streams, cmd, streams.err); + return STATUS_INVALID_ARGS; + } + + int retval; + int scope = compute_scope(opts); // calculate the variable scope based on the provided options + wchar_t *dest = argv[0]; + argv++; + argc--; + + std::vector indexes; + int idx_count = parse_index(indexes, dest, scope, streams); + if (idx_count == -1) { + builtin_print_help(parser, streams, cmd, streams.err); + return STATUS_INVALID_ARGS; + } + + if (!valid_var_name(dest)) { + streams.err.append_format(BUILTIN_ERR_VARNAME, cmd, dest); + builtin_print_help(parser, streams, cmd, streams.err); + return STATUS_INVALID_ARGS; + } + + if (idx_count != 0) { + // Handle the uncommon case of setting specific slices of a var. + return set_slices(cmd, opts, dest, indexes, argc, argv, parser, streams); + } + + // This is the simple, common, case. Set the var to the specified values. + wcstring_list_t values; + for (int i = 0; i < argc; i++) values.push_back(argv[i]); + retval = my_env_set(cmd, dest, values, scope, streams); + if (retval != STATUS_CMD_OK) return retval; + return check_global_scope_exists(cmd, opts, dest, streams); +} + /// The set builtin creates, updates, and erases (removes, deletes) variables. int builtin_set(parser_t &parser, io_streams_t &streams, wchar_t **argv) { const int incoming_exit_status = proc_get_last_status(); @@ -506,126 +610,29 @@ int builtin_set(parser_t &parser, io_streams_t &streams, wchar_t **argv) { int optind; int retval = parse_cmd_opts(opts, &optind, argc, argv, parser, streams); if (retval != STATUS_CMD_OK) return retval; + argv += optind; + argc -= optind; if (opts.print_help) { builtin_print_help(parser, streams, cmd, streams.out); return STATUS_CMD_OK; } - retval = validate_cmd_opts(cmd, opts, optind, argc, parser, streams); + retval = validate_cmd_opts(cmd, opts, argc, parser, streams); if (retval != STATUS_CMD_OK) return retval; - if (opts.query) return builtin_set_query(cmd, opts, optind, argc, argv, parser, streams); - if (opts.list || optind == argc) { - return builtin_set_list(cmd, opts, optind, argc, argv, parser, streams); - } - - // Variables used for performing the actual work. - int scope = compute_scope(opts); // calculate the variable scope based on the provided options - wchar_t *dest = wcsdup(argv[optind]); - assert(dest); - - bool slice = false; - if (wchar_t *p = wcschr(dest, L'[')) { - *p = L'\0'; - slice = true; - } - - if (!valid_var_name(dest)) { - streams.err.append_format(BUILTIN_ERR_VARNAME, cmd, dest); - builtin_print_help(parser, streams, cmd, streams.err); - return STATUS_INVALID_ARGS; - } - - // Set assignment can work in two modes, either using slices or using the whole array. We detect - // which mode is used here. - if (slice) { - std::vector indexes; - wcstring_list_t result; - - const env_var_t dest_str = env_get_string(dest, scope); - if (!dest_str.missing()) { - tokenize_variable_array(dest_str, result); - } else if (opts.erase) { - retval = STATUS_CMD_ERROR; - } - - if (retval == STATUS_CMD_OK) { - for (; optind < argc; optind++) { - if (!parse_index(indexes, argv[optind], dest, result.size(), streams)) { - builtin_print_help(parser, streams, cmd, streams.err); - retval = STATUS_CMD_ERROR; - break; - } - - size_t idx_count = indexes.size(); - size_t val_count = argc - optind - 1; - - if (!opts.erase) { - if (val_count < idx_count) { - streams.err.append_format(_(BUILTIN_SET_ARG_COUNT), cmd); - builtin_print_help(parser, streams, cmd, streams.err); - retval = STATUS_CMD_ERROR; - break; - } - if (val_count == idx_count) { - optind++; - break; - } - } - } - } - - if (retval == STATUS_CMD_OK) { - // Slice indexes have been calculated, do the actual work. - if (opts.erase) { - erase_values(result, indexes); - my_env_set(dest, result, scope, streams); - } else { - wcstring_list_t value; - - while (optind < argc) { - value.push_back(argv[optind++]); - } - - if (update_values(result, indexes, value)) { - streams.err.append_format(L"%ls: ", cmd); - streams.err.append_format(ARRAY_BOUNDS_ERR); - streams.err.push_back(L'\n'); - } - - my_env_set(dest, result, scope, streams); - } - } + if (opts.query) { + retval = builtin_set_query(cmd, opts, argc, argv, parser, streams); + } else if (opts.erase) { + retval = builtin_set_erase(cmd, opts, argc, argv, parser, streams); + } else if (opts.list) { // explicit list the vars we know about + retval = builtin_set_list(cmd, opts, argc, argv, parser, streams); + } else if (argc == 0) { // implicit list the vars we know about + retval = builtin_set_list(cmd, opts, argc, argv, parser, streams); } else { - optind++; - // No slicing. - if (opts.erase) { - if (optind != argc) { - streams.err.append_format(_(L"%ls: Values cannot be specfied with erase\n"), cmd); - builtin_print_help(parser, streams, cmd, streams.err); - retval = STATUS_CMD_ERROR; - } else { - retval = env_remove(dest, scope); - } - } else { - wcstring_list_t val; - for (int i = optind; i < argc; i++) val.push_back(argv[i]); - retval = my_env_set(dest, val, scope, streams); - } + retval = builtin_set_set(cmd, opts, argc, argv, parser, streams); } - // Check if we are setting variables above the effective scope. See - // https://github.com/fish-shell/fish-shell/issues/806 - env_var_t global_dest = env_get_string(dest, ENV_GLOBAL); - if (opts.universal && !global_dest.missing()) { - streams.err.append_format( - _(L"%ls: Warning: universal scope selected, but a global variable '%ls' exists.\n"), - L"set", dest); - } - - free(dest); - if (retval == STATUS_CMD_OK && opts.preserve_failure_exit_status) retval = incoming_exit_status; return retval; } diff --git a/src/expand.h b/src/expand.h index 39a102f02..a904090e7 100644 --- a/src/expand.h +++ b/src/expand.h @@ -89,9 +89,6 @@ enum expand_error_t { EXPAND_WILDCARD_MATCH }; -/// Error issued on array out of bounds. -#define ARRAY_BOUNDS_ERR _(L"Array index out of bounds") - /// Perform various forms of expansion on in, such as tilde expansion (\~USER becomes the users home /// directory), variable expansion (\$VAR_NAME becomes the value of the environment variable /// VAR_NAME), cmdsubst expansion and wildcard expansion. The results are inserted into the list diff --git a/tests/test4.in b/tests/test4.in index 08bc1be44..eba51c3e4 100644 --- a/tests/test4.in +++ b/tests/test4.in @@ -161,10 +161,10 @@ end; set -U -e baz -echo "Verify subcommand statuses" +echo "# Verify subcommand statuses" echo (false) $status (true) $status (false) $status -echo "Verify that set passes through exit status, except when passed -n or -q or -e" +echo "# Verify that set passes through exit status, except when passed -n or -q or -e" false ; set foo bar ; echo 1 $status # passthrough true ; set foo bar ; echo 2 $status # passthrough false ; set -q foo ; echo 3 $status # no passthrough @@ -178,7 +178,7 @@ false ; set foo (echo A; true) ; echo 10 $status $foo true ; set foo (echo B; false) ; echo 11 $status $foo true -echo "Verify set -ql behavior" # see 2502 +echo "# Verify set -ql behavior" # see 2502 function setql_check set -l setql_foo val if set -ql setql_foo diff --git a/tests/test4.out b/tests/test4.out index 903782b99..ede533712 100644 --- a/tests/test4.out +++ b/tests/test4.out @@ -20,9 +20,9 @@ Test 19 pass Test 20 pass Test 21 pass Test 22 pass -Verify subcommand statuses +# Verify subcommand statuses 1 0 1 -Verify that set passes through exit status, except when passed -n or -q or -e +# Verify that set passes through exit status, except when passed -n or -q or -e 1 1 2 0 3 0 @@ -34,5 +34,5 @@ Verify that set passes through exit status, except when passed -n or -q or -e 9 121 10 0 A 11 1 B -Verify set -ql behavior +# Verify set -ql behavior Pass