Add and use new exit code for env_remove() when var doesn't exist

The previous commit caused the tests to fail since env_remove() was
returning a blanket `!0` when a variable couldn't be unset because it
didn't exist in the first place. This caused the wrong message to be
emitted since the code clashed with a return code for `env_set()`.

Added `ENV_NOT_FOUND` to signify that the variable requested unset
didn't exist in the first place, but _not_ printing the error message
currently so as not to break existing behavior before checking if this
is something we want.
This commit is contained in:
Mahmoud Al-Qudsi 2018-03-31 22:12:52 -05:00
parent 0e0168ef18
commit 01452da5bf
3 changed files with 13 additions and 4 deletions

View file

@ -305,6 +305,12 @@ static void handle_env_return(int retval, const wchar_t *cmd, const wchar_t *key
retval = STATUS_CMD_ERROR;
break;
}
case ENV_NOT_FOUND: {
streams.err.append_format(
_(L"%ls: The variable '%ls' does not exist\n"), cmd, key);
retval = STATUS_CMD_ERROR;
break;
}
default: {
DIE("unexpected env_set() ret val");
break;
@ -622,6 +628,11 @@ static int builtin_set_erase(const wchar_t *cmd, set_cmd_opts_t &opts, int argc,
if (idx_count == 0) { // unset the var
retval = env_remove(dest, scope);
// Temporarily swallowing ENV_NOT_FOUND errors to prevent
// breaking all tests that unset variables that aren't set.
if (retval != ENV_NOT_FOUND) {
handle_env_return(retval, cmd, dest, streams);
}
} else { // remove just the specified indexes of the var
const auto dest_var = env_get(dest, scope);
if (!dest_var) return STATUS_CMD_ERROR;
@ -631,8 +642,6 @@ static int builtin_set_erase(const wchar_t *cmd, set_cmd_opts_t &opts, int argc,
retval = my_env_set(cmd, dest, scope, result, streams);
}
handle_env_return(retval, cmd, dest, streams);
if (retval != STATUS_CMD_OK) return retval;
return check_global_scope_exists(cmd, opts, dest, streams);
}

View file

@ -1286,7 +1286,7 @@ int env_remove(const wcstring &key, int var_mode) {
react_to_variable_change(L"ERASE", key);
return !erased;
return erased ? ENV_OK : ENV_NOT_FOUND;
}
const wcstring_list_t &env_var_t::as_list() const { return vals; }

View file

@ -44,7 +44,7 @@ enum {
typedef uint32_t env_mode_flags_t;
/// Return values for `env_set()`.
enum { ENV_OK, ENV_PERM, ENV_SCOPE, ENV_INVALID };
enum { ENV_OK, ENV_PERM, ENV_SCOPE, ENV_INVALID, ENV_NOT_FOUND };
/// A struct of configuration directories, determined in main() that fish will optionally pass to
/// env_init.