From 79b811bfcbd48bba69f9a10d68759dcbcdc3c901 Mon Sep 17 00:00:00 2001 From: mukunda katta Date: Fri, 15 May 2026 08:31:07 -0700 Subject: [PATCH] fix: keep in-place suffix attached --- src/sed/mod.rs | 49 +++++++++++++++++++++++++++++++++++---- tests/by-util/test_sed.rs | 41 ++++++++++++++++++++++---------- 2 files changed, 74 insertions(+), 16 deletions(-) diff --git a/src/sed/mod.rs b/src/sed/mod.rs index 1206140c..33cf4e65 100644 --- a/src/sed/mod.rs +++ b/src/sed/mod.rs @@ -26,6 +26,7 @@ use crate::sed::processor::process_all_files; use crate::sed::script_line_provider::ScriptValue; use clap::{Arg, ArgMatches, Command, arg, crate_version}; use std::collections::HashMap; +use std::ffi::OsString; use std::path::PathBuf; use uucore::error::{UResult, UUsageError}; use uucore::format_usage; @@ -35,7 +36,7 @@ const USAGE: &str = "sed [OPTION]... [script] [file]..."; #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let matches = uu_app().try_get_matches_from(args)?; + let matches = uu_app().try_get_matches_from(normalize_in_place_args(args))?; let (scripts, files) = get_scripts_files(&matches)?; let mut context = build_context(&matches); @@ -44,6 +45,21 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { Ok(()) } +/// Convert GNU-style attached `-iSUFFIX` to clap's long `--in-place=SUFFIX`. +fn normalize_in_place_args(args: impl IntoIterator) -> Vec { + args.into_iter() + .map(|arg| { + if let Some(arg_str) = arg.to_str() + && let Some(suffix) = arg_str.strip_prefix("-i") + && !suffix.is_empty() + { + return OsString::from(format!("--in-place={suffix}")); + } + arg + }) + .collect() +} + #[allow(clippy::cognitive_complexity)] pub fn uu_app() -> Command { #[cfg(windows)] @@ -93,7 +109,9 @@ pub fn uu_app() -> Command { .short('i') .long("in-place") .help("Edit files in place, making a backup if SUFFIX is supplied.") + .value_name("SUFFIX") .num_args(0..=1) + .require_equals(true) .default_missing_value(""), // Access with .get_one::("line-length") arg!(-l --length "Specify the 'l' command line-wrap length.") @@ -231,7 +249,11 @@ mod tests { // Helper function for supplying arguments fn get_test_matches(args: &[&str]) -> ArgMatches { - uu_app().get_matches_from(["myapp"].iter().chain(args.iter())) + let args = ["myapp"] + .into_iter() + .chain(args.iter().copied()) + .map(OsString::from); + uu_app().get_matches_from(normalize_in_place_args(args)) } #[test] @@ -323,7 +345,11 @@ mod tests { // build_context fn test_matches(args: &[&str]) -> ArgMatches { - uu_app().get_matches_from(["sed"].into_iter().chain(args.iter().copied())) + let args = ["sed"] + .into_iter() + .chain(args.iter().copied()) + .map(OsString::from); + uu_app().get_matches_from(normalize_in_place_args(args)) } #[test] @@ -391,13 +417,28 @@ mod tests { #[test] fn test_in_place_with_suffix() { - let matches = test_matches(&["-i", ".bak"]); + let matches = test_matches(&["-i.bak"]); let ctx = build_context(&matches); assert!(ctx.in_place); assert_eq!(ctx.in_place_suffix, Some(".bak".to_string())); } + #[test] + fn test_in_place_with_separate_suffix_arg_leaves_suffix_as_script() { + let matches = test_matches(&["-i", "s/foo/bar/", "file"]); + let ctx = build_context(&matches); + let (scripts, files) = get_scripts_files(&matches).unwrap(); + + assert!(ctx.in_place); + assert!(ctx.in_place_suffix.is_none()); + assert_eq!( + scripts, + vec![ScriptValue::StringVal("s/foo/bar/".to_string())] + ); + assert_eq!(files, vec![PathBuf::from("file")]); + } + #[test] fn test_length_default_and_custom() { let matches_default = test_matches(&[]); diff --git a/tests/by-util/test_sed.rs b/tests/by-util/test_sed.rs index b31fed76..62d28502 100644 --- a/tests/by-util/test_sed.rs +++ b/tests/by-util/test_sed.rs @@ -863,15 +863,9 @@ fn in_place_edit_backup() -> std::io::Result<()> { let path = temp.path().to_path_buf(); let temp_path = temp.into_temp_path(); - // Run the sed-like command with -i (in-place edit) + // Run the sed-like command with a backup suffix attached to -i. new_ucmd!() - .args(&[ - "-i", - ".bak", - "-e", - "s/world/universe/", - path.to_str().unwrap(), - ]) + .args(&["-i.bak", "-e", "s/world/universe/", path.to_str().unwrap()]) .succeeds(); // Read edited file @@ -892,6 +886,31 @@ fn in_place_edit_backup() -> std::io::Result<()> { Ok(()) } +#[test] +fn in_place_separate_suffix_arg_is_script() -> std::io::Result<()> { + let mut temp = NamedTempFile::new()?; + writeln!(temp.as_file_mut(), "quick brown fox")?; + + let path = temp.path().to_path_buf(); + let temp_path = temp.into_temp_path(); + + new_ucmd!() + .args(&["-i", "s/fox/vox/", path.to_str().unwrap()]) + .succeeds(); + + let actual = std::fs::read_to_string(&path)?; + assert_eq!(actual, "quick brown vox\n"); + + let backup_path = path.with_file_name(format!( + "{}s/fox/vox/", + path.file_name().unwrap().to_string_lossy() + )); + assert!(!backup_path.exists()); + + temp_path.close()?; + Ok(()) +} + #[cfg(unix)] #[test] fn in_place_edit_follow_symlink_edits_target() -> Result<(), Box> { @@ -965,8 +984,7 @@ fn in_place_edit_follow_symlink_with_backup() -> Result<(), Box Result<(), Box