diff --git a/src/builtins/abbr.rs b/src/builtins/abbr.rs index 7e310e5c4..115fb057a 100644 --- a/src/builtins/abbr.rs +++ b/src/builtins/abbr.rs @@ -418,12 +418,12 @@ fn abbr_erase(opts: &Options, parser: &Parser) -> Option { return STATUS_CMD_ERROR; } - // Erase each. If any is not found, return ENV_NOT_FOUND which is historical. + // Erase each. If any is not found, return NotFound which is historical. abbrs::with_abbrs_mut(|abbrs| -> Option { let mut result = STATUS_CMD_OK; for arg in &opts.args { if !abbrs.erase(arg) { - result = Some(EnvStackSetResult::ENV_NOT_FOUND.into()); + result = Some(EnvStackSetResult::NotFound.into()); } // Erase the old uvar - this makes `abbr -e` work. let esc_src = escape(arg); @@ -431,7 +431,7 @@ fn abbr_erase(opts: &Options, parser: &Parser) -> Option { let var_name = WString::from_str("_fish_abbr_") + esc_src.as_utfstr(); let ret = parser.vars().remove(&var_name, EnvMode::UNIVERSAL); - if ret == EnvStackSetResult::ENV_OK { + if ret == EnvStackSetResult::Ok { result = STATUS_CMD_OK }; } diff --git a/src/builtins/set.rs b/src/builtins/set.rs index eea9a1928..1b22ae8b0 100644 --- a/src/builtins/set.rs +++ b/src/builtins/set.rs @@ -310,29 +310,29 @@ fn warn_if_uvar_shadows_global( fn handle_env_return(retval: EnvStackSetResult, cmd: &wstr, key: &wstr, streams: &mut IoStreams) { match retval { - EnvStackSetResult::ENV_OK => (), - EnvStackSetResult::ENV_PERM => { + EnvStackSetResult::Ok => (), + EnvStackSetResult::Perm => { streams.err.append(wgettext_fmt!( "%ls: Tried to change the read-only variable '%ls'\n", cmd, key )); } - EnvStackSetResult::ENV_SCOPE => { + EnvStackSetResult::Scope => { streams.err.append(wgettext_fmt!( "%ls: Tried to modify the special variable '%ls' with the wrong scope\n", cmd, key )); } - EnvStackSetResult::ENV_INVALID => { + EnvStackSetResult::Invalid => { streams.err.append(wgettext_fmt!( "%ls: Tried to modify the special variable '%ls' to an invalid value\n", cmd, key )); } - EnvStackSetResult::ENV_NOT_FOUND => { + EnvStackSetResult::NotFound => { streams.err.append(wgettext_fmt!( "%ls: The variable '%ls' does not exist\n", cmd, @@ -780,13 +780,13 @@ fn erase( if split.indexes.is_empty() { // unset the var retval = parser.vars().remove(split.varname, scope); - // When a non-existent-variable is unset, return ENV_NOT_FOUND as $status + // When a non-existent-variable is unset, return NotFound as $status // but do not emit any errors at the console as a compromise between user // friendliness and correctness. - if retval != EnvStackSetResult::ENV_NOT_FOUND { + if retval != EnvStackSetResult::NotFound { handle_env_return(retval, cmd, split.varname, streams); } - if retval == EnvStackSetResult::ENV_OK && !opts.no_event { + if retval == EnvStackSetResult::Ok && !opts.no_event { event::fire(parser, Event::variable_erase(split.varname.to_owned())); } } else { @@ -808,7 +808,7 @@ fn erase( // Set $status to the last error value. // This is cheesy, but I don't expect this to be checked often. - if retval != EnvStackSetResult::ENV_OK { + if retval != EnvStackSetResult::Ok { ret = env_result_to_status(retval); } } @@ -818,11 +818,11 @@ fn erase( fn env_result_to_status(retval: EnvStackSetResult) -> Option { Some(match retval { - EnvStackSetResult::ENV_OK => 0, - EnvStackSetResult::ENV_PERM => 1, - EnvStackSetResult::ENV_SCOPE => 2, - EnvStackSetResult::ENV_INVALID => 3, - EnvStackSetResult::ENV_NOT_FOUND => 4, + EnvStackSetResult::Ok => 0, + EnvStackSetResult::Perm => 1, + EnvStackSetResult::Scope => 2, + EnvStackSetResult::Invalid => 3, + EnvStackSetResult::NotFound => 4, }) } @@ -981,7 +981,7 @@ fn set_internal( let retval = env_set_reporting_errors(cmd, opts, split.varname, scope, new_values, streams, parser); - if retval == EnvStackSetResult::ENV_OK { + if retval == EnvStackSetResult::Ok { warn_if_uvar_shadows_global(cmd, opts, split.varname, streams, parser); } env_result_to_status(retval) diff --git a/src/env/environment.rs b/src/env/environment.rs index 008b547a3..8348b3504 100644 --- a/src/env/environment.rs +++ b/src/env/environment.rs @@ -42,27 +42,27 @@ static UVARS_LOCALLY_MODIFIED: RelaxedAtomicBool = RelaxedAtomicBool::new(false) /// Return values for `EnvStack::set()`. #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum EnvStackSetResult { - ENV_OK, - ENV_PERM, - ENV_SCOPE, - ENV_INVALID, - ENV_NOT_FOUND, + Ok, // The variable was set successfully. + Perm, // The variable is read-only. + Scope, // Variable cannot be set in the given scope. + Invalid, // The variable's value is invalid (e.g. umask). + NotFound, // The variable was not found (only possible when removing a variable). } impl Default for EnvStackSetResult { fn default() -> Self { - EnvStackSetResult::ENV_OK + EnvStackSetResult::Ok } } impl From for c_int { fn from(r: EnvStackSetResult) -> Self { match r { - EnvStackSetResult::ENV_OK => 0, - EnvStackSetResult::ENV_PERM => 1, - EnvStackSetResult::ENV_SCOPE => 2, - EnvStackSetResult::ENV_INVALID => 3, - EnvStackSetResult::ENV_NOT_FOUND => 4, + EnvStackSetResult::Ok => 0, + EnvStackSetResult::Perm => 1, + EnvStackSetResult::Scope => 2, + EnvStackSetResult::Invalid => 3, + EnvStackSetResult::NotFound => 4, } } } @@ -228,7 +228,7 @@ impl EnvStack { } let ret: ModResult = self.lock().set(key, mode, vals); - if ret.status == EnvStackSetResult::ENV_OK { + if ret.status == EnvStackSetResult::Ok { // If we modified the global state, or we are principal, then dispatch changes. // Important to not hold the lock here. if ret.global_modified || self.is_principal() { @@ -277,7 +277,7 @@ impl EnvStack { pub fn remove(&self, key: &wstr, mode: EnvMode) -> EnvStackSetResult { let ret = self.lock().remove(key, mode); #[allow(clippy::collapsible_if)] - if ret.status == EnvStackSetResult::ENV_OK { + if ret.status == EnvStackSetResult::Ok { if ret.global_modified || self.is_principal() { // Important to not hold the lock here. env_dispatch_var_change(key, self); diff --git a/src/env/environment_impl.rs b/src/env/environment_impl.rs index e1712cd6a..d88ae60ee 100644 --- a/src/env/environment_impl.rs +++ b/src/env/environment_impl.rs @@ -65,10 +65,10 @@ fn next_export_generation() -> ExportGeneration { fn set_umask(list_val: &Vec) -> EnvStackSetResult { if list_val.len() != 1 || list_val[0].is_empty() { - return EnvStackSetResult::ENV_INVALID; + return EnvStackSetResult::Invalid; } let Ok(mask) = fish_wcstol_radix(&list_val[0], 8) else { - return EnvStackSetResult::ENV_INVALID; + return EnvStackSetResult::Invalid; }; #[allow( @@ -77,12 +77,12 @@ fn set_umask(list_val: &Vec) -> EnvStackSetResult { clippy::absurd_extreme_comparisons )] if mask > 0o777 || mask < 0 { - return EnvStackSetResult::ENV_INVALID; + return EnvStackSetResult::Invalid; } // Do not actually create a umask variable. On env_stack_t::get() it will be calculated. // SAFETY: umask cannot fail. unsafe { libc::umask(mask as libc::mode_t) }; - EnvStackSetResult::ENV_OK + EnvStackSetResult::Ok } /// A query for environment variables. @@ -738,7 +738,7 @@ impl EnvStackImpl { flags.pathvar = Some(query.pathvar); } - let mut result = ModResult::new(EnvStackSetResult::ENV_OK); + let mut result = ModResult::new(EnvStackSetResult::Ok); if query.has_scope { // The user requested a particular scope. // If we don't have uvars, fall back to using globals. @@ -799,26 +799,26 @@ impl EnvStackImpl { let query = Query::new(mode); // Users can't remove read-only keys. if query.user && is_read_only(key) { - return ModResult::new(EnvStackSetResult::ENV_SCOPE); + return ModResult::new(EnvStackSetResult::Scope); } // Helper to invoke remove_from_chain and map a false return to not found. fn remove_from_chain(node: &mut EnvNodeRef, key: &wstr) -> EnvStackSetResult { if EnvStackImpl::remove_from_chain(node, key) { - EnvStackSetResult::ENV_OK + EnvStackSetResult::Ok } else { - EnvStackSetResult::ENV_NOT_FOUND + EnvStackSetResult::NotFound } } - let mut result = ModResult::new(EnvStackSetResult::ENV_OK); + let mut result = ModResult::new(EnvStackSetResult::Ok); if query.has_scope { // The user requested erasing from a particular scope. if query.universal { if uvars().remove(key) { - result.status = EnvStackSetResult::ENV_OK; + result.status = EnvStackSetResult::Ok; } else { - result.status = EnvStackSetResult::ENV_NOT_FOUND; + result.status = EnvStackSetResult::NotFound; } // Note we have historically set this even if the uvar is not found. result.uvar_modified = true; @@ -846,7 +846,7 @@ impl EnvStackImpl { } else if uvars().remove(key) { result.uvar_modified = true; } else { - result.status = EnvStackSetResult::ENV_NOT_FOUND; + result.status = EnvStackSetResult::NotFound; } result } @@ -937,12 +937,12 @@ impl EnvStackImpl { // If a variable is electric, it may only be set in the global scope. if query.has_scope && !query.global { - return Some(EnvStackSetResult::ENV_SCOPE); + return Some(EnvStackSetResult::Scope); } // If the variable is read-only, the user may not set it. if query.user && ev.readonly() { - return Some(EnvStackSetResult::ENV_PERM); + return Some(EnvStackSetResult::Perm); } // Be picky about exporting. @@ -953,7 +953,7 @@ impl EnvStackImpl { query.unexports }; if !matches { - return Some(EnvStackSetResult::ENV_SCOPE); + return Some(EnvStackSetResult::Scope); } } @@ -967,7 +967,7 @@ impl EnvStackImpl { self.base.perproc_data.pwd = pwd; self.base.globals.borrow_mut().changed_exported(); } - return Some(EnvStackSetResult::ENV_OK); + return Some(EnvStackSetResult::Ok); } // Claim the value. let val = std::mem::take(val); @@ -979,7 +979,7 @@ impl EnvStackImpl { pathvar: Some(false), }; Self::set_in_node(&mut self.base.globals, key, val, flags); - return Some(EnvStackSetResult::ENV_OK); + return Some(EnvStackSetResult::Ok); } /// Set a universal variable, inheriting as applicable from the given old variable. diff --git a/src/parse_execution.rs b/src/parse_execution.rs index 50c2cfd92..6bf6f0d56 100644 --- a/src/parse_execution.rs +++ b/src/parse_execution.rs @@ -990,7 +990,7 @@ impl<'a> ParseExecutionContext { EnvMode::LOCAL | EnvMode::USER, var.map_or(vec![], |var| var.as_list().to_owned()), ); - assert!(retval == EnvStackSetResult::ENV_OK); + assert!(retval == EnvStackSetResult::Ok); trace_if_enabled_with_args(ctx.parser(), L!("for"), &arguments); @@ -1010,7 +1010,7 @@ impl<'a> ParseExecutionContext { .vars() .set(&for_var_name, EnvMode::USER, vec![val]); assert!( - retval == EnvStackSetResult::ENV_OK, + retval == EnvStackSetResult::Ok, "for loop variable should have been successfully set" ); event::fire(ctx.parser(), evt.clone()); diff --git a/src/parser.rs b/src/parser.rs index e1502ccf6..7dccb02ef 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -847,7 +847,6 @@ impl Parser { } /// Cover of vars().set(), which also fires any returned event handlers. - /// Return a value like ENV_OK. pub fn set_var_and_fire( &self, key: &wstr, @@ -855,7 +854,7 @@ impl Parser { vals: Vec, ) -> EnvStackSetResult { let res = self.vars().set(key, mode, vals); - if res == EnvStackSetResult::ENV_OK { + if res == EnvStackSetResult::Ok { event::fire(self, Event::variable_set(key.to_owned())); } res diff --git a/src/tests/expand.rs b/src/tests/expand.rs index 092dae6f0..533091621 100644 --- a/src/tests/expand.rs +++ b/src/tests/expand.rs @@ -361,7 +361,7 @@ fn test_expand_overflow() { let parser = Parser::principal_parser().shared(); parser.vars().push(true); let set = parser.vars().set(L!("bigvar"), EnvMode::LOCAL, vals); - assert_eq!(set, EnvStackSetResult::ENV_OK); + assert_eq!(set, EnvStackSetResult::Ok); let mut errors = ParseErrorList::new(); let ctx =