From 84d96f9d028a4dd8c0f98ba6db2ba6dc90c79dcb Mon Sep 17 00:00:00 2001 From: zhitkoff Date: Sat, 26 Aug 2023 11:11:46 -0400 Subject: [PATCH] split: refactor for more common use case --- src/uu/split/src/split.rs | 81 ++++++++++++++++++- tests/by-util/test_split.rs | 152 +++++++++++++++++++++++++++++++++++- 2 files changed, 229 insertions(+), 4 deletions(-) diff --git a/src/uu/split/src/split.rs b/src/uu/split/src/split.rs index 71aacfed0..c4b7e6d66 100644 --- a/src/uu/split/src/split.rs +++ b/src/uu/split/src/split.rs @@ -51,14 +51,66 @@ const AFTER_HELP: &str = help_section!("after help", "split.md"); #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { + let args = args.collect_lossy(); + + let (args, obs_lines) = handle_obsolete(&args[..]); + let matches = uu_app().try_get_matches_from(args)?; - match Settings::from(&matches) { + + match Settings::from(&matches, &obs_lines) { Ok(settings) => split(&settings), Err(e) if e.requires_usage() => Err(UUsageError::new(1, format!("{e}"))), Err(e) => Err(USimpleError::new(1, format!("{e}"))), } } +/// Extract obsolete shorthand (if any) for specifying lines in following scenarios (and similar) +/// `split -22 file` would mean `split -l 22 file` +/// `split -2de file` would mean `split -l 2 -d -e file` +/// `split -x300e file` would mean `split -x -l 300 -e file` +/// `split -x300e -22 file` would mean `split -x -e -l 22 file` (last obsolete lines option wins) +/// following GNU `split` behavior +fn handle_obsolete(args: &[String]) -> (Vec, Option) { + let mut v: Vec = vec![]; + let mut obs_lines = None; + for arg in args.iter() { + let slice = &arg; + if slice.starts_with('-') && !slice.starts_with("--") { + // start of the short option string + // extract numeric part and filter it out + let mut obs_lines_extracted: Vec = vec![]; + let filtered_slice: Vec = slice + .chars() + .filter(|c| { + if c.is_ascii_digit() { + obs_lines_extracted.push(*c); + false + } else { + true + } + }) + .collect(); + + if filtered_slice.get(1).is_some() { + // there were some short options in front of obsolete lines number + // i.e. '-xd100' or similar + // preserve it + v.push(filtered_slice.iter().collect()) + } + if !obs_lines_extracted.is_empty() { + // obsolete lines value was extracted + obs_lines = Some(obs_lines_extracted.iter().collect()); + } + } else { + // not a short option + // preserve it + v.push(arg.to_owned()); + } + } + println!("{:#?}",v); + (v, obs_lines) +} + pub fn uu_app() -> Command { Command::new(uucore::util_name()) .version(crate_version!()) @@ -357,6 +409,17 @@ impl fmt::Display for StrategyError { } impl Strategy { + /// Parse a strategy from obsolete line option value + fn from_obs(obs_value: &str) -> Result { + let n = parse_size(obs_value).map_err(StrategyError::Lines)?; + if n > 0 { + Ok(Self::Lines(n)) + } else { + Err(StrategyError::Lines(ParseSizeError::ParseFailure( + obs_value.to_string(), + ))) + } + } /// Parse a strategy from the command-line arguments. fn from(matches: &ArgMatches) -> Result { fn get_and_parse( @@ -506,7 +569,7 @@ impl fmt::Display for SettingsError { impl Settings { /// Parse a strategy from the command-line arguments. - fn from(matches: &ArgMatches) -> Result { + fn from(matches: &ArgMatches, obs_lines: &Option) -> Result { let additional_suffix = matches .get_one::(OPT_ADDITIONAL_SUFFIX) .unwrap() @@ -514,7 +577,19 @@ impl Settings { if additional_suffix.contains('/') { return Err(SettingsError::SuffixContainsSeparator(additional_suffix)); } - let strategy = Strategy::from(matches).map_err(SettingsError::Strategy)?; + + // obsolete lines option cannot be used simultaneously with OPT_LINES + let strategy = match obs_lines { + Some(obs_value) => { + if matches.value_source(OPT_LINES) == Some(ValueSource::CommandLine) { + return Err(SettingsError::Strategy(StrategyError::MultipleWays)); + } else { + Strategy::from_obs(obs_value).map_err(SettingsError::Strategy)? + } + } + None => Strategy::from(matches).map_err(SettingsError::Strategy)?, + }; + let (suffix_type, suffix_start) = suffix_type_from(matches)?; let suffix_length_str = matches.get_one::(OPT_SUFFIX_LENGTH).unwrap(); let suffix_length: usize = suffix_length_str diff --git a/tests/by-util/test_split.rs b/tests/by-util/test_split.rs index c5f32482e..fe753f084 100644 --- a/tests/by-util/test_split.rs +++ b/tests/by-util/test_split.rs @@ -2,7 +2,7 @@ // // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore xzaaa sixhundredfiftyonebytes ninetyonebytes threebytes asciilowercase fghij klmno pqrst uvwxyz fivelines twohundredfortyonebytes onehundredlines nbbbb +// spell-checker:ignore xzaaa sixhundredfiftyonebytes ninetyonebytes threebytes asciilowercase fghij klmno pqrst uvwxyz fivelines twohundredfortyonebytes onehundredlines nbbbb dxen use crate::common::util::{AtPath, TestScenario}; use rand::{thread_rng, Rng, SeedableRng}; @@ -320,6 +320,156 @@ fn test_split_lines_number() { .stderr_only("split: invalid number of lines: '2fb'\n"); } +/// Test for obsolete lines option standalone +#[test] +fn test_split_obs_lines_standalone() { + let (at, mut ucmd) = at_and_ucmd!(); + let name = "obs-lines-standalone"; + RandomFile::new(&at, name).add_lines(4); + ucmd.args(&["-2", name]).succeeds().no_stderr().no_stdout(); + let glob = Glob::new(&at, ".", r"x[[:alpha:]][[:alpha:]]$"); + assert_eq!(glob.count(), 2); + assert_eq!(glob.collate(), at.read_bytes(name)); +} + +/// Test for invalid obsolete lines option +#[test] +fn test_split_invalid_obs_lines_standalone() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.touch("file"); + + scene + .ucmd() + .args(&["-2fb", "file"]) + .fails() + .code_is(1) + .stderr_only("error: unexpected argument '-f' found\n"); +} + +/// Test for obsolete lines option as part of combined short options +#[test] +fn test_split_obs_lines_within_combined_shorts() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + let name = "obs-lines-within-shorts"; + RandomFile::new(&at, name).add_lines(400); + + scene + .ucmd() + .args(&["-d200xe", name]) + .succeeds() + .no_stderr() + .no_stdout(); + let glob = Glob::new(&at, ".", r"x[[:alpha:]][[:alpha:]]$"); + assert_eq!(glob.count(), 2); + assert_eq!(glob.collate(), at.read_bytes(name)) +} + +/// Test for obsolete lines option starts as part of combined short options +#[test] +fn test_split_obs_lines_starts_combined_shorts() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + let name = "obs-lines-starts-shorts"; + RandomFile::new(&at, name).add_lines(400); + + scene + .ucmd() + .args(&["-x200d", name]) + .succeeds() + .no_stderr() + .no_stdout(); + let glob = Glob::new(&at, ".", r"x[[:alpha:]][[:alpha:]]$"); + assert_eq!(glob.count(), 2); + assert_eq!(glob.collate(), at.read_bytes(name)) +} + +/// Test for using both obsolete lines (standalone) option and short/long lines option simultaneously +#[test] +fn test_split_both_lines_and_obs_lines_standalone() { + // This test will ensure that if both lines option '-l' or '--lines' + // and obsolete lines option '-100' are used + // it fails + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.touch("file"); + + scene + .ucmd() + .args(&["-l", "2", "-2", "file"]) + .fails() + .code_is(1) + .stderr_contains("split: cannot split in more than one way\n"); + scene + .ucmd() + .args(&["--lines", "2", "-2", "file"]) + .fails() + .code_is(1) + .stderr_contains("split: cannot split in more than one way\n"); +} + +/// Test for using more than one obsolete lines option (standalone) +/// last one wins +#[test] +fn test_split_multiple_obs_lines_standalone() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + let name = "multiple-obs-lines"; + RandomFile::new(&at, name).add_lines(400); + + scene + .ucmd() + .args(&["-3000", "-200", name]) + .succeeds() + .no_stderr() + .no_stdout(); + let glob = Glob::new(&at, ".", r"x[[:alpha:]][[:alpha:]]$"); + assert_eq!(glob.count(), 2); + assert_eq!(glob.collate(), at.read_bytes(name)) +} + +/// Test for using more than one obsolete lines option within combined shorts +/// last one wins +#[test] +fn test_split_multiple_obs_lines_within_combined() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + let name = "multiple-obs-lines"; + RandomFile::new(&at, name).add_lines(400); + + scene + .ucmd() + .args(&["-x5000d -e200x", name]) + .succeeds() + .no_stderr() + .no_stdout(); + let glob = Glob::new(&at, ".", r"x[[:alpha:]][[:alpha:]]$"); + assert_eq!(glob.count(), 2); + assert_eq!(glob.collate(), at.read_bytes(name)) +} + +/// Test for using both obsolete lines option within combined shorts with conflicting -n option simultaneously +#[test] +fn test_split_obs_lines_within_combined_with_number() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.touch("file"); + + scene + .ucmd() + .args(&["-3dxen", "4", "file"]) + .fails() + .code_is(1) + .stderr_contains("split: cannot split in more than one way\n"); + scene + .ucmd() + .args(&["-dxe30n", "4", "file"]) + .fails() + .code_is(1) + .stderr_contains("split: cannot split in more than one way\n"); +} + #[test] fn test_split_invalid_bytes_size() { new_ucmd!()