From 640844529c243b6cf25769f07df2c41a8b09f516 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sun, 29 Mar 2026 09:27:20 +0000 Subject: [PATCH 1/2] refactor(builtins): migrate sort, uniq, tr, tar, rg to ArgParser Replace manual `while i < args.len()` index management with ArgParser in 5 builtins: sort, uniq, tr (cuttr.rs), tar (archive.rs), and rg. Add `bool_flags()` method to ArgParser for combined short boolean flags like `-rnuf`, used by sort, uniq, and tr. Closes #880 --- crates/bashkit/src/builtins/archive.rs | 47 ++++---- crates/bashkit/src/builtins/arg_parser.rs | 64 +++++++++++ crates/bashkit/src/builtins/cuttr.rs | 23 ++-- crates/bashkit/src/builtins/rg.rs | 77 +++++++++----- crates/bashkit/src/builtins/sortuniq.rs | 124 +++++++++------------- 5 files changed, 196 insertions(+), 139 deletions(-) diff --git a/crates/bashkit/src/builtins/archive.rs b/crates/bashkit/src/builtins/archive.rs index 9efd671e..4765b007 100644 --- a/crates/bashkit/src/builtins/archive.rs +++ b/crates/bashkit/src/builtins/archive.rs @@ -91,13 +91,18 @@ impl Builtin for Tar { let mut files: Vec = Vec::new(); // Parse arguments - let mut i = 0; - while i < ctx.args.len() { - let arg = &ctx.args[i]; - if arg.starts_with('-') && !arg.starts_with("--") { - // Track whether 'f' or 'C' consumed the next arg - let mut consumed_next = false; - for c in arg[1..].chars() { + let mut p = super::arg_parser::ArgParser::new(ctx.args); + while !p.is_done() { + if let Some(val) = p.flag_value_opt("-f") { + archive_file = Some(val.to_string()); + } else if let Some(val) = p.flag_value_opt("-C") { + change_dir = Some(val.to_string()); + } else if p.is_flag() { + // Combined short flags like -czf where f/C take the next arg + let arg = p.current().unwrap(); + let chars: Vec = arg[1..].chars().collect(); + p.advance(); + for c in &chars { match c { 'c' => create = true, 'x' => extract = true, @@ -105,28 +110,24 @@ impl Builtin for Tar { 'v' => verbose = true, 'z' => gzip = true, 'O' => to_stdout = true, - 'f' => { - i += 1; - consumed_next = true; - if i >= ctx.args.len() { + 'f' => match p.positional() { + Some(val) => archive_file = Some(val.to_string()), + None => { return Ok(ExecResult::err( "tar: option requires an argument -- 'f'\n".to_string(), 2, )); } - archive_file = Some(ctx.args[i].clone()); - } - 'C' => { - i += 1; - consumed_next = true; - if i >= ctx.args.len() { + }, + 'C' => match p.positional() { + Some(val) => change_dir = Some(val.to_string()), + None => { return Ok(ExecResult::err( "tar: option requires an argument -- 'C'\n".to_string(), 2, )); } - change_dir = Some(ctx.args[i].clone()); - } + }, _ => { return Ok(ExecResult::err( format!("tar: invalid option -- '{}'\n", c), @@ -134,14 +135,10 @@ impl Builtin for Tar { )); } } - if consumed_next { - break; - } } - } else { - files.push(arg.clone()); + } else if let Some(arg) = p.positional() { + files.push(arg.to_string()); } - i += 1; } // Check for exactly one of -c, -x, -t diff --git a/crates/bashkit/src/builtins/arg_parser.rs b/crates/bashkit/src/builtins/arg_parser.rs index 8be4211c..da5874b6 100644 --- a/crates/bashkit/src/builtins/arg_parser.rs +++ b/crates/bashkit/src/builtins/arg_parser.rs @@ -192,6 +192,26 @@ impl<'a> ArgParser<'a> { .map(|s| s.starts_with('-') && s.len() > 1) .unwrap_or(false) } + + /// Try to consume combined boolean short flags (e.g., `-rnuf`). + /// + /// If the current arg starts with `-` (not `--`), has length > 1, and + /// every character after `-` is in `allowed`, advances and returns the + /// matched chars. Otherwise returns an empty vec without advancing. + pub fn bool_flags(&mut self, allowed: &str) -> Vec { + if let Some(arg) = self.current() + && arg.starts_with('-') + && !arg.starts_with("--") + && arg.len() > 1 + { + let chars: Vec = arg[1..].chars().collect(); + if chars.iter().all(|c| allowed.contains(*c)) { + self.advance(); + return chars; + } + } + Vec::new() + } } #[cfg(test)] @@ -331,6 +351,50 @@ mod tests { assert_eq!(p.rest().len(), 2); } + #[test] + fn test_bool_flags() { + let a = args(&["-rnuf", "file"]); + let mut p = ArgParser::new(&a); + let flags = p.bool_flags("rnufsz"); + assert_eq!(flags, vec!['r', 'n', 'u', 'f']); + assert_eq!(p.current(), Some("file")); + } + + #[test] + fn test_bool_flags_no_match_unknown_char() { + let a = args(&["-rxn", "file"]); + let mut p = ArgParser::new(&a); + let flags = p.bool_flags("rn"); // 'x' not allowed + assert!(flags.is_empty()); + assert_eq!(p.current(), Some("-rxn")); // not advanced + } + + #[test] + fn test_bool_flags_long_flag_ignored() { + let a = args(&["--verbose"]); + let mut p = ArgParser::new(&a); + let flags = p.bool_flags("verbose"); + assert!(flags.is_empty()); + } + + #[test] + fn test_bool_flags_single_dash_ignored() { + let a = args(&["-"]); + let mut p = ArgParser::new(&a); + let flags = p.bool_flags("abc"); + assert!(flags.is_empty()); + assert_eq!(p.current(), Some("-")); // not advanced + } + + #[test] + fn test_bool_flags_single_char() { + let a = args(&["-v"]); + let mut p = ArgParser::new(&a); + let flags = p.bool_flags("v"); + assert_eq!(flags, vec!['v']); + assert!(p.is_done()); + } + #[test] fn test_is_flag() { let a = args(&["-v", "-", "file", "--long"]); diff --git a/crates/bashkit/src/builtins/cuttr.rs b/crates/bashkit/src/builtins/cuttr.rs index 2e386374..a7f898b5 100644 --- a/crates/bashkit/src/builtins/cuttr.rs +++ b/crates/bashkit/src/builtins/cuttr.rs @@ -281,13 +281,12 @@ impl Builtin for Tr { let mut complement = false; // Parse flags (can be combined like -ds, -cd) - let mut non_flag_args: Vec<&String> = Vec::new(); - for arg in ctx.args.iter() { - if arg.starts_with('-') - && arg.len() > 1 - && arg.chars().skip(1).all(|ch| "dscC".contains(ch)) - { - for ch in arg.chars().skip(1) { + let mut non_flag_args: Vec = Vec::new(); + let mut p = super::arg_parser::ArgParser::new(ctx.args); + while !p.is_done() { + let flags = p.bool_flags("dscC"); + if !flags.is_empty() { + for ch in flags { match ch { 'd' => delete = true, 's' => squeeze = true, @@ -295,8 +294,8 @@ impl Builtin for Tr { _ => {} } } - } else { - non_flag_args.push(arg); + } else if let Some(arg) = p.positional() { + non_flag_args.push(arg.to_string()); } } @@ -304,7 +303,7 @@ impl Builtin for Tr { return Ok(ExecResult::err("tr: missing operand\n".to_string(), 1)); } - let mut set1 = expand_char_set(non_flag_args[0]); + let mut set1 = expand_char_set(&non_flag_args[0]); if complement { // Complement: use all byte-range chars (0-255) NOT in set1. // Covers full Latin-1 range so binary data from /dev/urandom @@ -321,7 +320,7 @@ impl Builtin for Tr { let result = if delete && squeeze { // -ds: delete SET1 chars, then squeeze SET2 chars let set2 = if non_flag_args.len() >= 2 { - expand_char_set(non_flag_args[1]) + expand_char_set(&non_flag_args[1]) } else { set1.clone() }; @@ -343,7 +342,7 @@ impl Builtin for Tr { )); } - let set2 = expand_char_set(non_flag_args[1]); + let set2 = expand_char_set(&non_flag_args[1]); let translated: String = stdin .chars() diff --git a/crates/bashkit/src/builtins/rg.rs b/crates/bashkit/src/builtins/rg.rs index 50aa7ce3..5219ace9 100644 --- a/crates/bashkit/src/builtins/rg.rs +++ b/crates/bashkit/src/builtins/rg.rs @@ -18,7 +18,7 @@ use async_trait::async_trait; use regex::Regex; -use super::search_common::{build_search_regex, collect_files_recursive, parse_numeric_flag_arg}; +use super::search_common::{build_search_regex, collect_files_recursive}; use super::{Builtin, Context, read_text_file, resolve_path}; use crate::error::{Error, Result}; use crate::interpreter::ExecResult; @@ -57,15 +57,38 @@ impl RgOptions { }; let mut positional = Vec::new(); - let mut i = 0; - - while i < args.len() { - let arg = &args[i]; - if arg.starts_with('-') && arg.len() > 1 && !arg.starts_with("--") { + let mut p = super::arg_parser::ArgParser::new(args); + + while !p.is_done() { + if let Ok(Some(val)) = p.flag_value("-m", "rg") { + opts.max_count = Some( + val.parse() + .map_err(|_| Error::Execution(format!("rg: invalid -m value: {val}")))?, + ); + } else if p.flag("--no-filename") { + opts.no_filename = true; + } else if p.flag("--no-line-number") { + opts.line_numbers = false; + } else if p.flag("--line-number") { + opts.line_numbers = true; + } else if p.flag("--color") { + // no-op (may have separate value arg like "never", skip it) + } else if p.current().is_some_and(|s| s.starts_with("--color=")) { + // --color=VALUE is a no-op + p.advance(); + } else if p.is_flag() { + // Combined short flags like -inFw + // Safe: is_flag() guarantees current() is Some + let arg = p.current().expect("is_flag guarantees Some"); + if arg.starts_with("--") { + // Unknown long option, skip + p.advance(); + continue; + } let chars: Vec = arg[1..].chars().collect(); - let mut j = 0; - while j < chars.len() { - match chars[j] { + p.advance(); + for (j, &c) in chars.iter().enumerate() { + match c { 'i' => opts.ignore_case = true, 'n' => opts.line_numbers = true, 'N' => opts.line_numbers = false, @@ -75,29 +98,31 @@ impl RgOptions { 'w' => opts.word_boundary = true, 'F' => opts.fixed_strings = true, 'm' => { - opts.max_count = - Some(parse_numeric_flag_arg(&chars, j, &mut i, args, "rg", "-m")?); + // Rest of this flag group or next arg is the value + let rest: String = chars[j + 1..].iter().collect(); + let num_str = if !rest.is_empty() { + rest + } else { + match p.positional() { + Some(v) => v.to_string(), + None => { + return Err(Error::Execution( + "rg: -m requires an argument".to_string(), + )); + } + } + }; + opts.max_count = Some(num_str.parse().map_err(|_| { + Error::Execution(format!("rg: invalid -m value: {num_str}")) + })?); break; } _ => {} // ignore unknown } - j += 1; } - } else if let Some(opt) = arg.strip_prefix("--") { - if opt == "no-filename" { - opts.no_filename = true; - } else if opt == "color" || opt.starts_with("color=") { - // no-op - } else if opt == "no-line-number" { - opts.line_numbers = false; - } else if opt == "line-number" { - opts.line_numbers = true; - } - // ignore other long options - } else { - positional.push(arg.clone()); + } else if let Some(arg) = p.positional() { + positional.push(arg.to_string()); } - i += 1; } if positional.is_empty() { diff --git a/crates/bashkit/src/builtins/sortuniq.rs b/crates/bashkit/src/builtins/sortuniq.rs index 47253c4c..b61e489b 100644 --- a/crates/bashkit/src/builtins/sortuniq.rs +++ b/crates/bashkit/src/builtins/sortuniq.rs @@ -129,65 +129,44 @@ impl Builtin for Sort { let mut zero_terminated = false; let mut files = Vec::new(); - let mut i = 0; - while i < ctx.args.len() { - let arg = &ctx.args[i]; - if arg == "-t" { - i += 1; - if i < ctx.args.len() { - delimiter = ctx.args[i].chars().next(); - } - } else if let Some(d) = arg.strip_prefix("-t") { - delimiter = d.chars().next(); - } else if arg == "-k" { - i += 1; - if i < ctx.args.len() { - // Parse key: "2" or "2,2" or "2n" - let key_spec = &ctx.args[i]; - let field_str: String = key_spec - .chars() - .take_while(|c| c.is_ascii_digit()) - .collect(); - key_field = field_str.parse().ok(); - // Check for type suffix in key spec - if key_spec.contains('n') { - numeric = true; - } - if key_spec.contains('r') { - reverse = true; - } - } - } else if let Some(k) = arg.strip_prefix("-k") { - let field_str: String = k.chars().take_while(|c| c.is_ascii_digit()).collect(); + let mut p = super::arg_parser::ArgParser::new(ctx.args); + while !p.is_done() { + if let Some(val) = p.flag_value_opt("-t") { + delimiter = val.chars().next(); + } else if let Some(val) = p.flag_value_opt("-k") { + // Parse key: "2" or "2,2" or "2n" + let field_str: String = val.chars().take_while(|c| c.is_ascii_digit()).collect(); key_field = field_str.parse().ok(); - if k.contains('n') { + if val.contains('n') { numeric = true; } - } else if arg == "-o" { - i += 1; - if i < ctx.args.len() { - output_file = Some(ctx.args[i].clone()); + if val.contains('r') { + reverse = true; } - } else if arg.starts_with('-') && !arg.starts_with("--") { - for c in arg.chars().skip(1) { - match c { - 'r' => reverse = true, - 'n' => numeric = true, - 'u' => unique = true, - 'f' => fold_case = true, - 's' => stable = true, - 'c' | 'C' => check_sorted = true, - 'h' => human_numeric = true, - 'M' => month_sort = true, - 'm' => merge = true, - 'z' => zero_terminated = true, - _ => {} + } else if let Some(val) = p.flag_value_opt("-o") { + output_file = Some(val.to_string()); + } else { + let flags = p.bool_flags("rnufscChMmz"); + if !flags.is_empty() { + for c in flags { + match c { + 'r' => reverse = true, + 'n' => numeric = true, + 'u' => unique = true, + 'f' => fold_case = true, + 's' => stable = true, + 'c' | 'C' => check_sorted = true, + 'h' => human_numeric = true, + 'M' => month_sort = true, + 'm' => merge = true, + 'z' => zero_terminated = true, + _ => {} + } } + } else if let Some(arg) = p.positional() { + files.push(arg.to_string()); } - } else { - files.push(arg.clone()); } - i += 1; } // Collect all input @@ -421,33 +400,26 @@ impl Builtin for Uniq { let mut skip_fields: usize = 0; let mut files = Vec::new(); - let mut idx = 0; - while idx < ctx.args.len() { - let arg = &ctx.args[idx]; - if arg == "-f" { - idx += 1; - if idx < ctx.args.len() { - skip_fields = ctx.args[idx].parse().unwrap_or(0); - } - } else if let Some(n) = arg - .strip_prefix("-f") - .filter(|s| s.chars().all(|c| c.is_ascii_digit())) - { - skip_fields = n.parse().unwrap_or(0); - } else if arg.starts_with('-') && !arg.starts_with("--") { - for c in arg.chars().skip(1) { - match c { - 'c' => count = true, - 'd' => only_duplicates = true, - 'u' => only_unique = true, - 'i' => case_insensitive = true, - _ => {} + let mut p = super::arg_parser::ArgParser::new(ctx.args); + while !p.is_done() { + if let Some(val) = p.flag_value_opt("-f") { + skip_fields = val.parse().unwrap_or(0); + } else { + let flags = p.bool_flags("cdui"); + if !flags.is_empty() { + for c in flags { + match c { + 'c' => count = true, + 'd' => only_duplicates = true, + 'u' => only_unique = true, + 'i' => case_insensitive = true, + _ => {} + } } + } else if let Some(arg) = p.positional() { + files.push(arg.to_string()); } - } else { - files.push(arg.clone()); } - idx += 1; } // Get input lines From 39abe001744480934291f79e2580ee1e080c1820 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sun, 29 Mar 2026 09:45:08 +0000 Subject: [PATCH 2/2] chore: update cargo-vet exemptions for dependency bumps Update version exemptions for 8 crates that received minor/patch updates: ctor 0.8.0, dtor 0.3.0, insta 1.47.1, nalgebra 0.33.3, napi 3.8.4, napi-derive 3.5.3, zerocopy 0.8.48, zerocopy-derive 0.8.48. --- supply-chain/config.toml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/supply-chain/config.toml b/supply-chain/config.toml index b2ec4b7e..17755864 100644 --- a/supply-chain/config.toml +++ b/supply-chain/config.toml @@ -291,7 +291,7 @@ version = "0.2.1" criteria = "safe-to-deploy" [[exemptions.ctor]] -version = "0.6.3" +version = "0.8.0" criteria = "safe-to-deploy" [[exemptions.ctor-proc-macro]] @@ -315,7 +315,7 @@ version = "0.2.5" criteria = "safe-to-deploy" [[exemptions.dtor]] -version = "0.1.1" +version = "0.3.0" criteria = "safe-to-deploy" [[exemptions.dtor-proc-macro]] @@ -575,7 +575,7 @@ version = "2.13.0" criteria = "safe-to-deploy" [[exemptions.insta]] -version = "1.47.0" +version = "1.47.1" criteria = "safe-to-run" [[exemptions.instant]] @@ -731,11 +731,11 @@ version = "1.2.0" criteria = "safe-to-deploy" [[exemptions.nalgebra]] -version = "0.33.2" +version = "0.33.3" criteria = "safe-to-deploy" [[exemptions.napi]] -version = "3.8.3" +version = "3.8.4" criteria = "safe-to-deploy" [[exemptions.napi-build]] @@ -743,7 +743,7 @@ version = "2.3.1" criteria = "safe-to-deploy" [[exemptions.napi-derive]] -version = "3.5.2" +version = "3.5.3" criteria = "safe-to-deploy" [[exemptions.napi-derive-backend]] @@ -1719,11 +1719,11 @@ version = "0.8.1" criteria = "safe-to-deploy" [[exemptions.zerocopy]] -version = "0.8.47" +version = "0.8.48" criteria = "safe-to-deploy" [[exemptions.zerocopy-derive]] -version = "0.8.47" +version = "0.8.48" criteria = "safe-to-deploy" [[exemptions.zerofrom]]