diff --git a/src/bin/fish.rs b/src/bin/fish.rs index 74e516305..1b53a012f 100644 --- a/src/bin/fish.rs +++ b/src/bin/fish.rs @@ -27,10 +27,12 @@ use fish::future::IsSomeAnd; use fish::{ ast::Ast, - builtins::fish_indent, - builtins::fish_key_reader, - builtins::shared::{ - BUILTIN_ERR_MISSING, BUILTIN_ERR_UNKNOWN, STATUS_CMD_OK, STATUS_CMD_UNKNOWN, + builtins::{ + fish_indent, fish_key_reader, + shared::{ + BUILTIN_ERR_MISSING, BUILTIN_ERR_UNKNOWN, STATUS_CMD_ERROR, STATUS_CMD_OK, + STATUS_CMD_UNKNOWN, + }, }, common::{ escape, get_executable_path, save_term_foreground_process_group, scoped_push_replacer, @@ -482,8 +484,8 @@ fn read_init(parser: &Parser, paths: &ConfigPaths) { } } -fn run_command_list(parser: &Parser, cmds: &[OsString]) -> i32 { - let mut retval = STATUS_CMD_OK; +fn run_command_list(parser: &Parser, cmds: &[OsString]) -> Result<(), libc::c_int> { + let mut retval = Ok(()); for cmd in cmds { let cmd_wcs = str2wcstring(cmd.as_bytes()); @@ -497,16 +499,16 @@ fn run_command_list(parser: &Parser, cmds: &[OsString]) -> i32 { // Construct a parsed source ref. let ps = Arc::new(ParsedSource::new(cmd_wcs, ast)); let _ = parser.eval_parsed_source(&ps, &IoChain::new(), None, BlockType::top); - retval = STATUS_CMD_OK; + retval = Ok(()); } else { let backtrace = parser.get_backtrace(&cmd_wcs, &errors); eprintf!("%s", backtrace); // XXX: Why is this the return for "unknown command"? - retval = STATUS_CMD_UNKNOWN; + retval = Err(STATUS_CMD_UNKNOWN); } } - retval.unwrap() + retval } fn fish_parse_opt(args: &mut [WString], opts: &mut FishCmdOpts) -> ControlFlow { @@ -720,7 +722,7 @@ fn main() { } fn throwing_main() -> i32 { - let mut res = 1; + let mut res = Err(STATUS_CMD_ERROR); signal_unblock_all(); topic_monitor::topic_monitor_init(); @@ -855,7 +857,7 @@ fn throwing_main() -> i32 { L!("fish_default_key_bindings").to_owned(), ); if function::exists(L!("fish_default_key_bindings"), parser) { - run_command_list(parser, &[OsString::from("fish_default_key_bindings")]); + let _ = run_command_list(parser, &[OsString::from("fish_default_key_bindings")]); } } @@ -863,7 +865,7 @@ fn throwing_main() -> i32 { term_copy_modes(); // Stomp the exit status of any initialization commands (issue #635). - parser.set_last_statuses(Statuses::just(STATUS_CMD_OK.unwrap())); + parser.set_last_statuses(Statuses::just(STATUS_CMD_OK)); // TODO: if-let-chains if opts.profile_startup_output.is_some() && opts.profile_startup_output != opts.profile_output { @@ -942,7 +944,7 @@ fn throwing_main() -> i32 { Some(Arc::new(rel_filename.to_owned())), ); res = reader_read(parser, f.as_raw_fd(), &IoChain::new()); - if res != 0 { + if res.is_err() { FLOGF!( warning, wgettext!("Error while reading file %ls\n"), @@ -953,8 +955,8 @@ fn throwing_main() -> i32 { } } - let exit_status = if res != 0 { - STATUS_CMD_UNKNOWN.unwrap() + let exit_status = if res.is_err() { + STATUS_CMD_UNKNOWN } else { parser.get_last_status() }; diff --git a/src/builtins/abbr.rs b/src/builtins/abbr.rs index 323a6ace7..97ec5ed3c 100644 --- a/src/builtins/abbr.rs +++ b/src/builtins/abbr.rs @@ -124,7 +124,7 @@ fn join(list: &[&wstr], sep: &wstr) -> WString { } // Print abbreviations in a fish-script friendly way. -fn abbr_show(streams: &mut IoStreams) -> Option { +fn abbr_show(streams: &mut IoStreams) -> BuiltinResult { let style = EscapeStringStyle::Script(Default::default()); abbrs::with_abbrs(|abbrs| { @@ -174,11 +174,11 @@ fn abbr_show(streams: &mut IoStreams) -> Option { } }); - return STATUS_CMD_OK; + return Ok(SUCCESS); } // Print the list of abbreviation names. -fn abbr_list(opts: &Options, streams: &mut IoStreams) -> Option { +fn abbr_list(opts: &Options, streams: &mut IoStreams) -> BuiltinResult { const subcmd: &wstr = L!("--list"); if !opts.args.is_empty() { streams.err.append(wgettext_fmt!( @@ -187,7 +187,7 @@ fn abbr_list(opts: &Options, streams: &mut IoStreams) -> Option { subcmd, &opts.args[0] )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } abbrs::with_abbrs(|abbrs| { for abbr in abbrs.list() { @@ -197,11 +197,11 @@ fn abbr_list(opts: &Options, streams: &mut IoStreams) -> Option { } }); - return STATUS_CMD_OK; + return Ok(SUCCESS); } // Rename an abbreviation, deleting any existing one with the given name. -fn abbr_rename(opts: &Options, streams: &mut IoStreams) -> Option { +fn abbr_rename(opts: &Options, streams: &mut IoStreams) -> BuiltinResult { const subcmd: &wstr = L!("--rename"); if opts.args.len() != 2 { @@ -210,7 +210,7 @@ fn abbr_rename(opts: &Options, streams: &mut IoStreams) -> Option { CMD, subcmd )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } let old_name = &opts.args[0]; let new_name = &opts.args[1]; @@ -220,7 +220,7 @@ fn abbr_rename(opts: &Options, streams: &mut IoStreams) -> Option { CMD, subcmd )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if contains_whitespace(new_name) { @@ -230,9 +230,9 @@ fn abbr_rename(opts: &Options, streams: &mut IoStreams) -> Option { subcmd, new_name.as_utfstr() )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } - abbrs::with_abbrs_mut(|abbrs| -> Option { + abbrs::with_abbrs_mut(|abbrs| -> BuiltinResult { if !abbrs.has_name(old_name) { streams.err.append(wgettext_fmt!( "%ls %ls: No abbreviation named %ls\n", @@ -240,7 +240,7 @@ fn abbr_rename(opts: &Options, streams: &mut IoStreams) -> Option { subcmd, old_name.as_utfstr() )); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } if abbrs.has_name(new_name) { streams.err.append(wgettext_fmt!( @@ -250,10 +250,10 @@ fn abbr_rename(opts: &Options, streams: &mut IoStreams) -> Option { new_name.as_utfstr(), old_name.as_utfstr() )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } abbrs.rename(old_name, new_name); - STATUS_CMD_OK + Ok(SUCCESS) }) } @@ -262,20 +262,20 @@ fn contains_whitespace(val: &wstr) -> bool { } // Test if any args is an abbreviation. -fn abbr_query(opts: &Options) -> Option { +fn abbr_query(opts: &Options) -> BuiltinResult { // Return success if any of our args matches an abbreviation. abbrs::with_abbrs(|abbrs| { for arg in opts.args.iter() { if abbrs.has_name(arg) { - return STATUS_CMD_OK; + return Ok(SUCCESS); } } - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); }) } // Add a named abbreviation. -fn abbr_add(opts: &Options, streams: &mut IoStreams) -> Option { +fn abbr_add(opts: &Options, streams: &mut IoStreams) -> BuiltinResult { const subcmd: &wstr = L!("--add"); if opts.args.len() < 2 && opts.function.is_none() { @@ -284,7 +284,7 @@ fn abbr_add(opts: &Options, streams: &mut IoStreams) -> Option { CMD, subcmd )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if opts.args.is_empty() || opts.args[0].is_empty() { @@ -293,7 +293,7 @@ fn abbr_add(opts: &Options, streams: &mut IoStreams) -> Option { CMD, subcmd )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } let name = &opts.args[0]; if name.chars().any(|c| c.is_whitespace()) { @@ -303,7 +303,7 @@ fn abbr_add(opts: &Options, streams: &mut IoStreams) -> Option { subcmd, name.as_utfstr() )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } let key: &wstr; @@ -331,7 +331,7 @@ fn abbr_add(opts: &Options, streams: &mut IoStreams) -> Option { .err .append(sprintf!("%ls: %*ls\n", CMD, offset, "^")); } - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } let anchored = regex_make_anchored(regex_pattern); let re = Box::new( @@ -352,7 +352,7 @@ fn abbr_add(opts: &Options, streams: &mut IoStreams) -> Option { streams .err .append(wgettext_fmt!(BUILTIN_ERR_TOO_MANY_ARGUMENTS, L!("abbr"))); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } let replacement = if let Some(ref function) = opts.function { // Abbreviation function names disallow spaces. @@ -363,7 +363,7 @@ fn abbr_add(opts: &Options, streams: &mut IoStreams) -> Option { CMD, function.as_utfstr() )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } function.clone() } else { @@ -390,7 +390,7 @@ fn abbr_add(opts: &Options, streams: &mut IoStreams) -> Option { "%ls: --command cannot be combined with --position command", CMD, )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } // Note historically we have allowed overwriting existing abbreviations. @@ -408,22 +408,22 @@ fn abbr_add(opts: &Options, streams: &mut IoStreams) -> Option { }) }); - return STATUS_CMD_OK; + return Ok(SUCCESS); } // Erase the named abbreviations. -fn abbr_erase(opts: &Options, parser: &Parser) -> Option { +fn abbr_erase(opts: &Options, parser: &Parser) -> BuiltinResult { if opts.args.is_empty() { // This has historically been a silent failure. - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } // Erase each. If any is not found, return NotFound which is historical. - abbrs::with_abbrs_mut(|abbrs| -> Option { - let mut result = STATUS_CMD_OK; + abbrs::with_abbrs_mut(|abbrs| -> BuiltinResult { + let mut result: BuiltinResult = Ok(SUCCESS); for arg in &opts.args { if !abbrs.erase(arg) { - result = Some(EnvStackSetResult::NotFound.into()); + result = EnvStackSetResult::NotFound.into(); } // Erase the old uvar - this makes `abbr -e` work. let esc_src = escape(arg); @@ -432,7 +432,7 @@ fn abbr_erase(opts: &Options, parser: &Parser) -> Option { let ret = parser.vars().remove(&var_name, EnvMode::UNIVERSAL); if ret == EnvStackSetResult::Ok { - result = STATUS_CMD_OK + result = Ok(SUCCESS) }; } } @@ -440,7 +440,7 @@ fn abbr_erase(opts: &Options, parser: &Parser) -> Option { }) } -pub fn abbr(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Option { +pub fn abbr(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> BuiltinResult { let mut argv_read = Vec::with_capacity(argv.len()); argv_read.extend_from_slice(argv); @@ -504,7 +504,7 @@ pub fn abbr(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt "%ls: Cannot specify multiple positions\n", CMD )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if w.woptarg == Some(L!("command")) { opts.position = Some(Position::Command); @@ -519,7 +519,7 @@ pub fn abbr(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt streams .err .append(L!("Position must be one of: command, anywhere.\n")); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } } 'r' => { @@ -528,7 +528,7 @@ pub fn abbr(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt "%ls: Cannot specify multiple regex patterns\n", CMD )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } opts.regex_pattern = w.woptarg.map(ToOwned::to_owned); } @@ -538,7 +538,7 @@ pub fn abbr(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt "%ls: Cannot specify multiple set-cursor options\n", CMD )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } // The default set-cursor indicator is '%'. let _ = opts @@ -566,15 +566,15 @@ pub fn abbr(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt } 'h' => { builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } ':' => { builtin_missing_argument(parser, streams, cmd, argv[w.wopt_index - 1], true); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } '?' => { builtin_unknown_option(parser, streams, cmd, argv[w.wopt_index - 1], false); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } _ => { panic!("unexpected retval from wgeopter.next()"); @@ -587,7 +587,7 @@ pub fn abbr(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt } if !opts.validate(streams) { - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if opts.add { diff --git a/src/builtins/argparse.rs b/src/builtins/argparse.rs index a9503aa06..e60979b1a 100644 --- a/src/builtins/argparse.rs +++ b/src/builtins/argparse.rs @@ -86,7 +86,7 @@ fn new() -> Self { fn check_for_mutually_exclusive_flags( opts: &ArgParseCmdOpts, streams: &mut IoStreams, -) -> Option { +) -> BuiltinResult { for opt_spec in opts.options.values() { if opt_spec.num_seen == 0 { continue; @@ -143,19 +143,19 @@ fn check_for_mutually_exclusive_flags( flag1, flag2 )); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } } } } - return STATUS_CMD_OK; + Ok(SUCCESS) } // This should be called after all the option specs have been parsed. At that point we have enough // information to parse the values associated with any `--exclusive` flags. -fn parse_exclusive_args(opts: &mut ArgParseCmdOpts, streams: &mut IoStreams) -> Option { +fn parse_exclusive_args(opts: &mut ArgParseCmdOpts, streams: &mut IoStreams) -> BuiltinResult { for raw_xflags in &opts.raw_exclusive_flags { let xflags: Vec<_> = raw_xflags.split(',').collect(); if xflags.len() < 2 { @@ -164,7 +164,7 @@ fn parse_exclusive_args(opts: &mut ArgParseCmdOpts, streams: &mut IoStreams) -> opts.name, raw_xflags )); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } let exclusive_set: &mut Vec = &mut vec![]; @@ -182,14 +182,14 @@ fn parse_exclusive_args(opts: &mut ArgParseCmdOpts, streams: &mut IoStreams) -> opts.name, flag )); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } // Store the set of exclusive flags for use when parsing the supplied set of arguments. opts.exclusive_flag_sets.push(exclusive_set.to_vec()); } - return STATUS_CMD_OK; + Ok(SUCCESS) } fn parse_flag_modifiers<'args>( @@ -430,8 +430,10 @@ fn collect_option_specs<'args>( argc: usize, args: &[&'args wstr], streams: &mut IoStreams, -) -> Option { - let cmd: &wstr = args[0]; +) -> BuiltinResult { + let Some(&cmd) = args.get(0) else { + return Err(STATUS_INVALID_ARGS); + }; // A counter to give short chars to long-only options because getopt needs that. // Luckily we have wgetopt so we can use wchars - this is one of the private use areas so we @@ -443,7 +445,7 @@ fn collect_option_specs<'args>( streams .err .append(wgettext_fmt!("%ls: Missing -- separator\n", cmd)); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if "--" == args[*optind] { @@ -452,7 +454,7 @@ fn collect_option_specs<'args>( } if !parse_option_spec(opts, args[*optind], &mut counter, streams) { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } *optind += 1; @@ -465,10 +467,10 @@ fn collect_option_specs<'args>( streams .err .append(wgettext_fmt!("%ls: Too many long-only options\n", cmd)); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } - return STATUS_CMD_OK; + return Ok(SUCCESS); } fn parse_cmd_opts<'args>( @@ -478,8 +480,10 @@ fn parse_cmd_opts<'args>( args: &mut [&'args wstr], parser: &Parser, streams: &mut IoStreams, -) -> Option { - let cmd = args[0]; +) -> BuiltinResult { + let Some(&cmd) = args.get(0) else { + return Err(STATUS_INVALID_ARGS); + }; let mut args_read = Vec::with_capacity(args.len()); args_read.extend_from_slice(args); @@ -503,7 +507,7 @@ fn parse_cmd_opts<'args>( cmd, w.woptarg.unwrap() )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } x.try_into().unwrap() } @@ -517,7 +521,7 @@ fn parse_cmd_opts<'args>( cmd, w.woptarg.unwrap() )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } x.try_into().unwrap() } @@ -530,18 +534,18 @@ fn parse_cmd_opts<'args>( args[w.wopt_index - 1], /* print_hints */ false, ); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } '?' => { builtin_unknown_option(parser, streams, cmd, args[w.wopt_index - 1], false); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } _ => panic!("unexpected retval from next_opt"), } } if opts.print_help { - return STATUS_CMD_OK; + return Ok(SUCCESS); } if "--" == args_read[w.wopt_index - 1] { @@ -553,7 +557,7 @@ fn parse_cmd_opts<'args>( streams .err .append(wgettext_fmt!("%ls: Missing -- separator\n", cmd)); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if opts.name.is_empty() { @@ -607,10 +611,10 @@ fn validate_arg<'opts>( is_long_flag: bool, woptarg: &'opts wstr, streams: &mut IoStreams, -) -> Option { +) -> BuiltinResult { // Obviously if there is no arg validation command we assume the arg is okay. if opt_spec.validation_command.is_empty() { - return STATUS_CMD_OK; + return Ok(SUCCESS); } let vars = parser.vars(); @@ -648,7 +652,7 @@ fn validate_arg<'opts>( streams.err.append_char('\n'); } vars.pop(); - Some(retval) + retval.map(|()| SUCCESS) } /// Return whether the option 'opt' is an implicit integer option. @@ -670,13 +674,9 @@ fn validate_and_store_implicit_int<'args>( w: &mut WGetopter, is_long_flag: bool, streams: &mut IoStreams, -) -> Option { +) -> BuiltinResult { let opt_spec = opts.options.get_mut(&opts.implicit_int_flag).unwrap(); - let retval = validate_arg(parser, &opts.name, opt_spec, is_long_flag, val, streams); - - if retval != STATUS_CMD_OK { - return retval; - } + validate_arg(parser, &opts.name, opt_spec, is_long_flag, val, streams)?; // It's a valid integer so store it and return success. opt_spec.vals.clear(); @@ -684,7 +684,7 @@ fn validate_and_store_implicit_int<'args>( opt_spec.num_seen += 1; w.remaining_text = L!(""); - return STATUS_CMD_OK; + Ok(SUCCESS) } fn handle_flag<'args>( @@ -694,7 +694,7 @@ fn handle_flag<'args>( is_long_flag: bool, woptarg: Option<&'args wstr>, streams: &mut IoStreams, -) -> Option { +) -> BuiltinResult { let opt_spec = opts.options.get_mut(&opt).unwrap(); opt_spec.num_seen += 1; @@ -708,14 +708,11 @@ fn handle_flag<'args>( WString::from_chars(['-', opt_spec.short_flag]) }; opt_spec.vals.push(s); - return STATUS_CMD_OK; + return Ok(SUCCESS); } if let Some(woptarg) = woptarg { - let retval = validate_arg(parser, &opts.name, opt_spec, is_long_flag, woptarg, streams); - if retval != STATUS_CMD_OK { - return retval; - } + validate_arg(parser, &opts.name, opt_spec, is_long_flag, woptarg, streams)?; } match opt_spec.num_allowed { @@ -733,7 +730,7 @@ fn handle_flag<'args>( } } - return STATUS_CMD_OK; + Ok(SUCCESS) } fn argparse_parse_flags<'args>( @@ -743,7 +740,7 @@ fn argparse_parse_flags<'args>( args: &mut [&'args wstr], optind: &mut usize, streams: &mut IoStreams, -) -> Option { +) -> BuiltinResult { let mut args_read = Vec::with_capacity(args.len()); args_read.extend_from_slice(args); @@ -756,7 +753,7 @@ fn argparse_parse_flags<'args>( let mut w = WGetopter::new(&short_options, &long_options, args); while let Some((opt, longopt_idx)) = w.next_opt_indexed() { let is_long_flag = longopt_idx.is_some(); - let retval = match opt { + let retval: BuiltinResult = match opt { ':' => { builtin_missing_argument( parser, @@ -765,7 +762,7 @@ fn argparse_parse_flags<'args>( args_read[w.wopt_index - 1], false, ); - STATUS_INVALID_ARGS + Err(STATUS_INVALID_ARGS) } '?' => { // It's not a recognized flag. See if it's an implicit int flag. @@ -786,7 +783,7 @@ fn argparse_parse_flags<'args>( opts.name, args_read[w.wopt_index - 1] )); - STATUS_INVALID_ARGS + Err(STATUS_INVALID_ARGS) } else { // Any unrecognized option is put back if ignore_unknown is used. // This allows reusing the same argv in multiple argparse calls, @@ -799,7 +796,7 @@ fn argparse_parse_flags<'args>( // Explain to wgetopt that we want to skip to the next arg, // because we can't handle this opt group. w.remaining_text = L!(""); - STATUS_CMD_OK + Ok(SUCCESS) } } NON_OPTION_CHAR => { @@ -814,13 +811,11 @@ fn argparse_parse_flags<'args>( // It's a recognized flag. _ => handle_flag(parser, opts, opt, is_long_flag, w.woptarg, streams), }; - if retval != STATUS_CMD_OK { - return retval; - } + retval?; } *optind = w.wopt_index; - return STATUS_CMD_OK; + return Ok(SUCCESS); } // This function mimics the `next_opt()` usage found elsewhere in our other builtin commands. @@ -832,31 +827,25 @@ fn argparse_parse_args<'args>( argc: usize, parser: &Parser, streams: &mut IoStreams, -) -> Option { +) -> BuiltinResult { if argc <= 1 { - return STATUS_CMD_OK; + return Ok(SUCCESS); } let mut optind = 0usize; - let retval = argparse_parse_flags(parser, opts, argc, args, &mut optind, streams); - if retval != STATUS_CMD_OK { - return retval; - } + argparse_parse_flags(parser, opts, argc, args, &mut optind, streams)?; - let retval = check_for_mutually_exclusive_flags(opts, streams); - if retval != STATUS_CMD_OK { - return retval; - } + check_for_mutually_exclusive_flags(opts, streams)?; opts.args.extend_from_slice(&args[optind..]); - return STATUS_CMD_OK; + Ok(SUCCESS) } fn check_min_max_args_constraints( opts: &ArgParseCmdOpts, streams: &mut IoStreams, -) -> Option { +) -> BuiltinResult { let cmd = &opts.name; if opts.args.len() < opts.min_args { @@ -866,7 +855,7 @@ fn check_min_max_args_constraints( opts.min_args, opts.args.len() )); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } if opts.max_args != usize::MAX && opts.args.len() > opts.max_args { @@ -876,10 +865,10 @@ fn check_min_max_args_constraints( opts.max_args, opts.args.len() )); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } - return STATUS_CMD_OK; + Ok(SUCCESS) } /// Put the result of parsing the supplied args into the caller environment as local vars. @@ -918,14 +907,17 @@ fn set_argparse_result_vars(vars: &EnvStack, opts: &ArgParseCmdOpts) { /// an external command also means its output has to be in a form that can be eval'd. Because our /// version is a builtin it can directly set variables local to the current scope (e.g., a /// function). It doesn't need to write anything to stdout that then needs to be eval'd. -pub fn argparse(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { - let cmd = args[0]; +pub fn argparse(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { + let Some(&cmd) = args.get(0) else { + return Err(STATUS_INVALID_ARGS); + }; + let argc = args.len(); let mut opts = ArgParseCmdOpts::new(); let mut optind = 0usize; let retval = parse_cmd_opts(&mut opts, &mut optind, argc, args, parser, streams); - if retval != STATUS_CMD_OK { + if retval.is_err() { // This is an error in argparse usage, so we append the error trailer with a stack trace. // The other errors are an error in using *the command* that is using argparse, // so our help doesn't apply. @@ -935,34 +927,25 @@ pub fn argparse(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> if opts.print_help { builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } - let retval = parse_exclusive_args(&mut opts, streams); - if retval != STATUS_CMD_OK { - return retval; - } + parse_exclusive_args(&mut opts, streams)?; // wgetopt expects the first argument to be the command, and skips it. // if optind was 0 we'd already have returned. assert!(optind > 0, "Optind is 0?"); - let retval = argparse_parse_args( + argparse_parse_args( &mut opts, &mut args[optind - 1..], argc - optind + 1, parser, streams, - ); - if retval != STATUS_CMD_OK { - return retval; - } + )?; - let retval = check_min_max_args_constraints(&opts, streams); - if retval != STATUS_CMD_OK { - return retval; - } + check_min_max_args_constraints(&opts, streams)?; set_argparse_result_vars(parser.vars(), &opts); - return retval; + Ok(SUCCESS) } diff --git a/src/builtins/bg.rs b/src/builtins/bg.rs index 10373603a..42fbe5739 100644 --- a/src/builtins/bg.rs +++ b/src/builtins/bg.rs @@ -10,7 +10,7 @@ fn send_to_bg( streams: &mut IoStreams, cmd: &wstr, job_pos: usize, -) -> Option { +) -> BuiltinResult { { let jobs = parser.jobs(); if !jobs[job_pos].wants_job_control() { @@ -24,7 +24,7 @@ fn send_to_bg( ) }; builtin_print_help_error(parser, streams, cmd, &err); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } let job = &jobs[job_pos]; @@ -37,26 +37,25 @@ fn send_to_bg( job.group().set_is_foreground(false); if !job.resume() { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } parser.job_promote_at(job_pos); - return STATUS_CMD_OK; + return Ok(SUCCESS); } /// Builtin for putting a job in the background. -pub fn bg(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { - let opts = match HelpOnlyCmdOpts::parse(args, parser, streams) { - Ok(opts) => opts, - Err(err @ Some(_)) if err != STATUS_CMD_OK => return err, - Err(err) => panic!("Illogical exit code from parse_options(): {err:?}"), +pub fn bg(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { + let opts = HelpOnlyCmdOpts::parse(args, parser, streams)?; + + let Some(&cmd) = args.get(0) else { + return Err(STATUS_INVALID_ARGS); }; - let cmd = args[0]; if opts.print_help { - builtin_print_help(parser, streams, args.get(0)?); - return STATUS_CMD_OK; + builtin_print_help(parser, streams, cmd); + return Ok(SUCCESS); } if opts.optind == args.len() { @@ -71,7 +70,7 @@ pub fn bg(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Optio streams .err .append(wgettext_fmt!("%ls: There are no suitable jobs\n", cmd)); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); }; return send_to_bg(parser, streams, cmd, job_pos); @@ -80,7 +79,7 @@ pub fn bg(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Optio // The user specified at least one job to be backgrounded. // If one argument is not a valid pid (i.e. integer >= 0), fail without backgrounding anything, // but still print errors for all of them. - let mut retval: Option = STATUS_CMD_OK; + let mut retval: BuiltinResult = Ok(SUCCESS); let pids: Vec = args[opts.optind..] .iter() .filter_map(|arg| match fish_wcstoi(arg).map(Pid::new) { @@ -91,21 +90,19 @@ pub fn bg(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Optio cmd, arg )); - retval = STATUS_INVALID_ARGS; + retval = Err(STATUS_INVALID_ARGS); None } }) .collect(); - if retval != STATUS_CMD_OK { - return retval; - } + retval?; // Background all existing jobs that match the pids. // Non-existent jobs aren't an error, but information about them is useful. for pid in pids { if let Some((job_pos, _job)) = parser.job_get_with_index_from_pid(pid) { - send_to_bg(parser, streams, cmd, job_pos); + send_to_bg(parser, streams, cmd, job_pos)?; } else { streams .err @@ -113,5 +110,5 @@ pub fn bg(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Optio } } - return STATUS_CMD_OK; + return Ok(SUCCESS); } diff --git a/src/builtins/bind.rs b/src/builtins/bind.rs index b930fcdb4..ac89b65f8 100644 --- a/src/builtins/bind.rs +++ b/src/builtins/bind.rs @@ -448,7 +448,7 @@ fn parse_cmd_opts( argv: &mut [&wstr], parser: &Parser, streams: &mut IoStreams, -) -> Option { +) -> BuiltinResult { let cmd = argv[0]; let short_options = L!(":aehkKfM:Lm:s"); const long_options: &[WOption] = &[ @@ -477,7 +477,7 @@ fn parse_cmd_opts( 'K' => opts.mode = BIND_KEY_NAMES, 'L' => { opts.list_modes = true; - return STATUS_CMD_OK; + return Ok(SUCCESS); } 'M' => { if !valid_var_name(w.woptarg.unwrap()) { @@ -486,7 +486,7 @@ fn parse_cmd_opts( cmd, w.woptarg.unwrap() )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } opts.bind_mode = w.woptarg.unwrap().to_owned(); opts.bind_mode_given = true; @@ -497,7 +497,7 @@ fn parse_cmd_opts( streams .err .append(wgettext_fmt!(BUILTIN_ERR_BIND_MODE, cmd, new_mode)); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } opts.sets_bind_mode = Some(new_mode.to_owned()); } @@ -512,11 +512,11 @@ fn parse_cmd_opts( } ':' => { builtin_missing_argument(parser, streams, cmd, argv[w.wopt_index - 1], true); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } '?' => { builtin_unknown_option(parser, streams, cmd, argv[w.wopt_index - 1], true); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } _ => { panic!("unexpected retval from WGetopter") @@ -524,7 +524,7 @@ fn parse_cmd_opts( } } *optind = w.wopt_index; - return STATUS_CMD_OK; + return Ok(SUCCESS); } impl BuiltinBind { @@ -534,22 +534,19 @@ pub fn bind( parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr], - ) -> Option { + ) -> BuiltinResult { let cmd = argv[0]; let mut optind = 0; - let retval = parse_cmd_opts(&mut self.opts, &mut optind, argv, parser, streams); - if retval != STATUS_CMD_OK { - return retval; - } + parse_cmd_opts(&mut self.opts, &mut optind, argv, parser, streams)?; if self.opts.list_modes { self.list_modes(streams); - return STATUS_CMD_OK; + return Ok(SUCCESS); } if self.opts.print_help { builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } // Default to user mode @@ -567,7 +564,7 @@ pub fn bind( true, /* user */ streams, ) { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } if self.opts.preset { @@ -577,13 +574,13 @@ pub fn bind( false, /* user */ streams, ) { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } } BIND_INSERT => { if self.insert(optind, argv, parser, streams) { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } BIND_KEY_NAMES => self.key_names(self.opts.all, streams), @@ -592,13 +589,13 @@ pub fn bind( streams .err .append(wgettext_fmt!("%ls: Invalid state\n", cmd)); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } - STATUS_CMD_OK + Ok(SUCCESS) } } -pub fn bind(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { +pub fn bind(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { BuiltinBind::new().bind(parser, streams, args) } diff --git a/src/builtins/block.rs b/src/builtins/block.rs index 2e2ad7e0b..46f645c66 100644 --- a/src/builtins/block.rs +++ b/src/builtins/block.rs @@ -27,7 +27,7 @@ fn parse_options( args: &mut [&wstr], parser: &Parser, streams: &mut IoStreams, -) -> Result<(Options, usize), Option> { +) -> Result<(Options, usize), ErrorCode> { let cmd = args[0]; const SHORT_OPTS: &wstr = L!(":eghl"); @@ -73,18 +73,14 @@ fn parse_options( } /// The block builtin, used for temporarily blocking events. -pub fn block(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { +pub fn block(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { let cmd = args[0]; - let opts = match parse_options(args, parser, streams) { - Ok((opts, _)) => opts, - Err(err @ Some(_)) if err != STATUS_CMD_OK => return err, - Err(err) => panic!("Illogical exit code from parse_options(): {err:?}"), - }; + let (opts, _) = parse_options(args, parser, streams)?; if opts.print_help { builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } if opts.erase { @@ -93,17 +89,17 @@ pub fn block(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Op "%ls: Can not specify scope when removing block\n", cmd )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if parser.global_event_blocks.load(Ordering::Relaxed) == 0 { streams .err .append(wgettext_fmt!("%ls: No blocks defined\n", cmd)); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } parser.global_event_blocks.fetch_sub(1, Ordering::Relaxed); - return STATUS_CMD_OK; + return Ok(SUCCESS); } let mut block_idx = 0; @@ -144,5 +140,5 @@ pub fn block(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Op parser.global_event_blocks.fetch_add(1, Ordering::Relaxed); } - return STATUS_CMD_OK; + return Ok(SUCCESS); } diff --git a/src/builtins/builtin.rs b/src/builtins/builtin.rs index 322e9c0f1..042da2d97 100644 --- a/src/builtins/builtin.rs +++ b/src/builtins/builtin.rs @@ -6,7 +6,7 @@ struct builtin_cmd_opts_t { list_names: bool, } -pub fn r#builtin(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Option { +pub fn r#builtin(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> BuiltinResult { let cmd = argv[0]; let argc = argv.len(); let print_hints = false; @@ -26,15 +26,15 @@ pub fn r#builtin(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) - 'n' => opts.list_names = true, 'h' => { builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } ':' => { builtin_missing_argument(parser, streams, cmd, argv[w.wopt_index - 1], print_hints); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } '?' => { builtin_unknown_option(parser, streams, cmd, argv[w.wopt_index - 1], print_hints); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } _ => { panic!("unexpected retval from WGetopter"); @@ -48,7 +48,7 @@ pub fn r#builtin(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) - cmd, wgettext!("--query and --names are mutually exclusive") )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } // If we don't have either, we print our help. @@ -56,17 +56,17 @@ pub fn r#builtin(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) - // the other decorator/builtins do. if !opts.query && !opts.list_names { builtin_print_help(parser, streams, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if opts.query { let optind = w.wopt_index; for arg in argv.iter().take(argc).skip(optind) { if builtin_exists(arg) { - return STATUS_CMD_OK; + return Ok(SUCCESS); } } - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } if opts.list_names { @@ -77,5 +77,5 @@ pub fn r#builtin(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) - } } - STATUS_CMD_OK + Ok(SUCCESS) } diff --git a/src/builtins/cd.rs b/src/builtins/cd.rs index 23c87df9d..f5caaa6cb 100644 --- a/src/builtins/cd.rs +++ b/src/builtins/cd.rs @@ -13,18 +13,16 @@ // The cd builtin. Changes the current directory to the one specified or to $HOME if none is // specified. The directory can be relative to any directory in the CDPATH variable. -pub fn cd(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { - let cmd = args[0]; - - let opts = match HelpOnlyCmdOpts::parse(args, parser, streams) { - Ok(opts) => opts, - Err(err @ Some(_)) if err != STATUS_CMD_OK => return err, - Err(err) => panic!("Illogical exit code from parse_options(): {err:?}"), +pub fn cd(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { + let Some(&cmd) = args.get(0) else { + return Err(STATUS_INVALID_ARGS); }; + let opts = HelpOnlyCmdOpts::parse(args, parser, streams)?; + if opts.print_help { builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } let vars = parser.vars(); @@ -42,7 +40,7 @@ pub fn cd(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Optio streams .err .append(wgettext_fmt!("%ls: Could not find home directory\n", cmd)); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } }; @@ -57,7 +55,7 @@ pub fn cd(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Optio if !parser.is_interactive() { streams.err.append(parser.current_line()); }; - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } let pwd = vars.get_pwd_slash(); @@ -74,7 +72,7 @@ pub fn cd(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Optio streams.err.append(parser.current_line()); } - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } let mut best_errno = 0; @@ -130,7 +128,7 @@ pub fn cd(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Optio parser.libdata_mut().cwd_fd = Some(dir_fd); parser.set_var_and_fire(L!("PWD"), EnvMode::EXPORT | EnvMode::GLOBAL, vec![norm_dir]); - return STATUS_CMD_OK; + return Ok(SUCCESS); } if best_errno == ENOTDIR { @@ -178,5 +176,5 @@ pub fn cd(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Optio streams.err.append(parser.current_line()); } - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } diff --git a/src/builtins/command.rs b/src/builtins/command.rs index cd579c1c2..5e17ffb1a 100644 --- a/src/builtins/command.rs +++ b/src/builtins/command.rs @@ -8,7 +8,7 @@ struct command_cmd_opts_t { find_path: bool, } -pub fn r#command(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Option { +pub fn r#command(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> BuiltinResult { let cmd = argv[0]; let argc = argv.len(); let print_hints = false; @@ -33,15 +33,15 @@ pub fn r#command(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) - 'v' => opts.find_path = true, 'h' => { builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } ':' => { builtin_missing_argument(parser, streams, cmd, argv[w.wopt_index - 1], print_hints); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } '?' => { builtin_unknown_option(parser, streams, cmd, argv[w.wopt_index - 1], print_hints); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } _ => { panic!("unexpected retval from wgeopter.next()"); @@ -52,7 +52,7 @@ pub fn r#command(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) - // Quiet implies find_path. if !opts.find_path && !opts.all && !opts.quiet { builtin_print_help(parser, streams, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } let mut res = false; @@ -69,7 +69,7 @@ pub fn r#command(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) - for path in paths.iter() { res = true; if opts.quiet { - return STATUS_CMD_OK; + return Ok(SUCCESS); } streams.out.appendln(path); @@ -80,8 +80,8 @@ pub fn r#command(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) - } if res { - STATUS_CMD_OK + Ok(SUCCESS) } else { - STATUS_CMD_UNKNOWN + Err(STATUS_CMD_UNKNOWN) } } diff --git a/src/builtins/commandline.rs b/src/builtins/commandline.rs index 4944b4dfc..849595beb 100644 --- a/src/builtins/commandline.rs +++ b/src/builtins/commandline.rs @@ -236,7 +236,7 @@ fn write_part( } /// The commandline builtin. It is used for specifying a new value for the commandline. -pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { +pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { let rstate = commandline_get_state(true); let mut buffer_part = None; @@ -315,7 +315,7 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) wgettext!("--tokens options are mutually exclusive") )); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } token_mode = Some(match c { 'x' => TokenMode::Expanded, @@ -342,15 +342,15 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) '\x04' => showing_suggestion = true, 'h' => { builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } ':' => { builtin_missing_argument(parser, streams, cmd, w.argv[w.wopt_index - 1], true); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } '?' => { builtin_unknown_option(parser, streams, cmd, w.argv[w.wopt_index - 1], true); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } _ => panic!(), } @@ -374,12 +374,12 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) { streams.err.append(wgettext_fmt!(BUILTIN_ERR_COMBO, cmd)); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if positional_args == 0 { builtin_missing_argument(parser, streams, cmd, L!("--function"), true); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } type RL = ReadlineCmd; @@ -389,7 +389,7 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) .err .append(wgettext_fmt!("%ls: Unknown input function '%ls'", cmd, arg)); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); }; // Don't enqueue a repaint if we're currently in the middle of one, // because that's an infinite loop. @@ -403,14 +403,14 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) reader_execute_readline_cmd(parser, CharEvent::from_readline(cmd)); } - return STATUS_CMD_OK; + return Ok(SUCCESS); } if selection_mode { if let Some(selection) = rstate.selection { streams.out.append(&rstate.text[selection]); } - return STATUS_CMD_OK; + return Ok(SUCCESS); } // Check for invalid switch combinations. @@ -419,7 +419,7 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) .err .append(wgettext_fmt!(BUILTIN_ERR_TOO_MANY_ARGUMENTS, cmd)); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if (search_mode || line_mode || column_mode || cursor_mode || paging_mode) @@ -429,7 +429,7 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) .err .append(wgettext_fmt!(BUILTIN_ERR_TOO_MANY_ARGUMENTS, cmd)); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if (buffer_part.is_some() || token_mode.is_some() || cut_at_cursor || search_field_mode) @@ -439,7 +439,7 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) { streams.err.append(wgettext_fmt!(BUILTIN_ERR_COMBO, cmd)); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if (token_mode.is_some() || cut_at_cursor) && positional_args != 0 { @@ -449,18 +449,18 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) "--cut-at-cursor and --tokens can not be used when setting the commandline" )); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if search_field_mode && (buffer_part.is_some() || token_mode.is_some()) { streams.err.append(wgettext_fmt!(BUILTIN_ERR_COMBO, cmd)); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if append_mode.is_some() && positional_args == 0 { // No tokens in insert mode just means we do nothing. - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } // Set default modes. @@ -477,7 +477,7 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) "--search-field" )); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } match buffer_part { TextScope::String | TextScope::Job | TextScope::Process => (), @@ -490,7 +490,7 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) "--current-token" )); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } } } @@ -513,7 +513,7 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) .err .append(wgettext_fmt!("%ls: line/column index starts at 1", cmd)); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); }; let new_pos = if line_mode { @@ -525,7 +525,7 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) .err .append(wgettext_fmt!("%ls: there is no line %ls\n", cmd, arg)); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); }; offset } else { @@ -543,7 +543,7 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) arg )); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } line_offset + new_coord }; @@ -564,47 +564,47 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) } )); } - return STATUS_CMD_OK; + return Ok(SUCCESS); } if search_mode { return if rstate.search_mode { - STATUS_CMD_OK + Ok(SUCCESS) } else { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) }; } if paging_mode { return if rstate.pager_mode { - STATUS_CMD_OK + Ok(SUCCESS) } else { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) }; } if paging_full_mode { return if rstate.pager_mode && rstate.pager_fully_disclosed { - STATUS_CMD_OK + Ok(SUCCESS) } else { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) }; } if selection_start_mode { let Some(selection) = rstate.selection else { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); }; streams.out.append(sprintf!("%lu\n", selection.start)); - return STATUS_CMD_OK; + return Ok(SUCCESS); } if selection_end_mode { let Some(selection) = rstate.selection else { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); }; streams.out.append(sprintf!("%lu\n", selection.end)); - return STATUS_CMD_OK; + return Ok(SUCCESS); } // At this point we have (nearly) exhausted the options which always operate on the true command @@ -617,7 +617,7 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) if search_field_mode { let Some((search_field_text, cursor_pos)) = rstate.search_field else { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); }; transient = search_field_text; current_buffer = &transient; @@ -647,21 +647,21 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) .append(L!(": Can not set commandline in non-interactive mode\n")); builtin_print_error_trailer(parser, streams.err, cmd); } - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } if is_valid { if current_buffer.is_empty() { - return Some(1); + return Err(STATUS_CMD_ERROR); } let res = parse_util_detect_errors(current_buffer, None, /*accept_incomplete=*/ true); return match res { - Ok(()) => STATUS_CMD_OK, + Ok(()) => Ok(SUCCESS), Err(err) => { if err.contains(ParserTestErrorBits::INCOMPLETE) { - Some(2) + Err(STATUS_INVALID_ARGS) } else { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) } } }; @@ -669,9 +669,9 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) if showing_suggestion { if reader_showing_suggestion(parser) { - return Some(0); + return Ok(SUCCESS); } - return Some(1); + return Err(STATUS_CMD_ERROR); } let range; @@ -720,7 +720,7 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) .out .append(sprintf!("%lu\n", current_cursor_pos - range.start)); } - return STATUS_CMD_OK; + return Ok(SUCCESS); } if positional_args == 0 { @@ -755,5 +755,5 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) ); } - STATUS_CMD_OK + Ok(SUCCESS) } diff --git a/src/builtins/complete.rs b/src/builtins/complete.rs index e87241182..3cd24c4e3 100644 --- a/src/builtins/complete.rs +++ b/src/builtins/complete.rs @@ -217,7 +217,7 @@ fn builtin_complete_print(cmd: &wstr, streams: &mut IoStreams, parser: &Parser) /// The complete builtin. Used for specifying programmable tab-completions. Calls the functions in /// complete.rs for any heavy lifting. -pub fn complete(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Option { +pub fn complete(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> BuiltinResult { let cmd = argv[0]; let argc = argv.len(); let mut result_mode = CompletionMode::default(); @@ -302,7 +302,7 @@ pub fn complete(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> cmd, w.woptarg.unwrap() )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } } 'd' => { @@ -321,7 +321,7 @@ pub fn complete(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> streams .err .append(wgettext_fmt!("%ls: -s requires a non-empty string\n", cmd,)); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } } 'l' => { @@ -331,7 +331,7 @@ pub fn complete(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> streams .err .append(wgettext_fmt!("%ls: -l requires a non-empty string\n", cmd,)); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } } 'o' => { @@ -341,7 +341,7 @@ pub fn complete(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> streams .err .append(wgettext_fmt!("%ls: -o requires a non-empty string\n", cmd,)); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } } 'S' => { @@ -351,7 +351,7 @@ pub fn complete(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> streams .err .append(wgettext_fmt!("%ls: -S requires a non-empty string\n", cmd,)); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } } 'a' => { @@ -375,15 +375,15 @@ pub fn complete(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> } 'h' => { builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } ':' => { builtin_missing_argument(parser, streams, cmd, argv[w.wopt_index - 1], true); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } '?' => { builtin_unknown_option(parser, streams, cmd, argv[w.wopt_index - 1], true); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } _ => panic!("unexpected retval from WGetopter"), } @@ -405,7 +405,7 @@ pub fn complete(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> "'--exclusive' and '--force-files'" )); } - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if w.wopt_index != argc { @@ -421,7 +421,7 @@ pub fn complete(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> .err .append(wgettext_fmt!(BUILTIN_ERR_TOO_MANY_ARGUMENTS, cmd)); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } } @@ -438,7 +438,7 @@ pub fn complete(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> )); streams.err.push('\n'); } - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } @@ -455,7 +455,7 @@ pub fn complete(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> )); streams.err.append(err_text); streams.err.push('\n'); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } @@ -469,7 +469,7 @@ pub fn complete(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> // This corresponds to using 'complete -C' in non-interactive mode. // See #2361 . builtin_missing_argument(parser, streams, cmd, L!("-C"), true); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } commandline_state.text } @@ -611,5 +611,5 @@ pub fn complete(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> } } - STATUS_CMD_OK + Ok(SUCCESS) } diff --git a/src/builtins/contains.rs b/src/builtins/contains.rs index b2b275aa6..4ce17b8bd 100644 --- a/src/builtins/contains.rs +++ b/src/builtins/contains.rs @@ -11,7 +11,7 @@ fn parse_options( args: &mut [&wstr], parser: &Parser, streams: &mut IoStreams, -) -> Result<(Options, usize), Option> { +) -> Result<(Options, usize), ErrorCode> { let cmd = args[0]; const SHORT_OPTS: &wstr = L!("+:hi"); @@ -46,18 +46,14 @@ fn parse_options( /// Implementation of the builtin contains command, used to check if a specified string is part of /// a list. -pub fn contains(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { +pub fn contains(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { let cmd = args[0]; - let (opts, optind) = match parse_options(args, parser, streams) { - Ok((opts, optind)) => (opts, optind), - Err(err @ Some(_)) if err != STATUS_CMD_OK => return err, - Err(err) => panic!("Illogical exit code from parse_options(): {err:?}"), - }; + let (opts, optind) = parse_options(args, parser, streams)?; if opts.print_help { builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } let needle = args.get(optind); @@ -67,7 +63,7 @@ pub fn contains(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> if opts.print_index { streams.out.appendln(i.to_wstring()); } - return STATUS_CMD_OK; + return Ok(SUCCESS); } } } else { @@ -76,5 +72,5 @@ pub fn contains(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> .append(wgettext_fmt!("%ls: Key not specified\n", cmd)); } - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } diff --git a/src/builtins/count.rs b/src/builtins/count.rs index 63298e02d..dab800a76 100644 --- a/src/builtins/count.rs +++ b/src/builtins/count.rs @@ -5,7 +5,7 @@ const COUNT_CHUNK_SIZE: usize = 512 * 256; /// Implementation of the builtin count command, used to count the number of arguments sent to it. -pub fn count(_parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Option { +pub fn count(_parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> BuiltinResult { // Always add the size of argv (minus 0, which is "count"). // That means if you call `something | count a b c`, you'll get the count of something _plus 3_. let mut numargs = argv.len() - 1; @@ -23,7 +23,7 @@ pub fn count(_parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> O streams.out.appendln(numargs.to_wstring()); if numargs == 0 { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } - STATUS_CMD_OK + Ok(SUCCESS) } diff --git a/src/builtins/disown.rs b/src/builtins/disown.rs index 507e79a9e..438eb420e 100644 --- a/src/builtins/disown.rs +++ b/src/builtins/disown.rs @@ -1,15 +1,14 @@ // Implementation of the disown builtin. -use super::shared::{builtin_print_help, STATUS_CMD_ERROR, STATUS_INVALID_ARGS}; +use super::prelude::*; use crate::io::IoStreams; use crate::parser::Parser; use crate::proc::{add_disowned_job, Job, Pid}; use crate::{ - builtins::shared::{HelpOnlyCmdOpts, STATUS_CMD_OK}, + builtins::shared::HelpOnlyCmdOpts, wchar::wstr, wutil::{fish_wcstoi, wgettext_fmt}, }; -use libc::c_int; use libc::SIGCONT; /// Helper for builtin_disown. @@ -43,20 +42,16 @@ fn disown_job(cmd: &wstr, streams: &mut IoStreams, j: &Job) { } /// Builtin for removing jobs from the job list. -pub fn disown(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { - let opts = match HelpOnlyCmdOpts::parse(args, parser, streams) { - Ok(opts) => opts, - Err(err @ Some(_)) if err != STATUS_CMD_OK => return err, - Err(err) => panic!("Illogical exit code from parse_options(): {err:?}"), - }; +pub fn disown(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { + let opts = HelpOnlyCmdOpts::parse(args, parser, streams)?; let cmd = args[0]; if opts.print_help { builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } - let mut retval; + let mut retval: BuiltinResult; if opts.optind == args.len() { // Select last constructed job (ie first job in the job queue) that is possible to disown. // Stopped jobs can be disowned (they will be continued). @@ -72,15 +67,15 @@ pub fn disown(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O if let Some(job) = job { disown_job(cmd, streams, &job); - retval = STATUS_CMD_OK; + retval = Ok(SUCCESS); } else { streams .err .append(wgettext_fmt!("%ls: There are no suitable jobs\n", cmd)); - retval = STATUS_CMD_ERROR; + retval = Err(STATUS_CMD_ERROR); } } else { - retval = STATUS_CMD_OK; + retval = Ok(SUCCESS); // If one argument is not a valid pid (i.e. integer >= 0), fail without disowning anything, // but still print errors for all of them. @@ -97,7 +92,7 @@ pub fn disown(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O cmd, arg )); - retval = STATUS_INVALID_ARGS; + retval = Err(STATUS_INVALID_ARGS); None } Some(pid) => parser.job_get_from_pid(pid).or_else(|| { @@ -113,9 +108,7 @@ pub fn disown(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O }) .collect(); - if retval != STATUS_CMD_OK { - return retval; - } + retval?; // One PID/JID may be repeated or multiple PIDs may refer to the same job; // include the job only once. diff --git a/src/builtins/echo.rs b/src/builtins/echo.rs index c8b99101c..837807af7 100644 --- a/src/builtins/echo.rs +++ b/src/builtins/echo.rs @@ -24,8 +24,10 @@ fn parse_options( args: &mut [&wstr], parser: &Parser, streams: &mut IoStreams, -) -> Result<(Options, usize), Option> { - let cmd = args[0]; +) -> Result<(Options, usize), ErrorCode> { + let Some(&cmd) = args.get(0) else { + return Err(STATUS_INVALID_ARGS); + }; const SHORT_OPTS: &wstr = L!("+:Eens"); const LONG_OPTS: &[WOption] = &[]; @@ -135,12 +137,8 @@ fn parse_numeric_sequence(chars: I) -> Option<(usize, u8)> /// /// Bash only respects `-n` if it's the first argument. We'll do the same. We also support a new, /// fish specific, option `-s` to mean "no spaces". -pub fn echo(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { - let (opts, optind) = match parse_options(args, parser, streams) { - Ok((opts, optind)) => (opts, optind), - Err(err @ Some(_)) if err != STATUS_CMD_OK => return err, - Err(err) => panic!("Illogical exit code from parse_options(): {err:?}"), - }; +pub fn echo(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { + let (opts, optind) = parse_options(args, parser, streams)?; // The special character \c can be used to indicate no more output. let mut output_stopped = false; @@ -220,5 +218,5 @@ pub fn echo(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Opt streams.out.append(out); } - STATUS_CMD_OK + Ok(SUCCESS) } diff --git a/src/builtins/emit.rs b/src/builtins/emit.rs index 23ecbc2aa..cad9a678f 100644 --- a/src/builtins/emit.rs +++ b/src/builtins/emit.rs @@ -1,25 +1,23 @@ use super::prelude::*; use crate::event; -pub fn emit(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Option { - let cmd = argv[0]; - - let opts = match HelpOnlyCmdOpts::parse(argv, parser, streams) { - Ok(opts) => opts, - Err(err @ Some(_)) if err != STATUS_CMD_OK => return err, - Err(err) => panic!("Illogical exit code from parse_options(): {err:?}"), +pub fn emit(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> BuiltinResult { + let Some(&cmd) = argv.get(0) else { + return Err(STATUS_INVALID_ARGS); }; + let opts = HelpOnlyCmdOpts::parse(argv, parser, streams)?; + if opts.print_help { builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } let Some(event_name) = argv.get(opts.optind) else { streams .err .append(sprintf!(L!("%ls: expected event name\n"), cmd)); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); }; event::fire_generic( @@ -31,5 +29,5 @@ pub fn emit(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt .collect(), ); - STATUS_CMD_OK + Ok(SUCCESS) } diff --git a/src/builtins/eval.rs b/src/builtins/eval.rs index d2b44d039..33197fccb 100644 --- a/src/builtins/eval.rs +++ b/src/builtins/eval.rs @@ -6,10 +6,10 @@ use crate::wcstringutil::join_strings; use libc::{STDERR_FILENO, STDOUT_FILENO}; -pub fn eval(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { +pub fn eval(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { let argc = args.len(); if argc <= 1 { - return STATUS_CMD_OK; + return Ok(SUCCESS); } let new_cmd = join_strings(&args[1..], ' '); @@ -28,7 +28,7 @@ pub fn eval(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Opt match IoBufferfill::create_opts(parser.libdata().read_limit, STDOUT_FILENO) { Err(_) => { // We were unable to create a pipe, probably fd exhaustion. - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } Ok(buffer) => { stdout_fill = Some(buffer.clone()); @@ -43,7 +43,7 @@ pub fn eval(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Opt match IoBufferfill::create_opts(parser.libdata().read_limit, STDERR_FILENO) { Err(_) => { // We were unable to create a pipe, probably fd exhaustion. - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } Ok(buffer) => { stderr_fill = Some(buffer.clone()); @@ -56,9 +56,9 @@ pub fn eval(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Opt let status = if res.was_empty { // Issue #5692, in particular, to catch `eval ""`, `eval "begin; end;"`, etc. // where we have an argument but nothing is executed. - STATUS_CMD_OK + Ok(SUCCESS) } else { - Some(res.status.status_value()) + BuiltinResult::from_dynamic(res.status.status_value()) }; // Finish the bufferfills - exhaust and close our pipes. diff --git a/src/builtins/exit.rs b/src/builtins/exit.rs index e5dd89de6..91406bf5a 100644 --- a/src/builtins/exit.rs +++ b/src/builtins/exit.rs @@ -1,11 +1,13 @@ +use std::ops::ControlFlow; + use super::prelude::*; use super::r#return::parse_return_value; /// Function for handling the exit builtin. -pub fn exit(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { +pub fn exit(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { let retval = match parse_return_value(args, parser, streams) { - Ok(v) => v, - Err(e) => return e, + ControlFlow::Continue(r) => r, + ControlFlow::Break(result) => return result, }; // Mark that we are exiting in the parser. @@ -14,5 +16,5 @@ pub fn exit(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Opt // behavior we want here. parser.libdata_mut().exit_current_script = true; - return Some(retval); + BuiltinResult::from_dynamic(retval) } diff --git a/src/builtins/fg.rs b/src/builtins/fg.rs index a07746607..7f9c49cf5 100644 --- a/src/builtins/fg.rs +++ b/src/builtins/fg.rs @@ -11,17 +11,16 @@ use super::prelude::*; /// Builtin for putting a job in the foreground. -pub fn fg(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Option { - let opts = match HelpOnlyCmdOpts::parse(argv, parser, streams) { - Ok(opts) => opts, - Err(err @ Some(_)) if err != STATUS_CMD_OK => return err, - Err(err) => panic!("Illogical exit code from parse_options(): {err:?}"), +pub fn fg(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> BuiltinResult { + let opts = HelpOnlyCmdOpts::parse(argv, parser, streams)?; + + let Some(&cmd) = argv.get(0) else { + return Err(STATUS_INVALID_ARGS); }; - let cmd = argv[0]; if opts.print_help { builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } let job; @@ -40,7 +39,7 @@ pub fn fg(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Optio streams .err .append(wgettext_fmt!("%ls: There are no suitable jobs\n", cmd)); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } Some((pos, j)) => { job_pos = Some(pos); @@ -116,7 +115,7 @@ pub fn fg(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Optio }; let Some(job) = job else { - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); }; let job_pos = job_pos.unwrap(); @@ -170,8 +169,8 @@ pub fn fg(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Optio } transfer.reclaim(); if resumed { - STATUS_CMD_OK + Ok(SUCCESS) } else { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) } } diff --git a/src/builtins/fish_indent.rs b/src/builtins/fish_indent.rs index d7fc14081..87e26bee1 100644 --- a/src/builtins/fish_indent.rs +++ b/src/builtins/fish_indent.rs @@ -18,7 +18,6 @@ use crate::ast::{ self, Ast, Category, Leaf, List, Node, NodeVisitor, SourceRangeList, Traversal, Type, }; -use crate::builtins::shared::{STATUS_CMD_ERROR, STATUS_CMD_OK}; use crate::common::{ str2wcstring, unescape_string, wcs2string, UnescapeFlags, UnescapeStringStyle, PROGRAM_NAME, }; @@ -855,15 +854,15 @@ fn throwing_main() -> i32 { } } - do_indent(&mut streams, args) + do_indent(&mut streams, args).builtin_status_code() } -pub fn fish_indent(_parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { +pub fn fish_indent(_parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { let args = args.iter_mut().map(|x| x.to_owned()).collect(); - Some(do_indent(streams, args)) + do_indent(streams, args) } -fn do_indent(streams: &mut IoStreams, args: Vec) -> i32 { +fn do_indent(streams: &mut IoStreams, args: Vec) -> BuiltinResult { // Types of output we support #[derive(Eq, PartialEq)] enum OutputType { @@ -904,7 +903,7 @@ enum OutputType { 'P' => DUMP_PARSE_TREE.store(true), 'h' => { print_help("fish_indent"); - return STATUS_CMD_OK.unwrap(); + return Ok(SUCCESS); } 'v' => { streams.out.appendln(wgettext_fmt!( @@ -912,7 +911,7 @@ enum OutputType { PROGRAM_NAME.get().unwrap(), crate::BUILD_VERSION )); - return STATUS_CMD_OK.unwrap(); + return Ok(SUCCESS); } 'w' => output_type = OutputType::File, 'i' => do_indent = false, @@ -922,7 +921,7 @@ enum OutputType { '\x02' => output_type = OutputType::Ansi, '\x03' => output_type = OutputType::PygmentsCsv, 'c' => output_type = OutputType::Check, - _ => return STATUS_CMD_ERROR.unwrap(), + _ => return Err(STATUS_CMD_ERROR), } } @@ -939,7 +938,7 @@ enum OutputType { "Expected file path to read/write for -w:\n\n $ %ls -w foo.fish", PROGRAM_NAME.get().unwrap() )); - return STATUS_CMD_ERROR.unwrap(); + return Err(STATUS_CMD_ERROR); } use std::os::fd::FromRawFd; let mut fd = unsafe { std::fs::File::from_raw_fd(streams.stdin_fd) }; @@ -949,7 +948,7 @@ enum OutputType { Err(_) => { // Don't close the fd std::mem::forget(fd); - return STATUS_CMD_ERROR.unwrap(); + return Err(STATUS_CMD_ERROR); } } std::mem::forget(fd); @@ -960,7 +959,7 @@ enum OutputType { Ok(file) => { match read_file(file) { Ok(s) => src = s, - Err(()) => return STATUS_CMD_ERROR.unwrap(), + Err(()) => return Err(STATUS_CMD_ERROR), } output_location = arg; } @@ -970,7 +969,7 @@ enum OutputType { arg, err.to_string() )); - return STATUS_CMD_ERROR.unwrap(); + return Err(STATUS_CMD_ERROR); } } } @@ -1051,7 +1050,7 @@ enum OutputType { output_location, err.to_string() )); - return STATUS_CMD_ERROR.unwrap(); + return Err(STATUS_CMD_ERROR); } } } @@ -1078,7 +1077,11 @@ enum OutputType { streams.out.append(str2wcstring(&colored_output)); i += 1; } - retval + if retval == 0 { + Ok(SUCCESS) + } else { + Err(retval) + } } static DUMP_PARSE_TREE: RelaxedAtomicBool = RelaxedAtomicBool::new(false); diff --git a/src/builtins/fish_key_reader.rs b/src/builtins/fish_key_reader.rs index da199d8fe..7d5400109 100644 --- a/src/builtins/fish_key_reader.rs +++ b/src/builtins/fish_key_reader.rs @@ -87,7 +87,7 @@ fn sequence_name(recent_chars: &mut Vec, c: char) -> Option { } /// Process the characters we receive as the user presses keys. -fn process_input(streams: &mut IoStreams, continuous_mode: bool, verbose: bool) -> i32 { +fn process_input(streams: &mut IoStreams, continuous_mode: bool, verbose: bool) -> BuiltinResult { let mut first_char_seen = false; let mut queue = InputEventQueue::new(STDIN_FILENO); let mut recent_chars1 = vec![]; @@ -125,11 +125,15 @@ fn process_input(streams: &mut IoStreams, continuous_mode: bool, verbose: bool) first_char_seen = true; } - 0 + Ok(SUCCESS) } /// Setup our environment (e.g., tty modes), process key strokes, then reset the environment. -fn setup_and_process_keys(streams: &mut IoStreams, continuous_mode: bool, verbose: bool) -> i32 { +fn setup_and_process_keys( + streams: &mut IoStreams, + continuous_mode: bool, + verbose: bool, +) -> BuiltinResult { signal_set_handlers(true); // We need to set the shell-modes for ICRNL, // in fish-proper this is done once a command is run. @@ -164,7 +168,7 @@ fn parse_flags( args: Vec, continuous_mode: &mut bool, verbose: &mut bool, -) -> ControlFlow { +) -> ControlFlow { let short_opts: &wstr = L!("+chvV"); let long_opts: &[WOption] = &[ wopt(L!("continuous"), ArgType::NoArgument, 'c'), @@ -182,7 +186,7 @@ fn parse_flags( } 'h' => { print_help("fish_key_reader"); - return ControlFlow::Break(0); + return ControlFlow::Break(Ok(SUCCESS)); } 'v' => { streams.out.appendln(wgettext_fmt!( @@ -190,7 +194,7 @@ fn parse_flags( PROGRAM_NAME.get().unwrap(), crate::BUILD_VERSION )); - return ControlFlow::Break(0); + return ControlFlow::Break(Ok(SUCCESS)); } 'V' => { *verbose = true; @@ -201,7 +205,7 @@ fn parse_flags( "fish_key_reader", w.argv[w.wopt_index - 1] )); - return ControlFlow::Break(1); + return ControlFlow::Break(Err(STATUS_CMD_ERROR)); } _ => panic!(), } @@ -212,7 +216,7 @@ fn parse_flags( streams .err .appendln(wgettext_fmt!("Expected no arguments, got %d", argc)); - return ControlFlow::Break(1); + return ControlFlow::Break(Err(STATUS_CMD_ERROR)); } ControlFlow::Continue(()) @@ -222,21 +226,21 @@ pub fn fish_key_reader( _parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr], -) -> Option { +) -> BuiltinResult { let mut continuous_mode = false; let mut verbose = false; let args = args.iter_mut().map(|x| x.to_owned()).collect(); - if let ControlFlow::Break(i) = parse_flags(streams, args, &mut continuous_mode, &mut verbose) { - return Some(i); + if let ControlFlow::Break(s) = parse_flags(streams, args, &mut continuous_mode, &mut verbose) { + return s; } if streams.stdin_fd < 0 || unsafe { libc::isatty(streams.stdin_fd) } == 0 { streams.err.appendln("Stdin must be attached to a tty."); - return Some(1); + return Err(STATUS_CMD_ERROR); } - Some(setup_and_process_keys(streams, continuous_mode, verbose)) + setup_and_process_keys(streams, continuous_mode, verbose) } pub fn main() { @@ -267,10 +271,10 @@ fn throwing_main() -> i32 { let args: Vec = std::env::args_os() .map(|osstr| str2wcstring(osstr.as_bytes())) .collect(); - if let ControlFlow::Break(i) = + if let ControlFlow::Break(s) = parse_flags(&mut streams, args, &mut continuous_mode, &mut verbose) { - return i; + return s.builtin_status_code(); } if !isatty(libc::STDIN_FILENO) { @@ -280,5 +284,5 @@ fn throwing_main() -> i32 { return 1; } - setup_and_process_keys(&mut streams, continuous_mode, verbose) + setup_and_process_keys(&mut streams, continuous_mode, verbose).builtin_status_code() } diff --git a/src/builtins/function.rs b/src/builtins/function.rs index 4bf4a81b2..c1e52cdab 100644 --- a/src/builtins/function.rs +++ b/src/builtins/function.rs @@ -77,7 +77,7 @@ fn parse_cmd_opts( argv: &mut [&wstr], parser: &Parser, streams: &mut IoStreams, -) -> Option { +) -> c_int { let cmd = L!("function"); let print_hints = false; let mut handling_named_arguments = false; @@ -231,7 +231,7 @@ fn validate_function_name( function_name: &mut WString, cmd: &wstr, streams: &mut IoStreams, -) -> Option { +) -> c_int { if argv.len() < 2 { // This is currently impossible but let's be paranoid. streams @@ -267,7 +267,7 @@ pub fn function( streams: &mut IoStreams, c_args: &mut [&wstr], func_node: NodeRef, -) -> Option { +) -> c_int { // The wgetopt function expects 'function' as the first argument. Make a new vec with // that property. This is needed because this builtin has a different signature than the other // builtins. diff --git a/src/builtins/functions.rs b/src/builtins/functions.rs index c6b5b3854..4c08359ff 100644 --- a/src/builtins/functions.rs +++ b/src/builtins/functions.rs @@ -74,7 +74,7 @@ fn parse_cmd_opts<'args>( argv: &mut [&'args wstr], parser: &Parser, streams: &mut IoStreams, -) -> Option { +) -> BuiltinResult { let cmd = L!("functions"); let print_hints = false; let mut w = WGetopter::new(SHORT_OPTIONS, LONG_OPTIONS, argv); @@ -99,11 +99,11 @@ fn parse_cmd_opts<'args>( } ':' => { builtin_missing_argument(parser, streams, cmd, argv[w.wopt_index - 1], print_hints); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } '?' => { builtin_unknown_option(parser, streams, cmd, argv[w.wopt_index - 1], print_hints); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } other => { panic!("Unexpected retval from WGetopter: {}", other); @@ -112,24 +112,25 @@ fn parse_cmd_opts<'args>( } *optind = w.wopt_index; - STATUS_CMD_OK + Ok(SUCCESS) } -pub fn functions(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { - let cmd = args[0]; +pub fn functions(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { + let Some(&cmd) = args.get(0) else { + return Err(STATUS_INVALID_ARGS); + }; let mut opts = FunctionsCmdOpts::default(); let mut optind = 0; - let retval = parse_cmd_opts(&mut opts, &mut optind, args, parser, streams); - if retval != STATUS_CMD_OK { - return retval; - } + + parse_cmd_opts(&mut opts, &mut optind, args, parser, streams)?; + // Shadow our args with the positionals let args = &args[optind..]; if opts.print_help { builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } let describe = opts.description.is_some(); @@ -141,13 +142,13 @@ pub fn functions(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) - { streams.err.append(wgettext_fmt!(BUILTIN_ERR_COMBO, cmd)); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if opts.report_metadata && opts.no_metadata { streams.err.append(wgettext_fmt!(BUILTIN_ERR_COMBO, cmd)); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if opts.erase { @@ -155,7 +156,7 @@ pub fn functions(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) - function::remove(arg); } // Historical - this never failed? - return STATUS_CMD_OK; + return Ok(SUCCESS); } if let Some(desc) = opts.description { @@ -165,7 +166,7 @@ pub fn functions(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) - cmd )); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } let current_func = args[0]; @@ -176,11 +177,11 @@ pub fn functions(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) - current_func )); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } function::set_desc(current_func, desc.into(), parser); - return STATUS_CMD_OK; + return Ok(SUCCESS); } if opts.report_metadata { @@ -197,7 +198,7 @@ pub fn functions(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) - 1, args.len() )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } let props = function::get_props_autoload(args[0], parser); let def_file = if let Some(p) = props.as_ref() { @@ -252,7 +253,7 @@ pub fn functions(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) - streams.out.appendln(desc); } // Historical - this never failed? - return STATUS_CMD_OK; + return Ok(SUCCESS); } if opts.handlers { @@ -264,14 +265,14 @@ pub fn functions(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) - "%ls: Expected generic | variable | signal | exit | job-id for --handlers-type\n", cmd )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } event::print(streams, opts.handlers_type.unwrap_or(L!(""))); - return STATUS_CMD_OK; + return Ok(SUCCESS); } if opts.query && args.is_empty() { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } if opts.list || args.is_empty() { @@ -295,7 +296,7 @@ pub fn functions(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) - streams.out.appendln(name); } } - return STATUS_CMD_OK; + return Ok(SUCCESS); } if opts.copy { @@ -305,7 +306,7 @@ pub fn functions(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) - cmd )); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } let current_func = args[0]; let new_func = args[1]; @@ -317,7 +318,7 @@ pub fn functions(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) - current_func )); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } if !valid_func_name(new_func) || parser_keywords_is_reserved(new_func) { @@ -327,7 +328,7 @@ pub fn functions(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) - new_func )); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if function::exists(new_func, parser) { @@ -338,15 +339,15 @@ pub fn functions(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) - current_func )); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } if function::copy(current_func, new_func.into(), parser) { - return STATUS_CMD_OK; + return Ok(SUCCESS); } - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } - let mut res: c_int = STATUS_CMD_OK.unwrap(); + let mut res: c_int = 0; let mut first = true; for arg in args.iter() { @@ -425,5 +426,5 @@ pub fn functions(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) - first = false; } - return Some(res); + BuiltinResult::from_dynamic(res) } diff --git a/src/builtins/history.rs b/src/builtins/history.rs index c86e0ac4e..8c0834280 100644 --- a/src/builtins/history.rs +++ b/src/builtins/history.rs @@ -139,34 +139,37 @@ fn parse_cmd_opts( argv: &mut [&wstr], parser: &Parser, streams: &mut IoStreams, -) -> Option { - let cmd = argv[0]; +) -> BuiltinResult { + let Some(&cmd) = argv.get(0) else { + return Err(STATUS_INVALID_ARGS); + }; + let mut w = WGetopter::new(short_options, longopts, argv); while let Some(opt) = w.next_opt() { match opt { '\x01' => { if !set_hist_cmd(cmd, &mut opts.hist_cmd, HistCmd::Delete, streams) { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } '\x02' => { if !set_hist_cmd(cmd, &mut opts.hist_cmd, HistCmd::Search, streams) { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } '\x03' => { if !set_hist_cmd(cmd, &mut opts.hist_cmd, HistCmd::Save, streams) { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } '\x04' => { if !set_hist_cmd(cmd, &mut opts.hist_cmd, HistCmd::Clear, streams) { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } '\x05' => { if !set_hist_cmd(cmd, &mut opts.hist_cmd, HistCmd::Merge, streams) { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } 'C' => { @@ -195,7 +198,7 @@ fn parse_cmd_opts( cmd, w.woptarg.unwrap() )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } }, 'z' => { @@ -206,7 +209,7 @@ fn parse_cmd_opts( } ':' => { builtin_missing_argument(parser, streams, cmd, argv[w.wopt_index - 1], true); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } '?' => { // Try to parse it as a number; e.g., "-123". @@ -214,7 +217,7 @@ fn parse_cmd_opts( Ok(x) => opts.max_items = Some(x as _), // todo!("historical behavior is to cast") Err(_) => { builtin_unknown_option(parser, streams, cmd, argv[w.wopt_index - 1], true); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } } w.remaining_text = L!(""); @@ -226,22 +229,23 @@ fn parse_cmd_opts( } *optind = w.wopt_index; - STATUS_CMD_OK + Ok(SUCCESS) } /// Manipulate history of interactive commands executed by the user. -pub fn history(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { +pub fn history(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { let mut opts = HistoryCmdOpts::default(); let mut optind = 0; - let retval = parse_cmd_opts(&mut opts, &mut optind, args, parser, streams); - if retval != STATUS_CMD_OK { - return retval; - } - let cmd = &args[0]; + + parse_cmd_opts(&mut opts, &mut optind, args, parser, streams)?; + + let Some(&cmd) = args.get(0) else { + return Err(STATUS_INVALID_ARGS); + }; if opts.print_help { builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } // Use the default history if we have none (which happens if invoked non-interactively, e.g. @@ -256,7 +260,7 @@ pub fn history(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> if optind < args.len() { if let Ok(subcmd) = HistCmd::try_from(args[optind]) { if !set_hist_cmd(cmd, &mut opts.hist_cmd, subcmd, streams) { - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } optind += 1; } @@ -266,7 +270,7 @@ pub fn history(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> // search term). let args = &args[optind..]; - let mut status = STATUS_CMD_OK; + let mut status = Ok(SUCCESS); match opts.hist_cmd { HistCmd::None | HistCmd::Search => { if !history.search( @@ -281,7 +285,7 @@ pub fn history(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> &parser.context().cancel_checker, streams, ) { - status = STATUS_CMD_ERROR; + status = Err(STATUS_CMD_ERROR); } } HistCmd::Delete => { @@ -293,13 +297,13 @@ pub fn history(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> streams .err .append(wgettext!("builtin history delete only supports --exact\n")); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if !opts.case_sensitive { streams.err.append(wgettext!( "builtin history delete --exact requires --case-sensitive\n" )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } for delete_string in args { @@ -308,21 +312,21 @@ pub fn history(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> } HistCmd::Clear => { if check_for_unexpected_hist_args(&opts, cmd, args, streams) { - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } history.clear(); history.save(); } HistCmd::ClearSession => { if check_for_unexpected_hist_args(&opts, cmd, args, streams) { - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } history.clear_session(); history.save(); } HistCmd::Merge => { if check_for_unexpected_hist_args(&opts, cmd, args, streams) { - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if in_private_mode(parser.vars()) { @@ -330,13 +334,13 @@ pub fn history(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> "%ls: can't merge history in private mode\n", cmd )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } history.incorporate_external_changes(); } HistCmd::Save => { if check_for_unexpected_hist_args(&opts, cmd, args, streams) { - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } history.save(); } diff --git a/src/builtins/jobs.rs b/src/builtins/jobs.rs index 70a2ccdbb..3088e06e6 100644 --- a/src/builtins/jobs.rs +++ b/src/builtins/jobs.rs @@ -1,9 +1,6 @@ // Functions for executing the jobs builtin. -use super::shared::{ - builtin_missing_argument, builtin_print_help, builtin_unknown_option, STATUS_CMD_ERROR, - STATUS_INVALID_ARGS, -}; +use super::prelude::*; use crate::common::{escape_string, timef, EscapeFlags, EscapeStringStyle}; use crate::io::IoStreams; use crate::job_group::{JobId, MaybeJobId}; @@ -13,11 +10,9 @@ use crate::wgetopt::{wopt, ArgType, WGetopter, WOption}; use crate::wutil::wgettext; use crate::{ - builtins::shared::STATUS_CMD_OK, wchar::{wstr, WString, L}, wutil::{fish_wcstoi, sprintf, wgettext_fmt}, }; -use libc::c_int; use std::num::NonZeroU32; /// Print modes for the jobs builtin. @@ -134,8 +129,12 @@ fn builtin_jobs_print(j: &Job, mode: JobsPrintMode, header: bool, streams: &mut ]; /// The jobs builtin. Used for printing running jobs. Defined in builtin_jobs.c. -pub fn jobs(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Option { - let cmd = argv[0]; +pub fn jobs(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> BuiltinResult { + let cmd = match argv.get(0) { + Some(cmd) => *cmd, + None => return Err(STATUS_INVALID_ARGS), + }; + let argc = argv.len(); let mut found = false; let mut mode = JobsPrintMode::Default; @@ -161,15 +160,15 @@ pub fn jobs(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt } 'h' => { builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } ':' => { builtin_missing_argument(parser, streams, cmd, argv[w.wopt_index - 1], true); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } '?' => { builtin_unknown_option(parser, streams, cmd, argv[w.wopt_index - 1], true); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } _ => panic!("unexpected retval from WGetopter"), } @@ -180,10 +179,10 @@ pub fn jobs(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt for j in &parser.jobs()[..] { if j.is_visible() { builtin_jobs_print(j, mode, !streams.out_is_redirected, streams); - return STATUS_CMD_OK; + return Ok(SUCCESS); } } - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } if w.wopt_index < argc { @@ -197,7 +196,7 @@ pub fn jobs(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt cmd, arg )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } Some(job_id) => { let job_id = if job_id == 0 { @@ -218,7 +217,7 @@ pub fn jobs(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt cmd, arg )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } Some(job_id) => { j = parser.job_get_from_pid(job_id); @@ -235,7 +234,7 @@ pub fn jobs(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt .err .append(wgettext_fmt!("%ls: No suitable job: %ls\n", cmd, arg)); } - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } } else { @@ -255,8 +254,8 @@ pub fn jobs(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt .out .append(wgettext_fmt!("%ls: There are no jobs\n", argv[0])); } - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } - STATUS_CMD_OK + Ok(SUCCESS) } diff --git a/src/builtins/math.rs b/src/builtins/math.rs index 5bbc103e5..630b530ae 100644 --- a/src/builtins/math.rs +++ b/src/builtins/math.rs @@ -32,7 +32,7 @@ fn parse_cmd_opts( args: &mut [&wstr], parser: &Parser, streams: &mut IoStreams, -) -> Result<(Options, usize), Option> { +) -> Result<(Options, usize), ErrorCode> { const cmd: &wstr = L!("math"); let print_hints = true; @@ -210,7 +210,7 @@ fn evaluate_expression( streams: &mut IoStreams, opts: &Options, expression: &wstr, -) -> Option { +) -> BuiltinResult { let ret = te_interp(expression); match ret { @@ -230,7 +230,7 @@ fn evaluate_expression( s.push('\n'); streams.out.append(s); - return STATUS_CMD_OK; + return Ok(SUCCESS); }; streams @@ -238,7 +238,7 @@ fn evaluate_expression( .append(sprintf!("%ls: Error: %ls\n", cmd, error_message)); streams.err.append(sprintf!("'%ls'\n", expression)); - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) } Err(err) => { streams.err.append(sprintf!( @@ -255,7 +255,7 @@ fn evaluate_expression( streams.err.append(sprintf!("%ls^\n", padding)); } - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) } } } @@ -264,17 +264,14 @@ fn evaluate_expression( const MATH_CHUNK_SIZE: usize = 1024; /// The math builtin evaluates math expressions. -pub fn math(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Option { +pub fn math(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> BuiltinResult { let cmd = argv[0]; - let (opts, mut optind) = match parse_cmd_opts(argv, parser, streams) { - Ok(x) => x, - Err(e) => return e, - }; + let (opts, mut optind) = parse_cmd_opts(argv, parser, streams)?; if opts.print_help { builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } let mut expression = WString::new(); @@ -289,7 +286,7 @@ pub fn math(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt streams .err .append(wgettext_fmt!(BUILTIN_ERR_MIN_ARG_COUNT1, cmd, 1, 0)); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } evaluate_expression(cmd, streams, &opts, &expression) diff --git a/src/builtins/path.rs b/src/builtins/path.rs index d50cd0948..a4b3fd13b 100644 --- a/src/builtins/path.rs +++ b/src/builtins/path.rs @@ -229,7 +229,7 @@ fn parse_opts<'args>( args: &mut [&'args wstr], parser: &Parser, streams: &mut IoStreams, -) -> Option { +) -> BuiltinResult { let cmd = args[0]; let mut args_read = Vec::with_capacity(args.len()); args_read.extend_from_slice(args); @@ -242,11 +242,11 @@ fn parse_opts<'args>( ':' => { streams.err.append(L!("path ")); // clone of string_error builtin_missing_argument(parser, streams, cmd, args_read[w.wopt_index - 1], false); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } '?' => { path_unknown_option(parser, streams, cmd, args_read[w.wopt_index - 1]); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } 'q' => { opts.quiet = true; @@ -270,7 +270,7 @@ fn parse_opts<'args>( for t in types_args { let Ok(r#type) = t.try_into() else { path_error!(streams, "%ls: Invalid type '%ls'\n", "path", t); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); }; *types |= r#type; } @@ -282,7 +282,7 @@ fn parse_opts<'args>( for p in perms_args { let Ok(perm) = p.try_into() else { path_error!(streams, "%ls: Invalid permission '%ls'\n", "path", p); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); }; *perms |= perm; } @@ -341,7 +341,7 @@ fn parse_opts<'args>( } _ => { path_unknown_option(parser, streams, cmd, args_read[w.wopt_index - 1]); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } } } @@ -357,17 +357,17 @@ fn parse_opts<'args>( if opts.arg1.is_none() && n_req_args == 1 { path_error!(streams, BUILTIN_ERR_ARG_COUNT0, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } } // At this point we should not have optional args and be reading args from stdin. if streams.stdin_is_directly_redirected && args.len() > *optind { path_error!(streams, BUILTIN_ERR_TOO_MANY_ARGUMENTS, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } - STATUS_CMD_OK + Ok(SUCCESS) } fn path_transform( @@ -376,14 +376,12 @@ fn path_transform( args: &mut [&wstr], func: impl Fn(&wstr) -> WString, custom_opts: impl Fn(&mut Options), -) -> Option { +) -> BuiltinResult { let mut opts = Options::default(); custom_opts(&mut opts); let mut optind = 0; - let retval = parse_opts(&mut opts, &mut optind, 0, args, parser, streams); - if retval != STATUS_CMD_OK { - return retval; - } + + parse_opts(&mut opts, &mut optind, 0, args, parser, streams)?; let mut n_transformed = 0; let arguments = arguments(args, &mut optind, streams).with_split_behavior(match opts.null_in { @@ -406,20 +404,20 @@ fn path_transform( // Return okay if path wasn't already in this form // TODO: Is that correct? if opts.quiet { - return STATUS_CMD_OK; + return Ok(SUCCESS); }; } path_out(streams, &opts, transformed); } if n_transformed > 0 { - STATUS_CMD_OK + Ok(SUCCESS) } else { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) } } -fn path_basename(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { +fn path_basename(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { path_transform( parser, streams, @@ -431,7 +429,7 @@ fn path_basename(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) - ) } -fn path_dirname(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { +fn path_dirname(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { path_transform(parser, streams, args, |s| wdirname(s).to_owned(), |_| {}) } @@ -443,18 +441,16 @@ fn normalize_help(path: &wstr) -> WString { np } -fn path_normalize(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { +fn path_normalize(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { path_transform(parser, streams, args, normalize_help, |_| {}) } -fn path_mtime(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { +fn path_mtime(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { let mut opts = Options::default(); opts.relative_valid = true; let mut optind = 0; - let retval = parse_opts(&mut opts, &mut optind, 0, args, parser, streams); - if retval != STATUS_CMD_OK { - return retval; - } + + parse_opts(&mut opts, &mut optind, 0, args, parser, streams)?; let mut n_transformed = 0; @@ -472,7 +468,7 @@ fn path_mtime(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O if ret != INVALID_FILE_ID { if opts.quiet { - return STATUS_CMD_OK; + return Ok(SUCCESS); } n_transformed += 1; if !opts.relative { @@ -487,9 +483,9 @@ fn path_mtime(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O } if n_transformed > 0 { - STATUS_CMD_OK + Ok(SUCCESS) } else { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) } } @@ -531,13 +527,11 @@ fn test_find_extension() { } } -fn path_extension(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { +fn path_extension(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { let mut opts = Options::default(); let mut optind = 0; - let retval = parse_opts(&mut opts, &mut optind, 0, args, parser, streams); - if retval != STATUS_CMD_OK { - return retval; - } + + parse_opts(&mut opts, &mut optind, 0, args, parser, streams)?; let mut n_transformed = 0; let arguments = arguments(args, &mut optind, streams).with_split_behavior(match opts.null_in { @@ -555,16 +549,16 @@ fn path_extension(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) let ext = arg.slice_from(pos); if opts.quiet && !ext.is_empty() { - return STATUS_CMD_OK; + return Ok(SUCCESS); } path_out(streams, &opts, ext); n_transformed += 1; } if n_transformed > 0 { - STATUS_CMD_OK + Ok(SUCCESS) } else { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) } } @@ -572,13 +566,11 @@ fn path_change_extension( parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr], -) -> Option { +) -> BuiltinResult { let mut opts = Options::default(); let mut optind = 0; - let retval = parse_opts(&mut opts, &mut optind, 1, args, parser, streams); - if retval != STATUS_CMD_OK { - return retval; - } + + parse_opts(&mut opts, &mut optind, 1, args, parser, streams)?; let mut n_transformed = 0usize; let arguments = arguments(args, &mut optind, streams).with_split_behavior(match opts.null_in { @@ -611,19 +603,17 @@ fn path_change_extension( } if n_transformed > 0 { - STATUS_CMD_OK + Ok(SUCCESS) } else { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) } } -fn path_resolve(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { +fn path_resolve(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { let mut opts = Options::default(); let mut optind = 0; - let retval = parse_opts(&mut opts, &mut optind, 0, args, parser, streams); - if retval != STATUS_CMD_OK { - return retval; - } + + parse_opts(&mut opts, &mut optind, 0, args, parser, streams)?; let mut n_transformed = 0usize; let arguments = arguments(args, &mut optind, streams).with_split_behavior(match opts.null_in { @@ -669,28 +659,26 @@ fn path_resolve(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> // Return 0 if we found a realpath. if opts.quiet { - return STATUS_CMD_OK; + return Ok(SUCCESS); } path_out(streams, &opts, real); n_transformed += 1; } if n_transformed > 0 { - STATUS_CMD_OK + Ok(SUCCESS) } else { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) } } -fn path_sort(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { +fn path_sort(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { let mut opts = Options::default(); opts.reverse_valid = true; opts.unique_valid = true; let mut optind = 0; - let retval = parse_opts(&mut opts, &mut optind, 0, args, parser, streams); - if retval != STATUS_CMD_OK { - return retval; - } + + parse_opts(&mut opts, &mut optind, 0, args, parser, streams)?; let keyfunc: fn(&wstr) -> &wstr = match &opts.key { Some(k) if k == "basename" => wbasename, @@ -703,7 +691,7 @@ fn path_sort(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Op None => wbasename, Some(k) => { path_error!(streams, "%ls: Invalid sort key '%ls'\n", args[0], k); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } }; @@ -744,7 +732,7 @@ fn path_sort(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Op } /* TODO: Return true only if already sorted? */ - STATUS_CMD_OK + Ok(SUCCESS) } fn filter_path(opts: &Options, path: &wstr, uid: Option, gid: Option) -> bool { @@ -853,16 +841,14 @@ fn path_filter_maybe_is( streams: &mut IoStreams, args: &mut [&wstr], is_is: bool, -) -> Option { +) -> BuiltinResult { let mut opts = Options::default(); opts.types_valid = true; opts.perms_valid = true; opts.invert_valid = true; let mut optind = 0; - let retval = parse_opts(&mut opts, &mut optind, 0, args, parser, streams); - if retval != STATUS_CMD_OK { - return retval; - } + + parse_opts(&mut opts, &mut optind, 0, args, parser, streams)?; // If we have been invoked as "path is", which is "path filter -q". if is_is { @@ -911,28 +897,30 @@ fn path_filter_maybe_is( } n_transformed += 1; if opts.quiet { - return STATUS_CMD_OK; + return Ok(SUCCESS); }; } if n_transformed > 0 { - STATUS_CMD_OK + Ok(SUCCESS) } else { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) } } -fn path_filter(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { +fn path_filter(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { path_filter_maybe_is(parser, streams, args, false) } -fn path_is(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { +fn path_is(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { path_filter_maybe_is(parser, streams, args, true) } /// The path builtin, for handling paths. -pub fn path(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { - let cmd = args[0]; +pub fn path(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { + let Some(&cmd) = args.get(0) else { + return Err(STATUS_INVALID_ARGS); + }; let argc = args.len(); if argc <= 1 { @@ -940,12 +928,12 @@ pub fn path(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Opt .err .append(wgettext_fmt!(BUILTIN_ERR_MISSING_SUBCMD, cmd)); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if args[1] == "-h" || args[1] == "--help" { builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } let subcmd_name = args[1]; @@ -966,14 +954,14 @@ pub fn path(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Opt .err .append(wgettext_fmt!(BUILTIN_ERR_INVALID_SUBCMD, cmd, subcmd_name)); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } }; if argc >= 3 && (args[2] == "-h" || args[2] == "--help") { // Unlike string, we don't have separate docs (yet) builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } let args = &mut args[1..]; return subcmd(parser, streams, args); diff --git a/src/builtins/printf.rs b/src/builtins/printf.rs index d3be1f710..1bb1ae798 100644 --- a/src/builtins/printf.rs +++ b/src/builtins/printf.rs @@ -79,7 +79,7 @@ struct builtin_printf_state_t<'a, 'b> { streams: &'a mut IoStreams<'b>, // The status of the operation. - exit_code: c_int, + exit_code: BuiltinResult, // Whether we should stop outputting. This gets set in the case of an error, and also with the // \c escape. @@ -588,7 +588,7 @@ fn nonfatal_error>(&mut self, errstr: Str) { // We set the exit code to error, because one occurred, // but we don't do an early exit so we still print what we can. - self.exit_code = STATUS_CMD_ERROR.unwrap(); + self.exit_code = Err(STATUS_CMD_ERROR); } fn fatal_error>(&mut self, errstr: Str) { @@ -610,7 +610,7 @@ fn fatal_error>(&mut self, errstr: Str) { self.streams.err.push('\n'); } - self.exit_code = STATUS_CMD_ERROR.unwrap(); + self.exit_code = Err(STATUS_CMD_ERROR); self.early_exit = true; } @@ -767,19 +767,19 @@ fn append_output(&mut self, c: char) { } /// The printf builtin. -pub fn printf(_parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Option { +pub fn printf(_parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> BuiltinResult { let mut argc = argv.len(); // Rebind argv as immutable slice (can't rearrange its elements), skipping the command name. let mut argv: &[&wstr] = &argv[1..]; argc -= 1; if argc < 1 { - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } let mut state = builtin_printf_state_t { streams, - exit_code: STATUS_CMD_OK.unwrap(), + exit_code: Ok(SUCCESS), early_exit: false, buff: WString::new(), locale: get_numeric_locale(), @@ -799,5 +799,5 @@ pub fn printf(_parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> break; } } - return Some(state.exit_code); + return state.exit_code; } diff --git a/src/builtins/pwd.rs b/src/builtins/pwd.rs index 130d9da9e..8643bbe06 100644 --- a/src/builtins/pwd.rs +++ b/src/builtins/pwd.rs @@ -12,7 +12,7 @@ wopt(L!("physical"), NoArgument, 'P'), ]; -pub fn pwd(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Option { +pub fn pwd(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> BuiltinResult { let cmd = argv[0]; let argc = argv.len(); let mut resolve_symlinks = false; @@ -23,11 +23,11 @@ pub fn pwd(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opti 'P' => resolve_symlinks = true, 'h' => { builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } '?' => { builtin_unknown_option(parser, streams, cmd, argv[w.wopt_index - 1], false); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } _ => panic!("unexpected retval from WGetopter"), } @@ -37,7 +37,7 @@ pub fn pwd(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opti streams .err .append(wgettext_fmt!(BUILTIN_ERR_ARG_COUNT1, cmd, 0, argc - 1)); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } let mut pwd = WString::new(); @@ -53,12 +53,12 @@ pub fn pwd(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opti cmd, errno().to_string() )); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } if pwd.is_empty() { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } streams.out.appendln(pwd); - return STATUS_CMD_OK; + Ok(SUCCESS) } diff --git a/src/builtins/random.rs b/src/builtins/random.rs index 5d5c70bdd..190d39d6b 100644 --- a/src/builtins/random.rs +++ b/src/builtins/random.rs @@ -9,7 +9,7 @@ static RNG: Lazy> = Lazy::new(|| Mutex::new(get_rng())); -pub fn random(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Option { +pub fn random(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> BuiltinResult { let cmd = argv[0]; let argc = argv.len(); let print_hints = false; @@ -23,15 +23,15 @@ pub fn random(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> O match c { 'h' => { builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } ':' => { builtin_missing_argument(parser, streams, cmd, argv[w.wopt_index - 1], print_hints); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } '?' => { builtin_unknown_option(parser, streams, cmd, argv[w.wopt_index - 1], print_hints); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } _ => { panic!("unexpected retval from WGetopter"); @@ -49,12 +49,12 @@ pub fn random(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> O streams .err .append(wgettext_fmt!("%ls: nothing to choose from\n", cmd)); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } let rand = RNG.lock().unwrap().gen_range(0..arg_count - 1); streams.out.appendln(argv[i + 1 + rand]); - return STATUS_CMD_OK; + return Ok(SUCCESS); } fn parse_ll(streams: &mut IoStreams, cmd: &wstr, num: &wstr) -> Result { let res = fish_wcstol(num); @@ -83,47 +83,47 @@ fn parse_ull(streams: &mut IoStreams, cmd: &wstr, num: &wstr) -> Result return STATUS_INVALID_ARGS, + Err(_) => return Err(STATUS_INVALID_ARGS), Ok(x) => { let mut engine = RNG.lock().unwrap(); *engine = SmallRng::seed_from_u64(x as u64); } } - return STATUS_CMD_OK; + return Ok(SUCCESS); } 2 => { // start is first, end is second match parse_ll(streams, cmd, argv[i]) { - Err(_) => return STATUS_INVALID_ARGS, + Err(_) => return Err(STATUS_INVALID_ARGS), Ok(x) => start = x, } match parse_ll(streams, cmd, argv[i + 1]) { - Err(_) => return STATUS_INVALID_ARGS, + Err(_) => return Err(STATUS_INVALID_ARGS), Ok(x) => end = x, } } 3 => { // start, step, end match parse_ll(streams, cmd, argv[i]) { - Err(_) => return STATUS_INVALID_ARGS, + Err(_) => return Err(STATUS_INVALID_ARGS), Ok(x) => start = x, } // start, step, end match parse_ull(streams, cmd, argv[i + 1]) { - Err(_) => return STATUS_INVALID_ARGS, + Err(_) => return Err(STATUS_INVALID_ARGS), Ok(0) => { streams .err .append(wgettext_fmt!("%ls: STEP must be a positive integer\n", cmd,)); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } Ok(x) => step = x, } match parse_ll(streams, cmd, argv[i + 2]) { - Err(_) => return STATUS_INVALID_ARGS, + Err(_) => return Err(STATUS_INVALID_ARGS), Ok(x) => end = x, } } @@ -131,7 +131,7 @@ fn parse_ull(streams: &mut IoStreams, cmd: &wstr, num: &wstr) -> Result Result Result<(Options, usize), Option> { +) -> Result<(Options, usize), ErrorCode> { let cmd = args[0]; let mut opts = Options::new(); let mut w = WGetopter::new(SHORT_OPTIONS, LONG_OPTIONS, args); @@ -211,8 +211,8 @@ fn read_interactive( right_prompt: &wstr, commandline: &wstr, inputfd: RawFd, -) -> Option { - let mut exit_res = STATUS_CMD_OK; +) -> BuiltinResult { + let mut exit_res = Ok(SUCCESS); // Construct a configuration. let mut conf = ReaderConfig::default(); @@ -256,7 +256,7 @@ fn read_interactive( buff.truncate(nchars); } } else { - exit_res = STATUS_CMD_ERROR; + exit_res = Err(STATUS_CMD_ERROR); } reader_pop(); exit_res @@ -272,8 +272,8 @@ fn read_interactive( /// of chars. /// /// Returns an exit status. -fn read_in_chunks(fd: RawFd, buff: &mut WString, split_null: bool, do_seek: bool) -> Option { - let mut exit_res = STATUS_CMD_OK; +fn read_in_chunks(fd: RawFd, buff: &mut WString, split_null: bool, do_seek: bool) -> BuiltinResult { + let mut exit_res = Ok(SUCCESS); let mut narrow_buff = vec![]; let mut eof = false; let mut finished = false; @@ -311,18 +311,18 @@ fn read_in_chunks(fd: RawFd, buff: &mut WString, split_null: bool, do_seek: bool } == -1 { perror("lseek"); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } finished = true; } else if narrow_buff.len() > READ_BYTE_LIMIT.load(Ordering::Relaxed) { - exit_res = STATUS_READ_TOO_MUCH; + exit_res = Err(STATUS_READ_TOO_MUCH); finished = true; } } *buff = str2wcstring(&narrow_buff); if buff.is_empty() && eof { - exit_res = STATUS_CMD_ERROR; + exit_res = Err(STATUS_CMD_ERROR); } exit_res @@ -336,8 +336,8 @@ fn read_one_char_at_a_time( buff: &mut WString, nchars: usize, split_null: bool, -) -> Option { - let mut exit_res = STATUS_CMD_OK; +) -> BuiltinResult { + let mut exit_res = Ok(SUCCESS); let mut eof = false; let mut nbytes = 0; @@ -379,7 +379,7 @@ fn read_one_char_at_a_time( } if nbytes > READ_BYTE_LIMIT.load(Ordering::Relaxed) { - exit_res = STATUS_READ_TOO_MUCH; + exit_res = Err(STATUS_READ_TOO_MUCH); break; } if eof { @@ -399,7 +399,7 @@ fn read_one_char_at_a_time( } if buff.is_empty() && eof { - exit_res = STATUS_CMD_ERROR; + exit_res = Err(STATUS_CMD_ERROR); } exit_res @@ -412,7 +412,7 @@ fn validate_read_args( argv: &[&wstr], parser: &Parser, streams: &mut IoStreams, -) -> Option { +) -> BuiltinResult { if opts.prompt.is_some() && opts.prompt_str.is_some() { streams.err.append(wgettext_fmt!( "%ls: Options %ls and %ls cannot be used together\n", @@ -421,7 +421,7 @@ fn validate_read_args( "-P", )); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if opts.delimiter.is_some() && opts.one_line { @@ -431,7 +431,7 @@ fn validate_read_args( "--delimiter", "--line" )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if opts.one_line && opts.split_null { streams.err.append(wgettext_fmt!( @@ -440,7 +440,7 @@ fn validate_read_args( "-z", "--line" )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if let Some(prompt_str) = opts.prompt_str.as_ref() { @@ -452,7 +452,7 @@ fn validate_read_args( if opts.place.contains(EnvMode::UNEXPORT) && opts.place.contains(EnvMode::EXPORT) { streams.err.append(wgettext_fmt!(BUILTIN_ERR_EXPUNEXP, cmd)); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if opts @@ -464,7 +464,7 @@ fn validate_read_args( { streams.err.append(wgettext_fmt!(BUILTIN_ERR_GLOCAL, cmd)); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } let argc = argv.len(); @@ -472,21 +472,21 @@ fn validate_read_args( streams .err .append(wgettext_fmt!(BUILTIN_ERR_MIN_ARG_COUNT1, cmd, 1, argc)); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if opts.array && argc != 1 { streams .err .append(wgettext_fmt!(BUILTIN_ERR_ARG_COUNT1, cmd, 1, argc)); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if opts.to_stdout && argc > 0 { streams .err .append(wgettext_fmt!(BUILTIN_ERR_MAX_ARG_COUNT1, cmd, 0, argc)); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if opts.tokenize && opts.delimiter.is_some() { @@ -496,7 +496,7 @@ fn validate_read_args( "--delimiter", "--tokenize" )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if opts.tokenize && opts.one_line { @@ -506,7 +506,7 @@ fn validate_read_args( "--line", "--tokenize" )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } // Verify all variable names. @@ -516,7 +516,7 @@ fn validate_read_args( .err .append(wgettext_fmt!(BUILTIN_ERR_VARNAME, cmd, arg)); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if EnvVar::flags_for(arg).contains(EnvVarFlags::READ_ONLY) { streams.err.append(wgettext_fmt!( @@ -525,22 +525,20 @@ fn validate_read_args( arg )); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } } - STATUS_CMD_OK + Ok(SUCCESS) } /// The read builtin. Reads from stdin and stores the values in environment variables. -pub fn read(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Option { +pub fn read(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> BuiltinResult { let mut buff = WString::new(); - let mut exit_res; + let mut exit_res: BuiltinResult; + + let (mut opts, optind) = parse_cmd_opts(argv, parser, streams)?; - let (mut opts, optind) = match parse_cmd_opts(argv, parser, streams) { - Ok(res) => res, - Err(retval) => return retval, - }; let cmd = argv[0]; let mut argv: &[&wstr] = argv; if !opts.to_stdout { @@ -554,20 +552,17 @@ pub fn read(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt if opts.print_help { builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } - let retval = validate_read_args(cmd, &mut opts, argv, parser, streams); - if retval != STATUS_CMD_OK { - return retval; - } + validate_read_args(cmd, &mut opts, argv, parser, streams)?; // stdin may have been explicitly closed if streams.stdin_fd < 0 { streams .err .append(wgettext_fmt!("%ls: stdin is closed\n", cmd)); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } if opts.one_line { @@ -634,7 +629,7 @@ pub fn read(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt read_one_char_at_a_time(streams.stdin_fd, &mut buff, opts.nchars, opts.split_null); } - if exit_res != STATUS_CMD_OK { + if exit_res.is_err() { clear_remaining_vars(&mut var_ptr); return exit_res; } diff --git a/src/builtins/realpath.rs b/src/builtins/realpath.rs index b93c27c70..628439a4d 100644 --- a/src/builtins/realpath.rs +++ b/src/builtins/realpath.rs @@ -25,7 +25,7 @@ fn parse_options( args: &mut [&wstr], parser: &Parser, streams: &mut IoStreams, -) -> Result<(Options, usize), Option> { +) -> Result<(Options, usize), ErrorCode> { let cmd = args[0]; let mut opts = Options::default(); @@ -54,17 +54,13 @@ fn parse_options( /// An implementation of the external realpath command. Doesn't support any options. /// In general scripts shouldn't invoke this directly. They should just use `realpath` which /// will fallback to this builtin if an external command cannot be found. -pub fn realpath(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { +pub fn realpath(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { let cmd = args[0]; - let (opts, optind) = match parse_options(args, parser, streams) { - Ok((opts, optind)) => (opts, optind), - Err(err @ Some(_)) if err != STATUS_CMD_OK => return err, - Err(err) => panic!("Illogical exit code from parse_options(): {err:?}"), - }; + let (opts, optind) = parse_options(args, parser, streams)?; if opts.print_help { builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } // TODO: allow arbitrary args. `realpath *` should print many paths @@ -76,7 +72,7 @@ pub fn realpath(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> args.len() - 1 )); builtin_print_help(parser, streams, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } let arg = args[optind]; @@ -102,7 +98,7 @@ pub fn realpath(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> .append(wgettext_fmt!("builtin %ls: Invalid arg: %ls\n", cmd, arg)); } - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } else { // We need to get the *physical* pwd here. @@ -121,11 +117,11 @@ pub fn realpath(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> cmd, errno().to_string() )); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } streams.out.append(L!("\n")); - STATUS_CMD_OK + Ok(SUCCESS) } diff --git a/src/builtins/return.rs b/src/builtins/return.rs index cb8abe907..bcba616a7 100644 --- a/src/builtins/return.rs +++ b/src/builtins/return.rs @@ -1,5 +1,7 @@ // Implementation of the return builtin. +use std::ops::ControlFlow; + use super::prelude::*; #[derive(Debug, Clone, Copy, Default)] @@ -11,7 +13,7 @@ fn parse_options( args: &mut [&wstr], parser: &Parser, streams: &mut IoStreams, -) -> Result<(Options, usize), Option> { +) -> ControlFlow { let cmd = args[0]; const SHORT_OPTS: &wstr = L!(":h"); @@ -26,13 +28,13 @@ fn parse_options( 'h' => opts.print_help = true, ':' => { builtin_missing_argument(parser, streams, cmd, args[w.wopt_index - 1], true); - return Err(STATUS_INVALID_ARGS); + return ControlFlow::Break(STATUS_INVALID_ARGS); } '?' => { // We would normally invoke builtin_unknown_option() and return an error. // But for this command we want to let it try and parse the value as a negative // return value. - return Ok((opts, w.wopt_index - 1)); + return ControlFlow::Continue((opts, w.wopt_index - 1)); } _ => { panic!("unexpected retval from next_opt"); @@ -40,14 +42,14 @@ fn parse_options( } } - Ok((opts, w.wopt_index)) + ControlFlow::Continue((opts, w.wopt_index)) } /// Function for handling the return builtin. -pub fn r#return(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { +pub fn r#return(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { let mut retval = match parse_return_value(args, parser, streams) { - Ok(v) => v, - Err(e) => return e, + ControlFlow::Continue(r) => r, + ControlFlow::Break(result) => return result, }; let has_function_block = parser.blocks_iter_rev().any(|b| b.is_function_call()); @@ -60,7 +62,9 @@ pub fn r#return(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> // Note in Rust, dividend % divisor has the same sign as the dividend. if retval < 0 { retval = 256 - (retval % 256).abs(); - } + }; + + let retval = BuiltinResult::from_dynamic(retval); // If we're not in a function, exit the current script (but not an interactive shell). if !has_function_block { @@ -68,48 +72,51 @@ pub fn r#return(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> if !ld.is_interactive { ld.exit_current_script = true; } - return Some(retval); + return retval; } // Mark a return in the libdata. parser.libdata_mut().returning = true; - return Some(retval); + retval } pub fn parse_return_value( args: &mut [&wstr], parser: &Parser, streams: &mut IoStreams, -) -> Result> { +) -> ControlFlow { + // TODO: use map_break let cmd = args[0]; let (opts, optind) = match parse_options(args, parser, streams) { - Ok((opts, optind)) => (opts, optind), - Err(err @ Some(_)) if err != STATUS_CMD_OK => return Err(err), - Err(err) => panic!("Illogical exit code from parse_options(): {err:?}"), + ControlFlow::Continue(o) => o, + ControlFlow::Break(error_code) => { + return ControlFlow::Break(BuiltinResult::Err(error_code)) + } }; + if opts.print_help { builtin_print_help(parser, streams, cmd); - return Err(STATUS_CMD_OK); + return ControlFlow::Break(Ok(SUCCESS)); } if optind + 1 < args.len() { streams .err .append(wgettext_fmt!(BUILTIN_ERR_TOO_MANY_ARGUMENTS, cmd)); builtin_print_error_trailer(parser, streams.err, cmd); - return Err(STATUS_INVALID_ARGS); + return ControlFlow::Break(Err(STATUS_INVALID_ARGS)); } if optind == args.len() { - Ok(parser.get_last_status()) + ControlFlow::Continue(parser.get_last_status()) } else { match fish_wcstoi(args[optind]) { - Ok(i) => Ok(i), + Ok(i) => ControlFlow::Continue(i), Err(_e) => { streams .err .append(wgettext_fmt!(BUILTIN_ERR_NOT_NUMBER, cmd, args[1])); builtin_print_error_trailer(parser, streams.err, cmd); - return Err(STATUS_INVALID_ARGS); + ControlFlow::Break(Err(STATUS_INVALID_ARGS)) } } } diff --git a/src/builtins/set.rs b/src/builtins/set.rs index 1b22ae8b0..a74236d10 100644 --- a/src/builtins/set.rs +++ b/src/builtins/set.rs @@ -96,7 +96,7 @@ fn parse( args: &mut [&wstr], parser: &Parser, streams: &mut IoStreams, - ) -> Result<(Options, usize), Option> { + ) -> Result, ErrorCode> { /// Values used for long-only options. const PATH_ARG: char = 1 as char; const UNPATH_ARG: char = 2 as char; @@ -198,12 +198,12 @@ fn parse( if opts.print_help { builtin_print_help(parser, streams, cmd); - return Err(STATUS_CMD_OK); + return Ok(None); } Self::validate(&opts, cmd, args, optind, parser, streams)?; - Ok((opts, optind)) + Ok(Some((opts, optind))) } fn validate( @@ -213,7 +213,7 @@ fn validate( optind: usize, parser: &Parser, streams: &mut IoStreams, - ) -> Result<(), Option> { + ) -> Result<(), ErrorCode> { // Can't query and erase or list. if opts.query && (opts.erase || opts.list) { streams.err.append(wgettext_fmt!(BUILTIN_ERR_COMBO, cmd)); @@ -530,7 +530,7 @@ fn erased_at_indexes(mut input: Vec, mut indexes: Vec) -> Vec Option { +fn list(opts: &Options, parser: &Parser, streams: &mut IoStreams) -> BuiltinResult { let names_only = opts.list; let mut names = parser.vars().get_names(opts.scope()); names.sort(); @@ -573,7 +573,7 @@ fn list(opts: &Options, parser: &Parser, streams: &mut IoStreams) -> Option Option { +) -> BuiltinResult { let mut retval = 0; let scope = opts.scope(); // No variables given, this is an error. // 255 is the maximum return code we allow. if args.is_empty() { - return Some(255); + return Err(STATUS_NO_VARIABLES_GIVEN); } for arg in args { let Some(split) = split_var_and_indexes(arg, scope, parser.vars(), streams) else { builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); }; if split.indexes.is_empty() { @@ -614,7 +614,7 @@ fn query( } } - Some(retval) + BuiltinResult::from_dynamic(retval) } fn show_scope(var_name: &wstr, scope: EnvMode, streams: &mut IoStreams, vars: &dyn Environment) { @@ -682,7 +682,7 @@ fn show_scope(var_name: &wstr, scope: EnvMode, streams: &mut IoStreams, vars: &d } /// Show mode. Show information about the named variable(s). -fn show(cmd: &wstr, parser: &Parser, streams: &mut IoStreams, args: &[&wstr]) -> Option { +fn show(cmd: &wstr, parser: &Parser, streams: &mut IoStreams, args: &[&wstr]) -> BuiltinResult { let vars = parser.vars(); if args.is_empty() { // show all vars @@ -716,7 +716,7 @@ fn show(cmd: &wstr, parser: &Parser, streams: &mut IoStreams, args: &[&wstr]) -> .err .append(wgettext_fmt!(BUILTIN_ERR_VARNAME, cmd, arg)); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if arg.contains('[') { @@ -725,7 +725,7 @@ fn show(cmd: &wstr, parser: &Parser, streams: &mut IoStreams, args: &[&wstr]) -> cmd )); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } show_scope(arg, EnvMode::LOCAL, streams, vars); @@ -745,7 +745,7 @@ fn show(cmd: &wstr, parser: &Parser, streams: &mut IoStreams, args: &[&wstr]) -> } } - STATUS_CMD_OK + Ok(SUCCESS) } fn erase( @@ -754,8 +754,8 @@ fn erase( parser: &Parser, streams: &mut IoStreams, args: &[&wstr], -) -> Option { - let mut ret = STATUS_CMD_OK; +) -> BuiltinResult { + let mut ret = Ok(SUCCESS); let scopes = opts.scope(); // `set -e` is allowed to be called with multiple scopes. for bit in (0..).take_while(|bit| 1 << bit <= EnvMode::USER.bits()) { @@ -766,7 +766,7 @@ fn erase( for arg in args { let Some(split) = split_var_and_indexes(arg, scope, parser.vars(), streams) else { builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); }; if !valid_var_name(split.varname) { @@ -774,7 +774,7 @@ fn erase( .err .append(wgettext_fmt!(BUILTIN_ERR_VARNAME, cmd, split.varname)); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } let retval; if split.indexes.is_empty() { @@ -792,7 +792,7 @@ fn erase( } else { // remove just the specified indexes of the var let Some(var) = split.var else { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); }; let result = erased_at_indexes(var.as_list().to_owned(), split.indexes); retval = env_set_reporting_errors( @@ -809,23 +809,13 @@ 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::Ok { - ret = env_result_to_status(retval); + ret = retval.into(); } } } ret } -fn env_result_to_status(retval: EnvStackSetResult) -> Option { - Some(match retval { - EnvStackSetResult::Ok => 0, - EnvStackSetResult::Perm => 1, - EnvStackSetResult::Scope => 2, - EnvStackSetResult::Invalid => 3, - EnvStackSetResult::NotFound => 4, - }) -} - /// Return a list of new values for the variable `varname`, respecting the `opts`. /// This handles the simple case where there are no indexes. fn new_var_values( @@ -897,16 +887,15 @@ fn set_internal( cmd: &wstr, opts: &Options, parser: &Parser, - streams: &mut IoStreams, argv: &[&wstr], -) -> Option { +) -> BuiltinResult { if argv.is_empty() { streams .err .append(wgettext_fmt!(BUILTIN_ERR_MIN_ARG_COUNT1, cmd, 1)); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } let scope = opts.scope(); @@ -915,7 +904,7 @@ fn set_internal( let Some(split) = split_var_and_indexes(var_expr, scope, parser.vars(), streams) else { builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); }; // Is the variable valid? @@ -932,7 +921,7 @@ fn set_internal( )); } builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } // Setting with explicit indexes like `set foo[3] ...` has additional error handling. @@ -944,7 +933,7 @@ fn set_internal( .err .append(wgettext_fmt!("%ls: array index out of bounds\n", cmd)); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } } // Append and prepend are disallowed. @@ -954,7 +943,7 @@ fn set_internal( cmd )); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } // Argument count and index count must agree. @@ -965,7 +954,7 @@ fn set_internal( split.indexes.len(), argv.len() )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } } @@ -984,15 +973,16 @@ fn set_internal( if retval == EnvStackSetResult::Ok { warn_if_uvar_shadows_global(cmd, opts, split.varname, streams, parser); } - env_result_to_status(retval) + + retval.into() } /// The set builtin creates, updates, and erases (removes, deletes) variables. -pub fn set(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { +pub fn set(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { let cmd = args[0]; - let (opts, optind) = match Options::parse(cmd, args, parser, streams) { - Ok(res) => res, - Err(err) => return err, + let (opts, optind) = match Options::parse(cmd, args, parser, streams)? { + Some((opts, optind)) => (opts, optind), + None => return Ok(SUCCESS), }; let args = &args[optind..]; @@ -1011,8 +1001,10 @@ pub fn set(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Opti set_internal(cmd, &opts, parser, streams, args) }; - if retval == STATUS_CMD_OK && opts.preserve_failure_exit_status { - return None; + if retval.is_ok() && opts.preserve_failure_exit_status { + return Ok(Success { + preserve_failure_exit_status: true, + }); } return retval; diff --git a/src/builtins/set_color.rs b/src/builtins/set_color.rs index e4300a54e..06ac23ab6 100644 --- a/src/builtins/set_color.rs +++ b/src/builtins/set_color.rs @@ -116,14 +116,14 @@ fn print_colors( ]; /// set_color builtin. -pub fn set_color(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Option { +pub fn set_color(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> BuiltinResult { // Variables used for parsing the argument list. let argc = argv.len(); // Some code passes variables to set_color that don't exist, like $fish_user_whatever. As a // hack, quietly return failure. if argc <= 1 { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } let mut bgcolor = None; @@ -143,7 +143,7 @@ pub fn set_color(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) - } 'h' => { builtin_print_help(parser, streams, argv[0]); - return STATUS_CMD_OK; + return Ok(SUCCESS); } 'o' => bold = true, 'i' => italics = true, @@ -154,7 +154,7 @@ pub fn set_color(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) - ':' => { // We don't error here because "-b" is the only option that requires an argument, // and we don't error for missing colors. - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } '?' => { builtin_unknown_option( @@ -164,7 +164,7 @@ pub fn set_color(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) - argv[w.wopt_index - 1], true, /* print_hints */ ); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } _ => unreachable!("unexpected retval from WGetopter"), } @@ -179,7 +179,7 @@ pub fn set_color(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) - argv[0], bgcolor.unwrap() )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if print { @@ -191,7 +191,7 @@ pub fn set_color(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) - } let args = &argv[wopt_index..argc]; print_colors(streams, args, bold, underline, italics, dim, reverse, bg); - return STATUS_CMD_OK; + return Ok(SUCCESS); } // Remaining arguments are foreground color. @@ -204,7 +204,7 @@ pub fn set_color(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) - argv[0], argv[wopt_index] )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); }; fgcolors.push(fg); wopt_index += 1; @@ -218,10 +218,10 @@ pub fn set_color(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) - // Test if we have at least basic support for setting fonts, colors and related bits - otherwise // just give up... let Some(term) = terminal::term() else { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); }; let Some(exit_attribute_mode) = &term.exit_attribute_mode else { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); }; let outp = &mut output::Outputter::new_buffering(); @@ -248,5 +248,5 @@ pub fn set_color(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) - let contents = outp.contents(); streams.out.append(str2wcstring(contents)); - STATUS_CMD_OK + Ok(SUCCESS) } diff --git a/src/builtins/shared.rs b/src/builtins/shared.rs index f80935333..5d36c20bc 100644 --- a/src/builtins/shared.rs +++ b/src/builtins/shared.rs @@ -16,7 +16,7 @@ use std::os::fd::FromRawFd; use std::sync::Arc; -pub type BuiltinCmd = fn(&Parser, &mut IoStreams, &mut [&wstr]) -> Option; +pub type BuiltinCmd = fn(&Parser, &mut IoStreams, &mut [&wstr]) -> BuiltinResult; /// The default prompt for the read command. pub const DEFAULT_READ_PROMPT: &wstr = @@ -77,29 +77,70 @@ // Return values (`$status` values for fish scripts) for various situations. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub struct Success { + pub preserve_failure_exit_status: bool, +} + +pub const SUCCESS: Success = Success { + preserve_failure_exit_status: false, +}; + +// To-do: this should be a nonzero type. +pub type ErrorCode = c_int; + +pub type BuiltinResult = Result; + +pub trait BuiltinResultExt { + fn from_dynamic(code: c_int) -> Self; + fn builtin_status_code(&self) -> c_int; +} + +impl BuiltinResultExt for BuiltinResult { + fn from_dynamic(code: c_int) -> Self { + if code == 0 { + Ok(SUCCESS) + } else { + Err(code) + } + } + fn builtin_status_code(&self) -> c_int { + match self { + Ok(success) => { + assert!(!success.preserve_failure_exit_status); + 0 + } + Err(err) => *err, + } + } +} + /// The status code used for normal exit in a command. -pub const STATUS_CMD_OK: Option = Some(0); +pub const STATUS_CMD_OK: c_int = 0; + /// The status code used for failure exit in a command (but not if the args were invalid). -pub const STATUS_CMD_ERROR: Option = Some(1); +pub const STATUS_CMD_ERROR: c_int = 1; /// The status code used for invalid arguments given to a command. This is distinct from valid /// arguments that might result in a command failure. An invalid args condition is something /// like an unrecognized flag, missing or too many arguments, an invalid integer, etc. -pub const STATUS_INVALID_ARGS: Option = Some(2); +pub const STATUS_INVALID_ARGS: c_int = 2; /// The status code used when a command was not found. -pub const STATUS_CMD_UNKNOWN: Option = Some(127); +pub const STATUS_CMD_UNKNOWN: c_int = 127; /// The status code used when an external command can not be run. -pub const STATUS_NOT_EXECUTABLE: Option = Some(126); +pub const STATUS_NOT_EXECUTABLE: c_int = 126; /// The status code used when a wildcard had no matches. -pub const STATUS_UNMATCHED_WILDCARD: Option = Some(124); +pub const STATUS_UNMATCHED_WILDCARD: c_int = 124; /// The status code used when illegal command name is encountered. -pub const STATUS_ILLEGAL_CMD: Option = Some(123); +pub const STATUS_ILLEGAL_CMD: c_int = 123; /// The status code used when `read` is asked to consume too much data. -pub const STATUS_READ_TOO_MUCH: Option = Some(122); +pub const STATUS_READ_TOO_MUCH: c_int = 122; /// The status code when an expansion fails, for example, "$foo[" -pub const STATUS_EXPAND_ERROR: Option = Some(121); +pub const STATUS_EXPAND_ERROR: c_int = 121; + +pub const STATUS_NO_VARIABLES_GIVEN: c_int = 255; /// Data structure to describe a builtin. struct BuiltinData { @@ -404,7 +445,7 @@ fn cmd_needs_help(cmd: &wstr) -> bool { /// Execute a builtin command pub fn builtin_run(parser: &Parser, argv: &mut [&wstr], streams: &mut IoStreams) -> ProcStatus { if argv.is_empty() { - return ProcStatus::from_exit_code(STATUS_INVALID_ARGS.unwrap()); + return ProcStatus::from_exit_code(STATUS_INVALID_ARGS); } // We can be handed a keyword by the parser as if it was a command. This happens when the user @@ -412,12 +453,12 @@ pub fn builtin_run(parser: &Parser, argv: &mut [&wstr], streams: &mut IoStreams) // handle displaying help for it here. if argv.len() == 2 && parse_util_argument_is_help(argv[1]) && cmd_needs_help(argv[0]) { builtin_print_help(parser, streams, argv[0]); - return ProcStatus::from_exit_code(STATUS_CMD_OK.unwrap()); + return ProcStatus::from_exit_code(STATUS_CMD_OK); } let Some(builtin) = builtin_lookup(argv[0]) else { FLOGF!(error, "%s", wgettext_fmt!(UNKNOWN_BUILTIN_ERR_MSG, argv[0])); - return ProcStatus::from_exit_code(STATUS_CMD_ERROR.unwrap()); + return ProcStatus::from_exit_code(STATUS_CMD_ERROR); }; let builtin_ret = (builtin.func)(parser, streams, argv); @@ -429,7 +470,17 @@ pub fn builtin_run(parser: &Parser, argv: &mut [&wstr], streams: &mut IoStreams) // Resolve our status code. // If the builtin itself produced an error, use that error. // Otherwise use any errors from writing to out and writing to err, in that order. - let mut code = builtin_ret.unwrap_or(0); + let mut code = match builtin_ret { + Ok(success) => { + if success.preserve_failure_exit_status { + return ProcStatus::empty(); + } else { + 0 + } + } + Err(err) => err, + }; + if code == 0 { code = out_ret; } @@ -443,10 +494,6 @@ pub fn builtin_run(parser: &Parser, argv: &mut [&wstr], streams: &mut IoStreams) code = 255; } - // Handle the case of an empty status. - if code == 0 && builtin_ret.is_none() { - return ProcStatus::empty(); - } if code < 0 { // If the code is below 0, constructing a proc_status_t // would assert() out, which is a terrible failure mode @@ -659,7 +706,7 @@ pub fn parse( args: &mut [&wstr], parser: &Parser, streams: &mut IoStreams, - ) -> Result> { + ) -> Result { let cmd = args[0]; let print_hints = true; @@ -846,26 +893,23 @@ fn next(&mut self) -> Option { /// A generic builtin that only supports showing a help message. This is only a placeholder that /// prints the help message. Useful for commands that live in the parser. -fn builtin_generic(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Option { +fn builtin_generic(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> BuiltinResult { let argc = argv.len(); - let opts = match HelpOnlyCmdOpts::parse(argv, parser, streams) { - Ok(opts) => opts, - Err(err) => return err, - }; + let opts = HelpOnlyCmdOpts::parse(argv, parser, streams)?; if opts.print_help { builtin_print_help(parser, streams, argv[0]); - return STATUS_CMD_OK; + return Ok(SUCCESS); } // Hackish - if we have no arguments other than the command, we are a "naked invocation" and we // just print help. if argc == 1 || argv[0] == "time" { builtin_print_help(parser, streams, argv[0]); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) } /// This function handles both the 'continue' and the 'break' builtins that are used for loop @@ -874,14 +918,14 @@ fn builtin_break_continue( parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr], -) -> Option { +) -> BuiltinResult { let is_break = argv[0] == "break"; let argc = argv.len(); if argc != 1 { let error_message = wgettext_fmt!(BUILTIN_ERR_UNKNOWN, argv[0], argv[1]); builtin_print_help_error(parser, streams, argv[0], &error_message); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } // Paranoia: ensure we have a real loop. @@ -899,7 +943,7 @@ fn builtin_break_continue( if !has_loop { let error_message = wgettext_fmt!("%ls: Not inside of loop\n", argv[0]); builtin_print_help_error(parser, streams, argv[0], &error_message); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } // Mark the status in the libdata. @@ -908,7 +952,7 @@ fn builtin_break_continue( } else { LoopStatus::continues }; - STATUS_CMD_OK + Ok(SUCCESS) } /// Implementation of the builtin breakpoint command, used to launch the interactive debugger. @@ -916,7 +960,7 @@ fn builtin_breakpoint( parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr], -) -> Option { +) -> BuiltinResult { let cmd = argv[0]; if argv.len() != 1 { streams.err.append(wgettext_fmt!( @@ -925,12 +969,12 @@ fn builtin_breakpoint( 0, argv.len() - 1 )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } // If we're not interactive then we can't enter the debugger. So treat this command as a no-op. if !parser.is_interactive() { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } // Ensure we don't allow creating a breakpoint at an interactive prompt. There may be a simpler @@ -944,28 +988,28 @@ fn builtin_breakpoint( "%ls: Command not valid at an interactive prompt\n", cmd, )); - return STATUS_ILLEGAL_CMD; + return Err(STATUS_ILLEGAL_CMD); } } let bpb = parser.push_block(Block::breakpoint_block()); let io_chain = &streams.io_chain; - reader_read(parser, STDIN_FILENO, io_chain); + reader_read(parser, STDIN_FILENO, io_chain)?; parser.pop_block(bpb); - Some(parser.get_last_status()) + BuiltinResult::from_dynamic(parser.get_last_status()) } -fn builtin_true(_parser: &Parser, _streams: &mut IoStreams, _argv: &mut [&wstr]) -> Option { - STATUS_CMD_OK +fn builtin_true(_parser: &Parser, _streams: &mut IoStreams, _argv: &mut [&wstr]) -> BuiltinResult { + Ok(SUCCESS) } -fn builtin_false(_parser: &Parser, _streams: &mut IoStreams, _argv: &mut [&wstr]) -> Option { - STATUS_CMD_ERROR +fn builtin_false(_parser: &Parser, _streams: &mut IoStreams, _argv: &mut [&wstr]) -> BuiltinResult { + Err(STATUS_CMD_ERROR) } -fn builtin_gettext(_parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Option { +fn builtin_gettext(_parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> BuiltinResult { for arg in &argv[1..] { streams.out.append(wgettext_str(arg)); } - STATUS_CMD_OK + Ok(SUCCESS) } diff --git a/src/builtins/source.rs b/src/builtins/source.rs index 6339d94f0..082ca9b9c 100644 --- a/src/builtins/source.rs +++ b/src/builtins/source.rs @@ -13,18 +13,18 @@ /// The source builtin, sometimes called `.`. Evaluates the contents of a file in the current /// context. -pub fn source(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { +pub fn source(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { let argc = args.len(); - let opts = match HelpOnlyCmdOpts::parse(args, parser, streams) { - Ok(opts) => opts, - Err(err) => return err, + let opts = HelpOnlyCmdOpts::parse(args, parser, streams)?; + let cmd = match args.get(0) { + Some(&cmd) => cmd, + None => return Err(STATUS_INVALID_ARGS), }; - let cmd = args[0]; if opts.print_help { builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } // If we open a file, this ensures it stays open until the end of scope. @@ -40,7 +40,7 @@ pub fn source(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O streams .err .append(wgettext_fmt!("%ls: stdin is closed\n", cmd)); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } // Either a bare `source` which means to implicitly read from stdin or an explicit `-`. if argc == optind && isatty(streams.stdin_fd) { @@ -49,7 +49,7 @@ pub fn source(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O "%ls: missing filename argument or input redirection\n", cmd )); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } func_filename = FilenameRef::new(L!("-").to_owned()); fd = streams.stdin_fd; @@ -66,7 +66,7 @@ pub fn source(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O &esc )); builtin_wperror(cmd, streams); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } }; @@ -90,20 +90,20 @@ pub fn source(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O let argv_list = remaining_args.iter().map(|&arg| arg.to_owned()).collect(); parser.vars().set_argv(argv_list); - let mut retval = reader_read(parser, fd, streams.io_chain); + let retval = reader_read(parser, fd, streams.io_chain); parser.pop_block(sb); - if retval != STATUS_CMD_OK.unwrap() { - let esc = escape(&func_filename); - streams.err.append(wgettext_fmt!( - "%ls: Error while reading file '%ls'\n", - cmd, - if esc == "-" { L!("") } else { &esc } - )); - } else { - retval = parser.get_last_status(); + match retval { + Ok(_) => BuiltinResult::from_dynamic(parser.get_last_status()), + Err(err) => { + let esc = escape(&func_filename); + streams.err.append(wgettext_fmt!( + "%ls: Error while reading file '%ls'\n", + cmd, + if esc == "-" { L!("") } else { &esc } + )); + Err(err) + } } - - Some(retval) } diff --git a/src/builtins/status.rs b/src/builtins/status.rs index 51c82361f..b26177c0e 100644 --- a/src/builtins/status.rs +++ b/src/builtins/status.rs @@ -210,7 +210,7 @@ fn parse_cmd_opts( args: &mut [&wstr], parser: &Parser, streams: &mut IoStreams, -) -> Option { +) -> BuiltinResult { let cmd = args[0]; let mut args_read = Vec::with_capacity(args.len()); @@ -230,13 +230,13 @@ fn parse_cmd_opts( cmd, arg )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } _ => { streams .err .append(wgettext_fmt!(BUILTIN_ERR_NOT_NUMBER, cmd, arg)); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } } }; @@ -253,12 +253,12 @@ fn parse_cmd_opts( _ => unreachable!(), }; if !opts.try_set_status_cmd(subcmd, streams) { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } 'j' => { if !opts.try_set_status_cmd(STATUS_SET_JOB_CONTROL, streams) { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } let Ok(job_mode) = w.woptarg.unwrap().try_into() else { streams.err.append(wgettext_fmt!( @@ -266,43 +266,43 @@ fn parse_cmd_opts( cmd, w.woptarg.unwrap() )); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); }; opts.new_job_control_mode = Some(job_mode); } IS_FULL_JOB_CTRL_SHORT => { if !opts.try_set_status_cmd(STATUS_IS_FULL_JOB_CTRL, streams) { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } IS_INTERACTIVE_JOB_CTRL_SHORT => { if !opts.try_set_status_cmd(STATUS_IS_INTERACTIVE_JOB_CTRL, streams) { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } IS_INTERACTIVE_READ_SHORT => { if !opts.try_set_status_cmd(STATUS_IS_INTERACTIVE_READ, streams) { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } IS_NO_JOB_CTRL_SHORT => { if !opts.try_set_status_cmd(STATUS_IS_NO_JOB_CTRL, streams) { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } FISH_PATH_SHORT => { if !opts.try_set_status_cmd(STATUS_FISH_PATH, streams) { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } 'h' => opts.print_help = true, ':' => { builtin_missing_argument(parser, streams, cmd, args[w.wopt_index - 1], false); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } '?' => { builtin_unknown_option(parser, streams, cmd, args[w.wopt_index - 1], false); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } _ => panic!("unexpected retval from WGetopter"), } @@ -310,23 +310,20 @@ fn parse_cmd_opts( *optind = w.wopt_index; - return STATUS_CMD_OK; + return Ok(SUCCESS); } -pub fn status(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { +pub fn status(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { let cmd = args[0]; let argc = args.len(); let mut opts = StatusCmdOpts::default(); let mut optind = 0usize; - let retval = parse_cmd_opts(&mut opts, &mut optind, args, parser, streams); - if retval != STATUS_CMD_OK { - return retval; - } + parse_cmd_opts(&mut opts, &mut optind, args, parser, streams)?; if opts.print_help { builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } // If a status command hasn't already been specified via a flag check the first word. @@ -335,7 +332,7 @@ pub fn status(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O match StatusCmd::from_wstr(args[optind].to_string().as_str()) { Some(s) => { if !opts.try_set_status_cmd(s, streams) { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } optind += 1; } @@ -343,7 +340,7 @@ pub fn status(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O streams .err .append(wgettext_fmt!(BUILTIN_ERR_INVALID_SUBCMD, cmd, args[1])); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } } } @@ -368,7 +365,7 @@ pub fn status(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O .append(wgettext_fmt!("Job control: %ls\n", job_control_mode)); streams.out.append(parser.stack_trace()); - return STATUS_CMD_OK; + return Ok(SUCCESS); }; match subcmd { @@ -384,7 +381,7 @@ pub fn status(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O 0, args.len() )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } j } @@ -397,7 +394,7 @@ pub fn status(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O 1, args.len() )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } let Ok(new_mode) = args[0].try_into() else { streams.err.append(wgettext_fmt!( @@ -405,7 +402,7 @@ pub fn status(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O cmd, args[0] )); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); }; new_mode } @@ -422,19 +419,18 @@ pub fn status(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O 1, args.len() )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } - use TestFeatureRetVal::*; - let mut retval = Some(TEST_FEATURE_NOT_RECOGNIZED as c_int); + let mut retval = TestFeatureRetVal::TEST_FEATURE_NOT_RECOGNIZED; for md in features::METADATA { if md.name == args[0] { retval = match feature_test(md.flag) { - true => Some(TEST_FEATURE_ON as c_int), - false => Some(TEST_FEATURE_OFF as c_int), + true => TestFeatureRetVal::TEST_FEATURE_ON, + false => TestFeatureRetVal::TEST_FEATURE_OFF, }; } } - return retval; + return Err(retval as i32); } ref s => { if !args.is_empty() { @@ -445,7 +441,7 @@ pub fn status(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O 0, args.len() )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } match s { STATUS_BUILDINFO => { @@ -485,7 +481,7 @@ pub fn status(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O .out .appendln(str2wcstring(features.join(" ").as_bytes())); streams.out.appendln(""); - return STATUS_CMD_OK; + return Ok(SUCCESS); } STATUS_BASENAME | STATUS_DIRNAME | STATUS_FILENAME => { let res = parser.current_filename(); @@ -515,65 +511,64 @@ pub fn status(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O } STATUS_IS_INTERACTIVE => { if is_interactive_session() { - return STATUS_CMD_OK; - } else { - return STATUS_CMD_ERROR; + return Ok(SUCCESS); } + return Err(STATUS_CMD_ERROR); } STATUS_IS_COMMAND_SUB => { if parser.libdata().is_subshell { - return STATUS_CMD_OK; + return Ok(SUCCESS); } else { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } STATUS_IS_BLOCK => { if parser.is_block() { - return STATUS_CMD_OK; + return Ok(SUCCESS); } else { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } STATUS_IS_BREAKPOINT => { if parser.is_breakpoint() { - return STATUS_CMD_OK; + return Ok(SUCCESS); } else { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } STATUS_IS_LOGIN => { if get_login() { - return STATUS_CMD_OK; + return Ok(SUCCESS); } else { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } STATUS_IS_FULL_JOB_CTRL => { if get_job_control_mode() == JobControl::all { - return STATUS_CMD_OK; + return Ok(SUCCESS); } else { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } STATUS_IS_INTERACTIVE_JOB_CTRL => { if get_job_control_mode() == JobControl::interactive { - return STATUS_CMD_OK; + return Ok(SUCCESS); } else { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } STATUS_IS_INTERACTIVE_READ => { if reader_in_interactive_read() { - return STATUS_CMD_OK; + return Ok(SUCCESS); } else { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } STATUS_IS_NO_JOB_CTRL => { if get_job_control_mode() == JobControl::none { - return STATUS_CMD_OK; + return Ok(SUCCESS); } else { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } STATUS_STACK_TRACE => { @@ -627,5 +622,5 @@ pub fn status(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O } }; - return retval; + Ok(SUCCESS) } diff --git a/src/builtins/string.rs b/src/builtins/string.rs index 9570d0fad..b62d6aaa6 100644 --- a/src/builtins/string.rs +++ b/src/builtins/string.rs @@ -52,7 +52,7 @@ fn parse_opts( args: &mut [&'args wstr], parser: &Parser, streams: &mut IoStreams, - ) -> Result> { + ) -> Result { let cmd = args[0]; let mut args_read = Vec::with_capacity(args.len()); args_read.extend_from_slice(args); @@ -95,8 +95,8 @@ fn take_args( optind: &mut usize, args: &[&'args wstr], streams: &mut IoStreams, - ) -> Option { - STATUS_CMD_OK + ) -> Result<(), ErrorCode> { + Ok(()) } /// Perform the business logic of the command. @@ -106,35 +106,41 @@ fn handle( streams: &mut IoStreams, optind: &mut usize, args: &[&'args wstr], - ) -> Option; + ) -> Result<(), ErrorCode>; fn run( &mut self, parser: &Parser, streams: &mut IoStreams, args: &mut [&'args wstr], - ) -> Option { + ) -> BuiltinResult { + match self.run_impl(parser, streams, args) { + Ok(()) => BuiltinResult::Ok(SUCCESS), + Err(err) => BuiltinResult::Err(err), + } + } + + fn run_impl( + &mut self, + parser: &Parser, + streams: &mut IoStreams, + args: &mut [&'args wstr], + ) -> Result<(), ErrorCode> { if args.len() >= 3 && (args[2] == "-h" || args[2] == "--help") { let string_dash_subcmd = WString::from(args[0]) + L!("-") + args[1]; builtin_print_help(parser, streams, &string_dash_subcmd); - return STATUS_CMD_OK; + return Ok(()); } let args = &mut args[1..]; - let mut optind = match self.parse_opts(args, parser, streams) { - Ok(optind) => optind, - Err(retval) => return retval, - }; + let mut optind = self.parse_opts(args, parser, streams)?; - let retval = self.take_args(&mut optind, args, streams); - if retval != STATUS_CMD_OK { - return retval; - } + self.take_args(&mut optind, args, streams)?; if streams.stdin_is_directly_redirected && args.len() > optind { string_error!(streams, BUILTIN_ERR_TOO_MANY_ARGUMENTS, args[0]); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } return self.handle(parser, streams, &mut optind, args); @@ -228,7 +234,7 @@ fn print_error( } } - fn retval(&self) -> Option { + fn retval(&self) -> ErrorCode { STATUS_INVALID_ARGS } } @@ -284,7 +290,7 @@ fn arguments<'iter, 'args>( } /// The string builtin, for manipulating strings. -pub fn string(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { +pub fn string(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { let cmd = args[0]; let argc = args.len(); @@ -293,12 +299,12 @@ pub fn string(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O .err .append(wgettext_fmt!(BUILTIN_ERR_MISSING_SUBCMD, cmd)); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if args[1] == "-h" || args[1] == "--help" { builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } let subcmd_name = args[1]; @@ -342,7 +348,7 @@ pub fn string(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O .err .append(wgettext_fmt!(BUILTIN_ERR_INVALID_SUBCMD, cmd, args[0])); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } } } diff --git a/src/builtins/string/collect.rs b/src/builtins/string/collect.rs index dbf7b3bbd..daa4076a7 100644 --- a/src/builtins/string/collect.rs +++ b/src/builtins/string/collect.rs @@ -28,7 +28,7 @@ fn handle( streams: &mut IoStreams, optind: &mut usize, args: &[&wstr], - ) -> Option { + ) -> Result<(), ErrorCode> { let mut appended = 0usize; for (arg, want_newline) in @@ -60,9 +60,9 @@ fn handle( } if appended > 0 { - STATUS_CMD_OK + Ok(()) } else { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) } } } diff --git a/src/builtins/string/escape.rs b/src/builtins/string/escape.rs index ad32dd3bf..0bc08cd74 100644 --- a/src/builtins/string/escape.rs +++ b/src/builtins/string/escape.rs @@ -34,7 +34,7 @@ fn handle( streams: &mut IoStreams, optind: &mut usize, args: &[&wstr], - ) -> Option { + ) -> Result<(), ErrorCode> { // Currently, only the script style supports options. // Ignore them for other styles for now. let style = match self.style { @@ -57,9 +57,9 @@ fn handle( } if escaped_any { - STATUS_CMD_OK + Ok(()) } else { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) } } } diff --git a/src/builtins/string/join.rs b/src/builtins/string/join.rs index 5dc4d2462..fa4b1d71c 100644 --- a/src/builtins/string/join.rs +++ b/src/builtins/string/join.rs @@ -39,19 +39,19 @@ fn take_args( optind: &mut usize, args: &[&'args wstr], streams: &mut IoStreams, - ) -> Option { + ) -> Result<(), ErrorCode> { if self.is_join0 { - return STATUS_CMD_OK; + return Ok(()); } let Some(arg) = args.get(*optind).copied() else { string_error!(streams, BUILTIN_ERR_ARG_COUNT0, args[0]); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); }; *optind += 1; self.sep = arg; - STATUS_CMD_OK + Ok(()) } fn handle( @@ -60,7 +60,7 @@ fn handle( streams: &mut IoStreams, optind: &mut usize, args: &[&wstr], - ) -> Option { + ) -> Result<(), ErrorCode> { let sep = &self.sep; let mut nargs = 0usize; let mut print_trailing_newline = true; @@ -76,7 +76,7 @@ fn handle( streams.out.append(arg); } else if nargs > 1 { - return STATUS_CMD_OK; + return Ok(()); } nargs += 1; print_trailing_newline = want_newline; @@ -91,9 +91,9 @@ fn handle( } if nargs > 1 { - STATUS_CMD_OK + Ok(()) } else { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) } } } diff --git a/src/builtins/string/length.rs b/src/builtins/string/length.rs index 8c4ede64a..410c3d260 100644 --- a/src/builtins/string/length.rs +++ b/src/builtins/string/length.rs @@ -28,7 +28,7 @@ fn handle( streams: &mut IoStreams, optind: &mut usize, args: &[&wstr], - ) -> Option { + ) -> Result<(), ErrorCode> { let mut nnonempty = 0usize; for (arg, _) in arguments(args, optind, streams) { @@ -54,7 +54,7 @@ fn handle( if !self.quiet { streams.out.appendln(max.to_wstring()); } else if nnonempty > 0 { - return STATUS_CMD_OK; + return Ok(()); } } } else { @@ -65,14 +65,14 @@ fn handle( if !self.quiet { streams.out.appendln(n.to_wstring()); } else if nnonempty > 0 { - return STATUS_CMD_OK; + return Ok(()); } } } if nnonempty > 0 { - STATUS_CMD_OK + Ok(()) } else { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) } } } diff --git a/src/builtins/string/match.rs b/src/builtins/string/match.rs index 18492710b..514be9c81 100644 --- a/src/builtins/string/match.rs +++ b/src/builtins/string/match.rs @@ -72,15 +72,15 @@ fn take_args( optind: &mut usize, args: &[&'args wstr], streams: &mut IoStreams, - ) -> Option { + ) -> Result<(), ErrorCode> { let cmd = args[0]; let Some(arg) = args.get(*optind).copied() else { string_error!(streams, BUILTIN_ERR_ARG_COUNT0, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); }; *optind += 1; self.pattern = arg; - STATUS_CMD_OK + Ok(()) } fn handle( @@ -89,7 +89,7 @@ fn handle( streams: &mut IoStreams, optind: &mut usize, args: &[&wstr], - ) -> Option { + ) -> Result<(), ErrorCode> { let cmd = args[0]; if self.entire && self.index { @@ -98,7 +98,7 @@ fn handle( cmd, wgettext!("--entire and --index are mutually exclusive") )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if self.invert_match && self.groups_only { @@ -107,7 +107,7 @@ fn handle( cmd, wgettext!("--invert and --groups-only are mutually exclusive") )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if self.entire && self.groups_only { @@ -116,14 +116,14 @@ fn handle( cmd, wgettext!("--entire and --groups-only are mutually exclusive") )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } let mut matcher = match StringMatcher::new(self.pattern, self) { Ok(m) => m, Err(e) => { e.print_error(args, streams); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } }; @@ -152,9 +152,9 @@ fn handle( } if match_count > 0 { - STATUS_CMD_OK + Ok(()) } else { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) } } } diff --git a/src/builtins/string/pad.rs b/src/builtins/string/pad.rs index adc4505df..9b704dbf1 100644 --- a/src/builtins/string/pad.rs +++ b/src/builtins/string/pad.rs @@ -66,7 +66,7 @@ fn handle<'args>( streams: &mut IoStreams, optind: &mut usize, args: &[&'args wstr], - ) -> Option { + ) -> Result<(), ErrorCode> { let mut max_width = 0usize; let mut inputs: Vec<(Cow<'args, wstr>, usize)> = Vec::new(); let mut print_trailing_newline = true; @@ -105,6 +105,6 @@ fn handle<'args>( streams.out.append(padded); } - STATUS_CMD_OK + Ok(()) } } diff --git a/src/builtins/string/repeat.rs b/src/builtins/string/repeat.rs index 4b5de4d5b..da3c18856 100644 --- a/src/builtins/string/repeat.rs +++ b/src/builtins/string/repeat.rs @@ -44,26 +44,26 @@ fn take_args( optind: &mut usize, args: &[&'_ wstr], streams: &mut IoStreams, - ) -> Option { + ) -> Result<(), ErrorCode> { if self.count.is_some() || self.max.is_some() { - return STATUS_CMD_OK; + return Ok(()); } let name = args[0]; let Some(arg) = args.get(*optind) else { string_error!(streams, BUILTIN_ERR_ARG_COUNT0, name); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); }; *optind += 1; let Ok(Ok(count)) = fish_wcstol(arg).map(|count| count.try_into()) else { string_error!(streams, "%ls: Invalid count value '%ls'\n", name, arg); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); }; self.count = Some(count); - return STATUS_CMD_OK; + Ok(()) } fn handle( @@ -72,7 +72,7 @@ fn handle( streams: &mut IoStreams, optind: &mut usize, args: &[&wstr], - ) -> Option { + ) -> Result<(), ErrorCode> { let max = self.max.unwrap_or_default(); let count = self.count.unwrap_or_default(); @@ -80,7 +80,7 @@ fn handle( // XXX: This used to be allowed, but returned 1. // Keep it that way for now instead of adding an error. // streams.err.append(L"Count or max must be greater than zero"); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } let mut all_empty = true; @@ -97,7 +97,7 @@ fn handle( if self.quiet { // Early out if we can - see #7495. - return STATUS_CMD_OK; + return Ok(()); } if !first { @@ -147,8 +147,8 @@ fn handle( // so we need to check every so often if the pipe has been closed. // If we didn't, running `string repeat -n LARGENUMBER foo | pv` // and pressing ctrl-c seems to hang. - if streams.out.flush_and_check_error() != STATUS_CMD_OK.unwrap() { - return STATUS_CMD_ERROR; + if streams.out.flush_and_check_error() != STATUS_CMD_OK { + return Err(STATUS_CMD_ERROR); } i -= chunk.len(); } @@ -168,9 +168,9 @@ fn handle( } if all_empty { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) } else { - STATUS_CMD_OK + Ok(()) } } } diff --git a/src/builtins/string/replace.rs b/src/builtins/string/replace.rs index 2b218e644..76192b561 100644 --- a/src/builtins/string/replace.rs +++ b/src/builtins/string/replace.rs @@ -61,22 +61,22 @@ fn take_args( optind: &mut usize, args: &[&'args wstr], streams: &mut IoStreams, - ) -> Option { + ) -> Result<(), ErrorCode> { let cmd = args[0]; let Some(pattern) = args.get(*optind).copied() else { string_error!(streams, BUILTIN_ERR_ARG_COUNT0, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); }; *optind += 1; let Some(replacement) = args.get(*optind).copied() else { string_error!(streams, BUILTIN_ERR_ARG_COUNT1, cmd, 1, 2); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); }; *optind += 1; self.pattern = pattern; self.replacement = replacement; - return STATUS_CMD_OK; + Ok(()) } fn handle( @@ -85,14 +85,14 @@ fn handle( streams: &mut IoStreams, optind: &mut usize, args: &[&wstr], - ) -> Option { + ) -> Result<(), ErrorCode> { let cmd = args[0]; let replacer = match StringReplacer::new(self.pattern, self.replacement, self) { Ok(x) => x, Err(e) => { e.print_error(args, streams); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } }; @@ -108,7 +108,7 @@ fn handle( cmd, e.error_message() ); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } }; replace_count += replaced as usize; @@ -121,20 +121,20 @@ fn handle( } if self.quiet && replace_count > 0 { - return STATUS_CMD_OK; + return Ok(()); } if self .max_matches .is_some_and(|max| max.get() == replace_count) { - return STATUS_CMD_OK; + return Ok(()); } } if replace_count > 0 { - STATUS_CMD_OK + Ok(()) } else { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) } } } diff --git a/src/builtins/string/shorten.rs b/src/builtins/string/shorten.rs index 7455da3a6..714426989 100644 --- a/src/builtins/string/shorten.rs +++ b/src/builtins/string/shorten.rs @@ -66,7 +66,7 @@ fn handle( streams: &mut IoStreams, optind: &mut usize, args: &[&wstr], - ) -> Option { + ) -> Result<(), ErrorCode> { let mut min_width = usize::MAX; let mut inputs = Vec::new(); @@ -84,7 +84,7 @@ fn handle( for (arg, _) in iter { streams.out.appendln(arg); } - return STATUS_CMD_OK; + return Ok(()); } for (arg, _) in iter { @@ -168,7 +168,7 @@ fn handle( pos += skip_escapes(&line, pos).max(1); } if self.quiet && pos != 0 { - return STATUS_CMD_OK; + return Ok(()); } let output = match pos { @@ -217,7 +217,7 @@ fn handle( } if self.quiet && pos != line.len() { - return STATUS_CMD_OK; + return Ok(()); } if pos == line.len() { @@ -235,9 +235,9 @@ fn handle( // Return true if we have shortened something and false otherwise. if nsub > 0 { - STATUS_CMD_OK + Ok(()) } else { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) } } } diff --git a/src/builtins/string/split.rs b/src/builtins/string/split.rs index 0b1218f72..d9989a61c 100644 --- a/src/builtins/string/split.rs +++ b/src/builtins/string/split.rs @@ -145,17 +145,17 @@ fn take_args( optind: &mut usize, args: &[&'args wstr], streams: &mut IoStreams, - ) -> Option { + ) -> Result<(), ErrorCode> { if self.is_split0 { - return STATUS_CMD_OK; + return Ok(()); } let Some(arg) = args.get(*optind).copied() else { string_error!(streams, BUILTIN_ERR_ARG_COUNT0, args[0]); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); }; *optind += 1; self.sep = arg; - return STATUS_CMD_OK; + return Ok(()); } fn handle( @@ -164,14 +164,14 @@ fn handle( streams: &mut IoStreams, optind: &mut usize, args: &[&'args wstr], - ) -> Option { + ) -> Result<(), ErrorCode> { if self.fields.is_empty() && self.allow_empty { streams.err.append(wgettext_fmt!( BUILTIN_ERR_COMBO2, args[0], wgettext!("--allow-empty is only valid with --fields") )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } let sep = self.sep; @@ -216,7 +216,7 @@ fn handle( // If we're quiet, we return early if we've found something to split. if self.quiet && splits.len() > 1 { - return STATUS_CMD_OK; + return Ok(()); } split_count += splits.len(); arg_count += 1; @@ -225,9 +225,9 @@ fn handle( if self.quiet { return if split_count > arg_count { - STATUS_CMD_OK + Ok(()) } else { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) }; } @@ -251,7 +251,7 @@ fn handle( for field in self.fields.iter() { // we already have checked the start if *field >= splits.len() { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } } } @@ -273,9 +273,9 @@ fn handle( // We split something if we have more split values than args. return if split_count > arg_count { - STATUS_CMD_OK + Ok(()) } else { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) }; } } diff --git a/src/builtins/string/sub.rs b/src/builtins/string/sub.rs index 14d1d3630..b945de075 100644 --- a/src/builtins/string/sub.rs +++ b/src/builtins/string/sub.rs @@ -52,7 +52,7 @@ fn handle( streams: &mut IoStreams, optind: &mut usize, args: &[&wstr], - ) -> Option { + ) -> Result<(), ErrorCode> { let cmd = args[0]; if self.length.is_some() && self.end.is_some() { streams.err.append(wgettext_fmt!( @@ -60,7 +60,7 @@ fn handle( cmd, wgettext!("--end and --length are mutually exclusive") )); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } let mut nsub = 0; @@ -101,14 +101,14 @@ fn handle( } nsub += 1; if self.quiet { - return STATUS_CMD_OK; + return Ok(()); } } if nsub > 0 { - STATUS_CMD_OK + Ok(()) } else { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) } } } diff --git a/src/builtins/string/transform.rs b/src/builtins/string/transform.rs index cc2e8de54..d7ec8b2f2 100644 --- a/src/builtins/string/transform.rs +++ b/src/builtins/string/transform.rs @@ -22,7 +22,7 @@ fn handle( streams: &mut IoStreams, optind: &mut usize, args: &[&wstr], - ) -> Option { + ) -> Result<(), ErrorCode> { let mut n_transformed = 0usize; for (arg, want_newline) in arguments(args, optind, streams) { @@ -36,14 +36,14 @@ fn handle( streams.out.append1('\n'); } } else if n_transformed > 0 { - return STATUS_CMD_OK; + return Ok(()); } } if n_transformed > 0 { - STATUS_CMD_OK + Ok(()) } else { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) } } } diff --git a/src/builtins/string/trim.rs b/src/builtins/string/trim.rs index 14ec11281..dddcaeb73 100644 --- a/src/builtins/string/trim.rs +++ b/src/builtins/string/trim.rs @@ -50,7 +50,7 @@ fn handle( streams: &mut IoStreams, optind: &mut usize, args: &[&wstr], - ) -> Option { + ) -> Result<(), ErrorCode> { // If neither left or right is specified, we do both. if !self.left && !self.right { self.left = true; @@ -86,14 +86,14 @@ fn handle( streams.out.append1('\n'); } } else if ntrim > 0 { - return STATUS_CMD_OK; + return Ok(()); } } if ntrim > 0 { - STATUS_CMD_OK + Ok(()) } else { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) } } } diff --git a/src/builtins/string/unescape.rs b/src/builtins/string/unescape.rs index 6e38c3c82..1a31d72e9 100644 --- a/src/builtins/string/unescape.rs +++ b/src/builtins/string/unescape.rs @@ -36,7 +36,7 @@ fn handle( streams: &mut IoStreams, optind: &mut usize, args: &[&wstr], - ) -> Option { + ) -> Result<(), ErrorCode> { let mut nesc = 0; for (arg, want_newline) in arguments(args, optind, streams) { if let Some(res) = unescape_string(&arg, self.style) { @@ -49,9 +49,9 @@ fn handle( } if nesc > 0 { - STATUS_CMD_OK + Ok(()) } else { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) } } } diff --git a/src/builtins/test.rs b/src/builtins/test.rs index 31102253b..b005811a7 100644 --- a/src/builtins/test.rs +++ b/src/builtins/test.rs @@ -985,10 +985,10 @@ fn unary_primary_evaluate( /// Evaluate a conditional expression given the arguments. For POSIX conformance this /// supports a more limited range of functionality. /// Return status is the final shell status, i.e. 0 for true, 1 for false and 2 for error. -pub fn test(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Option { +pub fn test(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> BuiltinResult { // The first argument should be the name of the command ('test'). if argv.is_empty() { - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } // Whether we are invoked with bracket '[' or not. @@ -1008,7 +1008,7 @@ pub fn test(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt .err .appendln(wgettext!("[: the last argument must be ']'")); builtin_print_error_trailer(parser, streams.err, program_name); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } } @@ -1026,12 +1026,12 @@ pub fn test(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt program_name )); builtin_print_error_trailer(parser, streams.err, program_name); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } else if argc == 1 { if args[0] == "-n" { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); } else if args[0] == "-z" { - return STATUS_CMD_OK; + return Ok(SUCCESS); } } } else if argc == 0 { @@ -1042,7 +1042,7 @@ pub fn test(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt )); streams.err.append(parser.current_line()); } - return STATUS_INVALID_ARGS; // Per 1003.1, exit false. + return Err(STATUS_INVALID_ARGS); // Per 1003.1, exit false. } else if argc == 1 { if should_flog!(deprecated_test) { if args[0] != "-z" { @@ -1055,9 +1055,9 @@ pub fn test(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt } // Per 1003.1, exit true if the arg is non-empty. return if args[0].is_empty() { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) } else { - STATUS_CMD_OK + Ok(SUCCESS) }; } @@ -1067,7 +1067,7 @@ pub fn test(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt let Some(expr) = expr else { streams.err.append(err); streams.err.append(parser.current_line()); - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); }; let mut eval_errors = Vec::new(); @@ -1081,12 +1081,12 @@ pub fn test(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt // because this isn't about passing the wrong options. streams.err.append(parser.current_line()); } - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } if result { - STATUS_CMD_OK + Ok(SUCCESS) } else { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) } } diff --git a/src/builtins/tests/string_tests.rs b/src/builtins/tests/string_tests.rs index 828abe85d..34ed52c2d 100644 --- a/src/builtins/tests/string_tests.rs +++ b/src/builtins/tests/string_tests.rs @@ -6,7 +6,9 @@ #[serial] fn test_string() { let _cleanup = test_init(); - use crate::builtins::shared::{STATUS_CMD_ERROR, STATUS_CMD_OK, STATUS_INVALID_ARGS}; + use crate::builtins::shared::{ + BuiltinResultExt, STATUS_CMD_ERROR, STATUS_CMD_OK, STATUS_INVALID_ARGS, + }; use crate::builtins::string::string; use crate::common::escape; use crate::future_feature_flags::{scoped_test, FeatureFlag}; @@ -21,7 +23,7 @@ macro_rules! test_cases { } // TODO: these should be individual tests, not all in one, port when we can run these with `cargo test` - fn string_test(mut args: Vec<&wstr>, expected_rc: Option, expected_out: &wstr) { + fn string_test(mut args: Vec<&wstr>, expected_rc: i32, expected_out: &wstr) { let parser = TestParser::new(); let mut outs = OutputStream::String(StringOutputStream::new()); let mut errs = OutputStream::Null; @@ -29,7 +31,7 @@ fn string_test(mut args: Vec<&wstr>, expected_rc: Option, expected_out: &ws let mut streams = IoStreams::new(&mut outs, &mut errs, &io_chain); streams.stdin_is_directly_redirected = false; // read from argv instead of stdin - let rc = string(&parser, &mut streams, args.as_mut_slice()).expect("string failed"); + let rc = string(&parser, &mut streams, args.as_mut_slice()); let actual = escape(outs.contents()); let expected = escape(expected_out); @@ -40,8 +42,8 @@ fn string_test(mut args: Vec<&wstr>, expected_rc: Option, expected_out: &ws // Check return code after so we get a chance to identify the difference first assert_eq!( - expected_rc.unwrap(), - rc, + expected_rc, + rc.builtin_status_code(), "string builtin returned unexpected return code" ); } diff --git a/src/builtins/tests/test_tests.rs b/src/builtins/tests/test_tests.rs index 8346a50e6..3fdefae04 100644 --- a/src/builtins/tests/test_tests.rs +++ b/src/builtins/tests/test_tests.rs @@ -25,19 +25,15 @@ fn run_one_test_test_mbracket(expected: i32, lst: &[&str], bracket: bool) -> boo let io_chain = IoChain::new(); let mut streams = IoStreams::new(&mut out, &mut err, &io_chain); - let result: Option = builtin_test(&parser, &mut streams, &mut argv); + let result = builtin_test(&parser, &mut streams, &mut argv).builtin_status_code(); - if result != Some(expected) { - let got = match result { - Some(r) => r.to_wstring(), - None => L!("nothing").to_owned(), - }; + if result != expected { eprintln!( "expected builtin_test() to return {}, got {}", - expected, got + expected, result ); } - result == Some(expected) + result == expected } fn run_test_test(expected: i32, lst: &[&str]) -> bool { @@ -59,16 +55,16 @@ fn test_test_brackets() { let args1 = &mut [L!("["), L!("foo")]; assert_eq!( builtin_test(&parser, &mut streams, args1), - STATUS_INVALID_ARGS + Err(STATUS_INVALID_ARGS) ); let args2 = &mut [L!("["), L!("foo"), L!("]")]; - assert_eq!(builtin_test(&parser, &mut streams, args2), STATUS_CMD_OK); + assert_eq!(builtin_test(&parser, &mut streams, args2), Ok(SUCCESS)); let args3 = &mut [L!("["), L!("foo"), L!("]"), L!("bar")]; assert_eq!( builtin_test(&parser, &mut streams, args3), - STATUS_INVALID_ARGS + Err(STATUS_INVALID_ARGS) ); } diff --git a/src/builtins/type.rs b/src/builtins/type.rs index 50e764d20..1c44626e1 100644 --- a/src/builtins/type.rs +++ b/src/builtins/type.rs @@ -17,7 +17,7 @@ struct type_cmd_opts_t { query: bool, } -pub fn r#type(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Option { +pub fn r#type(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> BuiltinResult { let cmd = argv[0]; let argc = argv.len(); let print_hints = false; @@ -48,15 +48,15 @@ pub fn r#type(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> O 'q' => opts.query = true, 'h' => { builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } ':' => { builtin_missing_argument(parser, streams, cmd, argv[w.wopt_index - 1], print_hints); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } '?' => { builtin_unknown_option(parser, streams, cmd, argv[w.wopt_index - 1], print_hints); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } _ => { panic!("unexpected retval from wgeopter.next()"); @@ -66,7 +66,7 @@ pub fn r#type(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> O if opts.query as i64 + opts.path as i64 + opts.get_type as i64 + opts.force_path as i64 > 1 { streams.err.append(wgettext_fmt!(BUILTIN_ERR_COMBO, cmd)); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } let mut res = false; @@ -80,7 +80,7 @@ pub fn r#type(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> O res = true; // Early out - query means *any of the args exists*. if opts.query { - return STATUS_CMD_OK; + return Ok(SUCCESS); } if !opts.get_type { let path = props.definition_file().unwrap_or(L!("")); @@ -164,7 +164,7 @@ pub fn r#type(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> O found += 1; res = true; if opts.query { - return STATUS_CMD_OK; + return Ok(SUCCESS); } if !opts.get_type { streams.out.append(wgettext_fmt!("%ls is a builtin\n", arg)); @@ -189,7 +189,7 @@ pub fn r#type(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> O found += 1; res = true; if opts.query { - return STATUS_CMD_OK; + return Ok(SUCCESS); } if !opts.get_type { if opts.path || opts.force_path { @@ -220,8 +220,8 @@ pub fn r#type(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> O } if res { - STATUS_CMD_OK + Ok(SUCCESS) } else { - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) } } diff --git a/src/builtins/ulimit.rs b/src/builtins/ulimit.rs index a1d322523..c800eee4c 100644 --- a/src/builtins/ulimit.rs +++ b/src/builtins/ulimit.rs @@ -101,9 +101,9 @@ fn set_limit( soft: bool, value: rlim_t, streams: &mut IoStreams, -) -> Option { +) -> BuiltinResult { let Some((mut rlim_cur, mut rlim_max)) = getrlimit(resource) else { - return STATUS_CMD_ERROR; + return Err(STATUS_CMD_ERROR); }; if hard { rlim_max = value; @@ -127,9 +127,9 @@ fn set_limit( builtin_wperror(L!("ulimit"), streams); } - STATUS_CMD_ERROR + Err(STATUS_CMD_ERROR) } else { - STATUS_CMD_OK + Ok(SUCCESS) } } @@ -168,7 +168,7 @@ fn default() -> Self { } } -pub fn ulimit(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option { +pub fn ulimit(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> BuiltinResult { let cmd = args[0]; const SHORT_OPTS: &wstr = L!(":HSabcdefilmnqrstuvwyKPTh"); @@ -231,15 +231,15 @@ pub fn ulimit(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O 'T' => opts.what = RLIMIT_NTHR(), 'h' => { builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } ':' => { builtin_missing_argument(parser, streams, cmd, w.argv[w.wopt_index - 1], true); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } '?' => { builtin_unknown_option(parser, streams, cmd, w.argv[w.wopt_index - 1], true); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } _ => { panic!("unexpected retval from WGetopter"); @@ -258,7 +258,7 @@ pub fn ulimit(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O )); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } let what: c_uint = opts.what.try_into().unwrap(); @@ -267,14 +267,14 @@ pub fn ulimit(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O let arg_count = argc - w.wopt_index; if arg_count == 0 { print(what, opts.hard, streams); - return STATUS_CMD_OK; + return Ok(SUCCESS); } else if arg_count != 1 { streams .err .append(wgettext_fmt!(BUILTIN_ERR_TOO_MANY_ARGUMENTS, cmd)); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } let mut hard = opts.hard; @@ -291,18 +291,18 @@ pub fn ulimit(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O cmd )); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } else if wcscasecmp(w.argv[w.wopt_index], L!("unlimited")) == Ordering::Equal { RLIM_INFINITY } else if wcscasecmp(w.argv[w.wopt_index], L!("hard")) == Ordering::Equal { match get(what, true) { Some(limit) => limit, - None => return STATUS_CMD_ERROR, + None => return Err(STATUS_CMD_ERROR), } } else if wcscasecmp(w.argv[w.wopt_index], L!("soft")) == Ordering::Equal { match get(what, soft) { Some(limit) => limit, - None => return STATUS_CMD_ERROR, + None => return Err(STATUS_CMD_ERROR), } } else if let Ok(limit) = fish_wcstol(w.argv[w.wopt_index]) { let Some(x) = get_multiplier(what).checked_mul(limit as rlim_t) else { @@ -312,7 +312,7 @@ pub fn ulimit(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O w.argv[w.wopt_index] )); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); }; x } else { @@ -322,7 +322,7 @@ pub fn ulimit(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O w.argv[w.wopt_index] )); builtin_print_error_trailer(parser, streams.err, cmd); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); }; set_limit(what, hard, soft, new_limit, streams) diff --git a/src/builtins/wait.rs b/src/builtins/wait.rs index 7571aa33d..0b07c746e 100644 --- a/src/builtins/wait.rs +++ b/src/builtins/wait.rs @@ -101,9 +101,9 @@ fn is_completed(wh: &WaitHandleRef) -> bool { /// Wait for the given wait handles to be marked as completed. /// If `any_flag` is set, wait for the first one; otherwise wait for all. /// Return a status code. -fn wait_for_completion(parser: &Parser, whs: &[WaitHandleRef], any_flag: bool) -> Option { +fn wait_for_completion(parser: &Parser, whs: &[WaitHandleRef], any_flag: bool) -> BuiltinResult { if whs.is_empty() { - return Some(0); + return Ok(SUCCESS); } let mut sigint = SigChecker::new_sighupint(); @@ -124,16 +124,16 @@ fn wait_for_completion(parser: &Parser, whs: &[WaitHandleRef], any_flag: bool) - } } } - return Some(0); + return Ok(SUCCESS); } if sigint.check() { - return Some(128 + libc::SIGINT); + return Err(128 + libc::SIGINT); } proc_wait_any(parser); } } -pub fn wait(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Option { +pub fn wait(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> BuiltinResult { let cmd = argv[0]; let argc = argv.len(); let mut any_flag = false; // flag for -n option @@ -157,11 +157,11 @@ pub fn wait(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt } ':' => { builtin_missing_argument(parser, streams, cmd, argv[w.wopt_index - 1], print_hints); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } '?' => { builtin_unknown_option(parser, streams, cmd, argv[w.wopt_index - 1], print_hints); - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } _ => { panic!("unexpected retval from wgeopter.next()"); @@ -171,7 +171,7 @@ pub fn wait(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt if print_help { builtin_print_help(parser, streams, cmd); - return STATUS_CMD_OK; + return Ok(SUCCESS); } if w.wopt_index == argc { @@ -214,7 +214,7 @@ pub fn wait(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt } } if wait_handles.is_empty() { - return STATUS_INVALID_ARGS; + return Err(STATUS_INVALID_ARGS); } return wait_for_completion(parser, &wait_handles, any_flag); } diff --git a/src/complete.rs b/src/complete.rs index 87ae3b3b0..ef8cbc044 100644 --- a/src/complete.rs +++ b/src/complete.rs @@ -907,7 +907,8 @@ fn condition_test(&mut self, condition: &wstr) -> bool { // Compute new value and reinsert it. let test_res = exec_subshell( condition, parser, None, false, /* don't apply exit status */ - ) == 0; + ) + .is_ok(); self.condition_cache.insert(condition.to_owned(), test_res); test_res } @@ -1025,7 +1026,7 @@ fn complete_cmd_desc(&mut self, s: &wstr) { // search if we know the location of the whatis database. This can take some time on slower // systems with a large set of manuals, but it should be ok since apropos is only called once. let mut list = vec![]; - exec_subshell( + let _ = exec_subshell( &lookup_cmd, parser, Some(&mut list), diff --git a/src/env/environment.rs b/src/env/environment.rs index e4722029b..d6f366984 100644 --- a/src/env/environment.rs +++ b/src/env/environment.rs @@ -4,6 +4,7 @@ }; use super::{ConfigPaths, ElectricVar}; use crate::abbrs::{abbrs_get_set, Abbreviation, Position}; +use crate::builtins::shared::{BuiltinResult, SUCCESS}; use crate::common::{str2wcstring, unescape_string, wcs2zstring, UnescapeStringStyle}; use crate::env::{EnvMode, EnvVar, Statuses}; use crate::env_dispatch::{env_dispatch_init, env_dispatch_var_change}; @@ -55,14 +56,14 @@ fn default() -> Self { } } -impl From for c_int { +impl From for BuiltinResult { fn from(r: EnvStackSetResult) -> Self { match r { - EnvStackSetResult::Ok => 0, - EnvStackSetResult::Perm => 1, - EnvStackSetResult::Scope => 2, - EnvStackSetResult::Invalid => 3, - EnvStackSetResult::NotFound => 4, + EnvStackSetResult::Ok => Ok(SUCCESS), + EnvStackSetResult::Perm => Err(1), + EnvStackSetResult::Scope => Err(2), + EnvStackSetResult::Invalid => Err(3), + EnvStackSetResult::NotFound => Err(4), } } } diff --git a/src/exec.rs b/src/exec.rs index bebd7dc8c..6afffc16c 100644 --- a/src/exec.rs +++ b/src/exec.rs @@ -4,7 +4,7 @@ // performed have been massive. use crate::builtins::shared::{ - builtin_run, STATUS_CMD_ERROR, STATUS_CMD_OK, STATUS_CMD_UNKNOWN, STATUS_NOT_EXECUTABLE, + builtin_run, ErrorCode, STATUS_CMD_ERROR, STATUS_CMD_UNKNOWN, STATUS_NOT_EXECUTABLE, STATUS_READ_TOO_MUCH, }; use crate::common::{ @@ -254,7 +254,7 @@ pub fn exec_subshell( parser: &Parser, outputs: Option<&mut Vec>, apply_exit_status: bool, -) -> libc::c_int { +) -> Result<(), ErrorCode> { let mut break_expand = false; exec_subshell_internal( cmd, @@ -276,7 +276,7 @@ pub fn exec_subshell_for_expand( parser: &Parser, job_group: Option<&JobGroupRef>, outputs: &mut Vec, -) -> libc::c_int { +) -> Result<(), ErrorCode> { parser.assert_can_execute(); let mut break_expand = true; let ret = exec_subshell_internal( @@ -292,7 +292,7 @@ pub fn exec_subshell_for_expand( if break_expand { ret } else { - STATUS_CMD_OK.unwrap() + Ok(()) } } @@ -311,16 +311,16 @@ fn exit_code_from_exec_error(err: libc::c_int) -> libc::c_int { ENOENT | ENOTDIR => { // This indicates either the command was not found, or a file redirection was not found. // We do not use posix_spawn file redirections so this is always command-not-found. - STATUS_CMD_UNKNOWN.unwrap() + STATUS_CMD_UNKNOWN } EACCES | ENOEXEC => { // The file is not executable for various reasons. - STATUS_NOT_EXECUTABLE.unwrap() + STATUS_NOT_EXECUTABLE } #[cfg(target_os = "macos")] libc::EBADARCH => { // This is for e.g. running ARM app on Intel Mac. - STATUS_NOT_EXECUTABLE.unwrap() + STATUS_NOT_EXECUTABLE } _ => { // Generic failure. @@ -1448,7 +1448,7 @@ fn exec_subshell_internal( break_expand: &mut bool, apply_exit_status: bool, is_subcmd: bool, -) -> libc::c_int { +) -> Result<(), ErrorCode> { parser.assert_can_execute(); let _is_subshell = scoped_push_replacer( |new_value| std::mem::replace(&mut parser.libdata_mut().is_subshell, new_value), @@ -1477,7 +1477,7 @@ fn exec_subshell_internal( let Ok(bufferfill) = IoBufferfill::create_opts(parser.libdata().read_limit, STDOUT_FILENO) else { *break_expand = true; - return STATUS_CMD_ERROR.unwrap(); + return Err(STATUS_CMD_ERROR); }; let mut io_chain = IoChain::new(); @@ -1486,17 +1486,23 @@ fn exec_subshell_internal( let buffer = IoBufferfill::finish(bufferfill); if buffer.discarded() { *break_expand = true; - return STATUS_READ_TOO_MUCH.unwrap(); + return Err(STATUS_READ_TOO_MUCH); } if eval_res.break_expand { *break_expand = true; - return eval_res.status.status_value(); + match eval_res.status.status_value() { + 0 => return Ok(()), + code => return Err(code), + } } if let Some(lst) = lst { populate_subshell_output(lst, &buffer, split_output); } *break_expand = false; - eval_res.status.status_value() + match eval_res.status.status_value() { + 0 => Ok(()), + code => Err(code), + } } diff --git a/src/expand.rs b/src/expand.rs index 9e912dfc1..da7a3657d 100644 --- a/src/expand.rs +++ b/src/expand.rs @@ -411,7 +411,7 @@ fn append_overflow_error( error.text = wgettext!("Expansion produced too many results").to_owned(); errors.push(error); } - ExpandResult::make_error(STATUS_EXPAND_ERROR.unwrap()) + ExpandResult::make_error(STATUS_EXPAND_ERROR) } /// Test if the specified string does not contain character which can not be used inside a quoted @@ -630,7 +630,7 @@ fn expand_variables( errors, ); } - return ExpandResult::make_error(STATUS_EXPAND_ERROR.unwrap()); + return ExpandResult::make_error(STATUS_EXPAND_ERROR); } // Do a dirty hack to make sliced history fast (#4650). We expand from either a variable, or a @@ -682,7 +682,7 @@ fn expand_variables( append_syntax_error!(errors, slice_start + bad_pos, "Invalid index value"); } } - return ExpandResult::make_error(STATUS_EXPAND_ERROR.unwrap()); + return ExpandResult::make_error(STATUS_EXPAND_ERROR); } } } @@ -860,7 +860,7 @@ fn expand_braces( if syntax_error { append_syntax_error!(errors, SOURCE_LOCATION_UNKNOWN, "Mismatched braces"); - return ExpandResult::make_error(STATUS_EXPAND_ERROR.unwrap()); + return ExpandResult::make_error(STATUS_EXPAND_ERROR); } let Some(brace_begin) = brace_begin else { @@ -938,7 +938,7 @@ pub fn expand_cmdsubst( ) { MaybeParentheses::Error => { append_syntax_error!(errors, SOURCE_LOCATION_UNKNOWN, "Mismatched parenthesis"); - return ExpandResult::make_error(STATUS_EXPAND_ERROR.unwrap()); + return ExpandResult::make_error(STATUS_EXPAND_ERROR); } MaybeParentheses::None => { if !out.add(input) { @@ -957,31 +957,32 @@ pub fn expand_cmdsubst( job_group.as_ref(), &mut sub_res, ); - if subshell_status != 0 { + + if let Err(subshell_status) = subshell_status { // TODO: Ad-hoc switch, how can we enumerate the possible errors more safely? let err = match subshell_status { - _ if subshell_status == STATUS_READ_TOO_MUCH.unwrap() => { + _ if subshell_status == STATUS_READ_TOO_MUCH => { wgettext!("Too much data emitted by command substitution so it was discarded") } // TODO: STATUS_CMD_ERROR is overused and too generic. We shouldn't have to test things // to figure out what error to show after we've already been given an error code. - _ if subshell_status == STATUS_CMD_ERROR.unwrap() => { + _ if subshell_status == STATUS_CMD_ERROR => { if ctx.parser().is_eval_depth_exceeded() { wgettext!("Unable to evaluate string substitution") } else { wgettext!("Too many active file descriptors") } } - _ if subshell_status == STATUS_CMD_UNKNOWN.unwrap() => { + _ if subshell_status == STATUS_CMD_UNKNOWN => { wgettext!("Unknown command") } - _ if subshell_status == STATUS_ILLEGAL_CMD.unwrap() => { + _ if subshell_status == STATUS_ILLEGAL_CMD => { wgettext!("Commandname was invalid") } - _ if subshell_status == STATUS_NOT_EXECUTABLE.unwrap() => { + _ if subshell_status == STATUS_NOT_EXECUTABLE => { wgettext!("Command not executable") } - _ if subshell_status == STATUS_INVALID_ARGS.unwrap() => { + _ if subshell_status == STATUS_INVALID_ARGS => { // TODO: Also overused // This is sent for: // invalid redirections or pipes (like `<&foo`), @@ -990,11 +991,11 @@ pub fn expand_cmdsubst( // time in a background job. wgettext!("Invalid arguments") } - _ if subshell_status == STATUS_EXPAND_ERROR.unwrap() => { + _ if subshell_status == STATUS_EXPAND_ERROR => { // Sent in `for $foo in ...` if $foo expands to more than one word wgettext!("Expansion error") } - _ if subshell_status == STATUS_UNMATCHED_WILDCARD.unwrap() => { + _ if subshell_status == STATUS_UNMATCHED_WILDCARD => { // Sent in `for $foo in ...` if $foo expands to more than one word wgettext!("Unmatched wildcard") } @@ -1026,7 +1027,7 @@ pub fn expand_cmdsubst( append_syntax_error!(errors, slice_begin + bad_pos, "Invalid index value"); } } - return ExpandResult::make_error(STATUS_EXPAND_ERROR.unwrap()); + return ExpandResult::make_error(STATUS_EXPAND_ERROR); } }; @@ -1329,7 +1330,7 @@ fn stage_cmdsubst(&mut self, input: WString, out: &mut CompletionReceiver) -> Ex let mut cursor = 0; match parse_util_locate_cmdsubst_range(&input, &mut cursor, true, None, None) { MaybeParentheses::Error => { - return ExpandResult::make_error(STATUS_EXPAND_ERROR.unwrap()); + return ExpandResult::make_error(STATUS_EXPAND_ERROR); } MaybeParentheses::None => { if !out.add(input) { @@ -1344,7 +1345,7 @@ fn stage_cmdsubst(&mut self, input: WString, out: &mut CompletionReceiver) -> Ex parens.end()-1, "command substitutions not allowed in command position. Try var=(your-cmd) $var ..." ); - return ExpandResult::make_error(STATUS_EXPAND_ERROR.unwrap()); + return ExpandResult::make_error(STATUS_EXPAND_ERROR); } } } else { diff --git a/src/io.rs b/src/io.rs index d68da89dc..8acc47f8c 100644 --- a/src/io.rs +++ b/src/io.rs @@ -668,7 +668,7 @@ pub fn flush_and_check_error(&mut self) -> libc::c_int { match self { OutputStream::Fd(stream) => stream.flush_and_check_error(), OutputStream::Buffered(stream) => stream.flush_and_check_error(), - OutputStream::Null | OutputStream::String(_) => STATUS_CMD_OK.unwrap(), + OutputStream::Null | OutputStream::String(_) => STATUS_CMD_OK, } } @@ -794,7 +794,6 @@ fn flush_and_check_error(&mut self) -> libc::c_int { } else { STATUS_CMD_OK } - .unwrap() } } @@ -839,7 +838,7 @@ fn append_with_separation( } fn flush_and_check_error(&mut self) -> libc::c_int { if self.buffer.discarded() { - return STATUS_READ_TOO_MUCH.unwrap(); + return STATUS_READ_TOO_MUCH; } 0 } diff --git a/src/parse_execution.rs b/src/parse_execution.rs index d588d92bf..2792a361e 100644 --- a/src/parse_execution.rs +++ b/src/parse_execution.rs @@ -192,7 +192,7 @@ fn eval_job_list( return report_error!( self, ctx, - STATUS_CMD_ERROR.unwrap(), + STATUS_CMD_ERROR, infinite_recursive_node, INFINITE_FUNC_RECURSION_ERR_MSG, func_name @@ -208,7 +208,7 @@ fn eval_job_list( return report_error!( self, ctx, - STATUS_CMD_ERROR.unwrap(), + STATUS_CMD_ERROR, job_list, CALL_STACK_LIMIT_EXCEEDED_ERR_MSG ); @@ -287,7 +287,7 @@ fn handle_command_not_found( return report_error!( self, ctx, - STATUS_NOT_EXECUTABLE.unwrap(), + STATUS_NOT_EXECUTABLE, &statement.command, concat!( "Unknown command. A component of '%ls' is not a ", @@ -299,7 +299,7 @@ fn handle_command_not_found( return report_error!( self, ctx, - STATUS_NOT_EXECUTABLE.unwrap(), + STATUS_NOT_EXECUTABLE, &statement.command, "Unknown command. A component of '%ls' is not a directory.", cmd @@ -310,7 +310,7 @@ fn handle_command_not_found( return report_error!( self, ctx, - STATUS_NOT_EXECUTABLE.unwrap(), + STATUS_NOT_EXECUTABLE, &statement.command, "Unknown command. '%ls' exists but is not an executable file.", cmd @@ -368,13 +368,7 @@ fn handle_command_not_found( // Here we want to report an error (so it shows a backtrace). // If the handler printed text, that's already shown, so error will be empty. - report_error_formatted!( - self, - ctx, - STATUS_CMD_UNKNOWN.unwrap(), - &statement.command, - error - ) + report_error_formatted!(self, ctx, STATUS_CMD_UNKNOWN, &statement.command, error) } // Utilities. @@ -504,13 +498,13 @@ fn expand_command( // This means that the error positions are relative to the beginning // of the token; we need to make them relative to the original source. parse_error_offset_source_start(&mut errors, pos_of_command_token); - return self.report_errors(ctx, STATUS_ILLEGAL_CMD.unwrap(), &errors); + return self.report_errors(ctx, STATUS_ILLEGAL_CMD, &errors); } ExpandResultCode::wildcard_no_match => { return report_error!( self, ctx, - STATUS_UNMATCHED_WILDCARD.unwrap(), + STATUS_UNMATCHED_WILDCARD, statement, WILDCARD_ERR_MSG, &self.node_source(statement) @@ -528,7 +522,7 @@ fn expand_command( return report_error!( self, ctx, - STATUS_ILLEGAL_CMD.unwrap(), + STATUS_ILLEGAL_CMD, &statement.command, "The expanded command was empty." ); @@ -545,7 +539,7 @@ fn expand_command( return report_error!( self, ctx, - STATUS_ILLEGAL_CMD.unwrap(), + STATUS_ILLEGAL_CMD, &statement.command, "The expanded command is a keyword." ); @@ -907,7 +901,7 @@ fn run_for_statement( return report_error!( self, ctx, - STATUS_EXPAND_ERROR.unwrap(), + STATUS_EXPAND_ERROR, &header.var_name, FAILED_EXPANSION_VARIABLE_NAME_ERR_MSG, for_var_name @@ -918,7 +912,7 @@ fn run_for_statement( return report_error!( self, ctx, - STATUS_INVALID_ARGS.unwrap(), + STATUS_INVALID_ARGS, header.var_name, BUILTIN_ERR_VARNAME, "for", @@ -939,7 +933,7 @@ fn run_for_statement( return report_error!( self, ctx, - STATUS_INVALID_ARGS.unwrap(), + STATUS_INVALID_ARGS, header.var_name, "%ls: %ls: cannot overwrite read-only variable", "for", @@ -1065,7 +1059,7 @@ fn run_if_statement( // 'if' condition failed, no else clause, return 0, we're done. // No job list means no successful conditions, so return 0 (issue #1443). ctx.parser() - .set_last_statuses(Statuses::just(STATUS_CMD_OK.unwrap())); + .set_last_statuses(Statuses::just(STATUS_CMD_OK)); } Some(job_list_to_execute) => { // Execute the job list we got. @@ -1119,7 +1113,7 @@ fn run_switch_statement( return report_error!( self, ctx, - STATUS_UNMATCHED_WILDCARD.unwrap(), + STATUS_UNMATCHED_WILDCARD, &statement.argument, WILDCARD_ERR_MSG, &self.node_source(&statement.argument) @@ -1130,7 +1124,7 @@ fn run_switch_statement( return report_error!( self, ctx, - STATUS_INVALID_ARGS.unwrap(), + STATUS_INVALID_ARGS, &statement.argument, "switch: Expected at most one argument, got %lu\n", switch_values_expanded.len() @@ -1321,7 +1315,7 @@ fn run_function_statement( statement as *const ast::BlockStatement, ), ); - let err_code = err_code.unwrap(); + ctx.parser().libdata_mut().status_count += 1; ctx.parser().set_last_statuses(Statuses::just(err_code)); @@ -1408,7 +1402,7 @@ fn expand_arguments_from_nodes( return report_error!( self, ctx, - STATUS_UNMATCHED_WILDCARD.unwrap(), + STATUS_UNMATCHED_WILDCARD, arg_node, WILDCARD_ERR_MSG, &self.node_source(*arg_node) @@ -1458,7 +1452,7 @@ fn determine_redirections( return report_error!( self, ctx, - STATUS_INVALID_ARGS.unwrap(), + STATUS_INVALID_ARGS, redir_node, "Invalid redirection: %ls", &self.node_source(redir_node) @@ -1483,7 +1477,7 @@ fn determine_redirections( return report_error!( self, ctx, - STATUS_INVALID_ARGS.unwrap(), + STATUS_INVALID_ARGS, redir_node, "Invalid redirection target: %ls", target @@ -1502,7 +1496,7 @@ fn determine_redirections( return report_error!( self, ctx, - STATUS_INVALID_ARGS.unwrap(), + STATUS_INVALID_ARGS, redir_node, "Requested redirection to '%ls', which is not a valid file descriptor", &spec.target @@ -1563,7 +1557,7 @@ fn run_1_job( return report_error!( self, ctx, - STATUS_INVALID_ARGS.unwrap(), + STATUS_INVALID_ARGS, job_node, ERROR_TIME_BACKGROUND ); @@ -1832,7 +1826,7 @@ fn populate_job_from_job_node( result = report_error!( self, ctx, - STATUS_INVALID_ARGS.unwrap(), + STATUS_INVALID_ARGS, &jc.pipe, ILLEGAL_FD_ERR_MSG, &self.node_source(&jc.pipe) diff --git a/src/parser.rs b/src/parser.rs index e6a2b4a83..8a0f40fd0 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -505,10 +505,10 @@ pub fn eval_with( eprintf!("%s\n", backtrace_and_desc); // Set a valid status. - self.set_last_statuses(Statuses::just(STATUS_ILLEGAL_CMD.unwrap())); + self.set_last_statuses(Statuses::just(STATUS_ILLEGAL_CMD)); let break_expand = true; EvalRes { - status: ProcStatus::from_exit_code(STATUS_ILLEGAL_CMD.unwrap()), + status: ProcStatus::from_exit_code(STATUS_ILLEGAL_CMD), break_expand, ..Default::default() } @@ -810,7 +810,7 @@ pub fn vars(&self) -> &EnvStack { &self.variables } - /// Get the variables as an Arc. + /// Get the variables as an Rc. pub fn vars_ref(&self) -> Rc { Rc::clone(&self.variables) } diff --git a/src/reader.rs b/src/reader.rs index ef00f2eca..ca13b4cc4 100644 --- a/src/reader.rs +++ b/src/reader.rs @@ -13,8 +13,8 @@ //! end of the list is reached, at which point regular searching will commence. use libc::{ - c_char, c_int, ECHO, EINTR, EIO, EISDIR, ENOTTY, EPERM, ESRCH, ICANON, ICRNL, IEXTEN, INLCR, - IXOFF, IXON, ONLCR, OPOST, O_NONBLOCK, O_RDONLY, SIGINT, SIGTTIN, STDIN_FILENO, STDOUT_FILENO, + c_char, ECHO, EINTR, EIO, EISDIR, ENOTTY, EPERM, ESRCH, ICANON, ICRNL, IEXTEN, INLCR, IXOFF, + IXON, ONLCR, OPOST, O_NONBLOCK, O_RDONLY, SIGINT, SIGTTIN, STDIN_FILENO, STDOUT_FILENO, TCSANOW, VMIN, VQUIT, VSUSP, VTIME, _POSIX_VDISABLE, }; use nix::fcntl::OFlag; @@ -44,6 +44,7 @@ use crate::abbrs::abbrs_match; use crate::ast::{self, Ast, Category, Traversal}; +use crate::builtins::shared::ErrorCode; use crate::builtins::shared::STATUS_CMD_ERROR; use crate::builtins::shared::STATUS_CMD_OK; use crate::color::RgbColor; @@ -618,7 +619,7 @@ fn vars(&self) -> &dyn Environment { /// Read commands from \c fd until encountering EOF. /// The fd is not closed. -pub fn reader_read(parser: &Parser, fd: RawFd, io: &IoChain) -> c_int { +pub fn reader_read(parser: &Parser, fd: RawFd, io: &IoChain) -> Result<(), ErrorCode> { // If reader_read is called recursively through the '.' builtin, we need to preserve // is_interactive. This, and signal handler setup is handled by // proc_push_interactive/proc_pop_interactive. @@ -643,7 +644,8 @@ pub fn reader_read(parser: &Parser, fd: RawFd, io: &IoChain) -> c_int { signal_set_handlers_once(interactive); let res = if interactive { - read_i(parser) + read_i(parser); + Ok(()) } else { read_ni(parser, fd, io) }; @@ -655,7 +657,7 @@ pub fn reader_read(parser: &Parser, fd: RawFd, io: &IoChain) -> c_int { } /// Read interactively. Read input from stdin while providing editing facilities. -fn read_i(parser: &Parser) -> i32 { +fn read_i(parser: &Parser) { assert_is_main_thread(); parser.assert_can_execute(); let mut conf = ReaderConfig::default(); @@ -748,14 +750,12 @@ fn read_i(parser: &Parser) -> i32 { EXIT_STATE.store(ExitState::FinishedHandlers as u8, Ordering::Relaxed); hup_jobs(&parser.jobs()); } - - 0 } /// Read non-interactively. Read input from stdin without displaying the prompt, using syntax /// highlighting. This is used for reading scripts and init files. /// The file is not closed. -fn read_ni(parser: &Parser, fd: RawFd, io: &IoChain) -> i32 { +fn read_ni(parser: &Parser, fd: RawFd, io: &IoChain) -> Result<(), ErrorCode> { let md = match fstat(fd) { Ok(md) => md, Err(err) => { @@ -763,7 +763,7 @@ fn read_ni(parser: &Parser, fd: RawFd, io: &IoChain) -> i32 { error, wgettext_fmt!("Unable to read input file: %s", err.to_string()) ); - return 1; + return Err(STATUS_CMD_ERROR); } }; @@ -775,7 +775,7 @@ fn read_ni(parser: &Parser, fd: RawFd, io: &IoChain) -> i32 { error, wgettext_fmt!("Unable to read input file: %s", Errno(EISDIR).to_string()) ); - return 1; + return Err(STATUS_CMD_ERROR); } // Read all data into a vec. @@ -805,7 +805,7 @@ fn read_ni(parser: &Parser, fd: RawFd, io: &IoChain) -> i32 { error, wgettext_fmt!("Unable to read input file: %s", err.to_string()) ); - return 1; + return Err(STATUS_CMD_ERROR); } } } @@ -831,14 +831,15 @@ fn read_ni(parser: &Parser, fd: RawFd, io: &IoChain) -> i32 { if errored { let sb = parser.get_backtrace(&s, &errors); eprintf!("%ls", sb); - return 1; + return Err(STATUS_CMD_ERROR); } // Construct a parsed source ref. // Be careful to transfer ownership, this could be a very large string. let ps = Arc::new(ParsedSource::new(s, ast)); parser.eval_parsed_source(&ps, io, None, BlockType::top); - 0 + + Ok(()) } /// Initialize the reader. @@ -2674,7 +2675,7 @@ fn handle_readline_command(&mut self, c: ReadlineCmd) { rl::CancelCommandline | rl::ClearCommandline => { if self.conf.exit_on_interrupt { self.parser - .set_last_statuses(Statuses::just(STATUS_CMD_ERROR.unwrap())); + .set_last_statuses(Statuses::just(STATUS_CMD_ERROR)); self.exit_loop_requested = true; return; } @@ -2966,8 +2967,7 @@ fn handle_readline_command(&mut self, c: ReadlineCmd) { } rl::Exit => { // This is by definition a successful exit, override the status - self.parser - .set_last_statuses(Statuses::just(STATUS_CMD_OK.unwrap())); + self.parser.set_last_statuses(Statuses::just(STATUS_CMD_OK)); self.exit_loop_requested = true; check_exit_loop_maybe_warning(Some(self)); } @@ -2979,8 +2979,7 @@ fn handle_readline_command(&mut self, c: ReadlineCmd) { self.delete_char(false); } else if c == rl::DeleteOrExit && el.is_empty() { // This is by definition a successful exit, override the status - self.parser - .set_last_statuses(Statuses::just(STATUS_CMD_OK.unwrap())); + self.parser.set_last_statuses(Statuses::just(STATUS_CMD_OK)); self.exit_loop_requested = true; check_exit_loop_maybe_warning(Some(self)); } @@ -4529,7 +4528,7 @@ pub fn reader_write_title( } let mut lst = vec![]; - exec_subshell( + let _ = exec_subshell( &fish_title_command, parser, Some(&mut lst), @@ -4559,7 +4558,7 @@ fn exec_mode_prompt(&mut self) { self.mode_prompt_buff.clear(); if function::exists(MODE_PROMPT_FUNCTION_NAME, self.parser) { let mut mode_indicator_list = vec![]; - exec_subshell( + let _ = exec_subshell( MODE_PROMPT_FUNCTION_NAME, self.parser, Some(&mut mode_indicator_list), @@ -4615,7 +4614,7 @@ fn exec_prompt(&mut self) { // producing an error. let left_prompt_deleted = zelf.conf.left_prompt_cmd == LEFT_PROMPT_FUNCTION_NAME && !function::exists(&zelf.conf.left_prompt_cmd, zelf.parser); - exec_subshell( + let _ = exec_subshell( if left_prompt_deleted { DEFAULT_PROMPT } else { @@ -4632,7 +4631,7 @@ fn exec_prompt(&mut self) { if function::exists(&zelf.conf.right_prompt_cmd, zelf.parser) { // Status is ignored. let mut prompt_list = vec![]; - exec_subshell( + let _ = exec_subshell( &zelf.conf.right_prompt_cmd, zelf.parser, Some(&mut prompt_list), @@ -5354,13 +5353,14 @@ fn expand_replacer( ); let mut outputs = vec![]; - let ret = exec_subshell( + if exec_subshell( &cmd, parser, Some(&mut outputs), /*apply_exit_status=*/ false, - ); - if ret != STATUS_CMD_OK.unwrap() { + ) + .is_err() + { return None; } let result = join_strings(&outputs, '\n'); @@ -5801,9 +5801,7 @@ fn should_add_to_history(&mut self, text: &wstr) -> bool { false, ); - let ret = exec_subshell(&cmd, parser, None, /*apply_exit_status=*/ false); - - ret == STATUS_CMD_OK.unwrap() + exec_subshell(&cmd, parser, None, /*apply_exit_status=*/ false).is_ok() } // Add the current command line contents to history. diff --git a/src/tests/parser.rs b/src/tests/parser.rs index 532d92db9..d34faa9d6 100644 --- a/src/tests/parser.rs +++ b/src/tests/parser.rs @@ -666,16 +666,10 @@ macro_rules! validate { parser.pushd("test/temp"); validate!(L!("echo -n"), STATUS_CMD_OK.unwrap()); validate!(L!("pwd"), STATUS_CMD_OK.unwrap()); - validate!( - L!("UNMATCHABLE_WILDCARD*"), - STATUS_UNMATCHED_WILDCARD.unwrap() - ); - validate!( - L!("UNMATCHABLE_WILDCARD**"), - STATUS_UNMATCHED_WILDCARD.unwrap() - ); - validate!(L!("?"), STATUS_UNMATCHED_WILDCARD.unwrap()); - validate!(L!("abc?def"), STATUS_UNMATCHED_WILDCARD.unwrap()); + validate!(L!("UNMATCHABLE_WILDCARD*"), STATUS_UNMATCHED_WILDCARD); + validate!(L!("UNMATCHABLE_WILDCARD**"), STATUS_UNMATCHED_WILDCARD); + validate!(L!("?"), STATUS_UNMATCHED_WILDCARD); + validate!(L!("abc?def"), STATUS_UNMATCHED_WILDCARD); parser.popd(); }