diff --git a/Cargo.lock b/Cargo.lock index d70d307..2244a3b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -122,7 +122,7 @@ checksum = "6184e33543162437515c2e2b48714794e37845ec9851711914eec9d308f6ebe8" [[package]] name = "diffutils" -version = "0.4.0" +version = "0.4.1" dependencies = [ "assert_cmd", "chrono", diff --git a/Cargo.toml b/Cargo.toml index 0e8dab9..f219dbb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "diffutils" -version = "0.4.0" +version = "0.4.1" edition = "2021" description = "A CLI app for generating diff files" license = "MIT OR Apache-2.0" diff --git a/LICENSE-APACHE b/LICENSE-APACHE index 1b5ec8b..3d8493e 100644 --- a/LICENSE-APACHE +++ b/LICENSE-APACHE @@ -1,3 +1,6 @@ +Copyright (c) Michael Howell +Copyright (c) uutils developers + Apache License Version 2.0, January 2004 http://www.apache.org/licenses/ diff --git a/LICENSE-MIT b/LICENSE-MIT index 31aa793..ba40932 100644 --- a/LICENSE-MIT +++ b/LICENSE-MIT @@ -1,3 +1,6 @@ +Copyright (c) Michael Howell +Copyright (c) uutils developers + Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the diff --git a/src/params.rs b/src/params.rs index 7e9cc78..62ee518 100644 --- a/src/params.rs +++ b/src/params.rs @@ -1,4 +1,5 @@ -use std::ffi::{OsStr, OsString}; +use std::ffi::OsString; +use std::path::PathBuf; use regex::Regex; @@ -11,17 +12,6 @@ pub enum Format { Ed, } -#[cfg(unix)] -fn osstr_bytes(osstr: &OsStr) -> &[u8] { - use std::os::unix::ffi::OsStrExt; - osstr.as_bytes() -} - -#[cfg(not(unix))] -fn osstr_bytes(osstr: &OsStr) -> Vec { - osstr.to_string_lossy().bytes().collect() -} - #[derive(Clone, Debug, Eq, PartialEq)] pub struct Params { pub from: OsString, @@ -50,7 +40,7 @@ impl Default for Params { } pub fn parse_params>(opts: I) -> Result { - let mut opts = opts.into_iter(); + let mut opts = opts.into_iter().peekable(); // parse CLI let Some(exe) = opts.next() else { @@ -60,8 +50,10 @@ pub fn parse_params>(opts: I) -> Result\d+)$").unwrap(); while let Some(param) = opts.next() { + let next_param = opts.peek(); if param == "--" { break; } @@ -87,6 +79,20 @@ pub fn parse_params>(opts: I) -> Result>(opts: I) -> Result { - params.context_count = (b - b'0') as usize; - while let Some(b'0'..=b'9') = bit.peek() { - params.context_count *= 10; - params.context_count += (bit.next().unwrap() - b'0') as usize; - } + match match_context_diff_params(¶m, next_param, format) { + Ok(DiffStyleMatch { + is_match, + context_count, + next_param_consumed, + }) => { + if is_match { + format = Some(Format::Context); + if context_count.is_some() { + context = context_count; } - b'c' => { - if format.is_some() && format != Some(Format::Context) { - return Err("Conflicting output style options".to_string()); - } - format = Some(Format::Context); + if next_param_consumed { + opts.next(); } - b'e' => { - if format.is_some() && format != Some(Format::Ed) { - return Err("Conflicting output style options".to_string()); - } - format = Some(Format::Ed); - } - b'u' => { - if format.is_some() && format != Some(Format::Unified) { - return Err("Conflicting output style options".to_string()); - } - format = Some(Format::Unified); + continue; + } + } + Err(error) => return Err(error), + } + match match_unified_diff_params(¶m, next_param, format) { + Ok(DiffStyleMatch { + is_match, + context_count, + next_param_consumed, + }) => { + if is_match { + format = Some(Format::Unified); + if context_count.is_some() { + context = context_count; } - b'U' => { - if format.is_some() && format != Some(Format::Unified) { - return Err("Conflicting output style options".to_string()); - } - format = Some(Format::Unified); - let context_count_maybe = if bit.peek().is_some() { - String::from_utf8(bit.collect::>()).ok() - } else { - opts.next().map(|x| x.to_string_lossy().into_owned()) - }; - if let Some(context_count_maybe) = - context_count_maybe.and_then(|x| x.parse().ok()) - { - params.context_count = context_count_maybe; - break; - } - return Err("Invalid context count".to_string()); + if next_param_consumed { + opts.next(); } - _ => return Err(format!("Unknown option: {}", String::from_utf8_lossy(&[b]))), + continue; } } - } else if from.is_none() { + Err(error) => return Err(error), + } + if param.to_string_lossy().starts_with('-') { + return Err(format!("Unknown option: {:?}", param)); + } + if from.is_none() { from = Some(param); } else if to.is_none() { to = Some(param); @@ -178,10 +172,125 @@ pub fn parse_params>(opts: I) -> Result ", exe.to_string_lossy())); }; + + // diff DIRECTORY FILE => diff DIRECTORY/FILE FILE + // diff FILE DIRECTORY => diff FILE DIRECTORY/FILE + let mut from_path: PathBuf = PathBuf::from(¶ms.from); + let mut to_path: PathBuf = PathBuf::from(¶ms.to); + + if from_path.is_dir() && to_path.is_file() { + from_path.push(to_path.file_name().unwrap()); + params.from = from_path.into_os_string(); + } else if from_path.is_file() && to_path.is_dir() { + to_path.push(from_path.file_name().unwrap()); + params.to = to_path.into_os_string(); + } + params.format = format.unwrap_or(Format::default()); + if let Some(context_count) = context { + params.context_count = context_count; + } Ok(params) } +struct DiffStyleMatch { + is_match: bool, + context_count: Option, + next_param_consumed: bool, +} + +fn match_context_diff_params( + param: &OsString, + next_param: Option<&OsString>, + format: Option, +) -> Result { + const CONTEXT_RE: &str = r"^(-[cC](?\d*)|--context(=(?\d*))?|-(?\d+)c)$"; + let regex = Regex::new(CONTEXT_RE).unwrap(); + let is_match = regex.is_match(param.to_string_lossy().as_ref()); + let mut context_count = None; + let mut next_param_consumed = false; + if is_match { + if format.is_some() && format != Some(Format::Context) { + return Err("Conflicting output style options".to_string()); + } + let captures = regex.captures(param.to_str().unwrap()).unwrap(); + let num = captures + .name("num1") + .or(captures.name("num2")) + .or(captures.name("num3")); + if let Some(numvalue) = num { + if !numvalue.as_str().is_empty() { + context_count = Some(numvalue.as_str().parse::().unwrap()); + } + } + if param == "-C" && next_param.is_some() { + match next_param.unwrap().to_string_lossy().parse::() { + Ok(context_size) => { + context_count = Some(context_size); + next_param_consumed = true; + } + Err(_) => { + return Err(format!( + "invalid context length '{}'", + next_param.unwrap().to_string_lossy() + )) + } + } + } + } + Ok(DiffStyleMatch { + is_match, + context_count, + next_param_consumed, + }) +} + +fn match_unified_diff_params( + param: &OsString, + next_param: Option<&OsString>, + format: Option, +) -> Result { + const UNIFIED_RE: &str = r"^(-[uU](?\d*)|--unified(=(?\d*))?|-(?\d+)u)$"; + let regex = Regex::new(UNIFIED_RE).unwrap(); + let is_match = regex.is_match(param.to_string_lossy().as_ref()); + let mut context_count = None; + let mut next_param_consumed = false; + if is_match { + if format.is_some() && format != Some(Format::Unified) { + return Err("Conflicting output style options".to_string()); + } + let captures = regex.captures(param.to_str().unwrap()).unwrap(); + let num = captures + .name("num1") + .or(captures.name("num2")) + .or(captures.name("num3")); + if let Some(numvalue) = num { + if !numvalue.as_str().is_empty() { + context_count = Some(numvalue.as_str().parse::().unwrap()); + } + } + if param == "-U" && next_param.is_some() { + match next_param.unwrap().to_string_lossy().parse::() { + Ok(context_size) => { + context_count = Some(context_size); + next_param_consumed = true; + } + Err(_) => { + return Err(format!( + "invalid context length '{}'", + next_param.unwrap().to_string_lossy() + )) + } + } + } + } + Ok(DiffStyleMatch { + is_match, + context_count, + next_param_consumed, + }) +} + #[cfg(test)] mod tests { use super::*; @@ -198,20 +307,148 @@ mod tests { }), parse_params([os("diff"), os("foo"), os("bar")].iter().cloned()) ); - } - #[test] - fn basics_ed() { assert_eq!( Ok(Params { from: os("foo"), to: os("bar"), - format: Format::Ed, ..Default::default() }), - parse_params([os("diff"), os("-e"), os("foo"), os("bar")].iter().cloned()) + parse_params( + [os("diff"), os("--normal"), os("foo"), os("bar")] + .iter() + .cloned() + ) ); } #[test] + fn basics_ed() { + for arg in ["-e", "--ed"] { + assert_eq!( + Ok(Params { + from: os("foo"), + to: os("bar"), + format: Format::Ed, + ..Default::default() + }), + parse_params([os("diff"), os(arg), os("foo"), os("bar")].iter().cloned()) + ); + } + } + #[test] + fn context_valid() { + for args in [vec!["-c"], vec!["--context"], vec!["--context="]] { + let mut params = vec!["diff"]; + params.extend(args); + params.extend(["foo", "bar"]); + assert_eq!( + Ok(Params { + from: os("foo"), + to: os("bar"), + format: Format::Context, + ..Default::default() + }), + parse_params(params.iter().map(|x| os(x))) + ); + } + for args in [ + vec!["-c42"], + vec!["-C42"], + vec!["-C", "42"], + vec!["--context=42"], + vec!["-42c"], + ] { + let mut params = vec!["diff"]; + params.extend(args); + params.extend(["foo", "bar"]); + assert_eq!( + Ok(Params { + from: os("foo"), + to: os("bar"), + format: Format::Context, + context_count: 42, + ..Default::default() + }), + parse_params(params.iter().map(|x| os(x))) + ); + } + } + #[test] + fn context_invalid() { + for args in [ + vec!["-c", "42"], + vec!["-c=42"], + vec!["-c="], + vec!["-C"], + vec!["-C=42"], + vec!["-C="], + vec!["--context42"], + vec!["--context", "42"], + vec!["-42C"], + ] { + let mut params = vec!["diff"]; + params.extend(args); + params.extend(["foo", "bar"]); + assert!(parse_params(params.iter().map(|x| os(x))).is_err()); + } + } + #[test] + fn unified_valid() { + for args in [vec!["-u"], vec!["--unified"], vec!["--unified="]] { + let mut params = vec!["diff"]; + params.extend(args); + params.extend(["foo", "bar"]); + assert_eq!( + Ok(Params { + from: os("foo"), + to: os("bar"), + format: Format::Unified, + ..Default::default() + }), + parse_params(params.iter().map(|x| os(x))) + ); + } + for args in [ + vec!["-u42"], + vec!["-U42"], + vec!["-U", "42"], + vec!["--unified=42"], + vec!["-42u"], + ] { + let mut params = vec!["diff"]; + params.extend(args); + params.extend(["foo", "bar"]); + assert_eq!( + Ok(Params { + from: os("foo"), + to: os("bar"), + format: Format::Unified, + context_count: 42, + ..Default::default() + }), + parse_params(params.iter().map(|x| os(x))) + ); + } + } + #[test] + fn unified_invalid() { + for args in [ + vec!["-u", "42"], + vec!["-u=42"], + vec!["-u="], + vec!["-U"], + vec!["-U=42"], + vec!["-U="], + vec!["--unified42"], + vec!["--unified", "42"], + vec!["-42U"], + ] { + let mut params = vec!["diff"]; + params.extend(args); + params.extend(["foo", "bar"]); + assert!(parse_params(params.iter().map(|x| os(x))).is_err()); + } + } + #[test] fn context_count() { assert_eq!( Ok(Params { @@ -504,7 +741,15 @@ mod tests { } #[test] fn conflicting_output_styles() { - for (arg1, arg2) in [("-u", "-c"), ("-u", "-e"), ("-c", "-u"), ("-c", "-U42")] { + for (arg1, arg2) in [ + ("-u", "-c"), + ("-u", "-e"), + ("-c", "-u"), + ("-c", "-U42"), + ("-u", "--normal"), + ("--normal", "-e"), + ("--context", "--normal"), + ] { assert!(parse_params( [os("diff"), os(arg1), os(arg2), os("foo"), os("bar")] .iter() diff --git a/tests/integration.rs b/tests/integration.rs index 853ba4d..9d0cfaf 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -6,8 +6,9 @@ use assert_cmd::cmd::Command; use diffutilslib::assert_diff_eq; use predicates::prelude::*; +use std::fs::File; use std::io::Write; -use tempfile::NamedTempFile; +use tempfile::{tempdir, NamedTempFile}; // Integration tests for the diffutils command @@ -18,7 +19,7 @@ fn unknown_param() -> Result<(), Box> { cmd.assert() .code(predicate::eq(2)) .failure() - .stderr(predicate::str::starts_with("Usage: ")); + .stderr(predicate::str::starts_with("Unknown option: \"--foobar\"")); Ok(()) } @@ -26,22 +27,26 @@ fn unknown_param() -> Result<(), Box> { fn cannot_read_files() -> Result<(), Box> { let file = NamedTempFile::new()?; + let nofile = NamedTempFile::new()?; + let nopath = nofile.into_temp_path(); + std::fs::remove_file(&nopath)?; + let mut cmd = Command::cargo_bin("diffutils")?; - cmd.arg("foo.txt").arg(file.path()); + cmd.arg(&nopath).arg(file.path()); cmd.assert() .code(predicate::eq(2)) .failure() .stderr(predicate::str::starts_with("Failed to read from-file")); let mut cmd = Command::cargo_bin("diffutils")?; - cmd.arg(file.path()).arg("foo.txt"); + cmd.arg(file.path()).arg(&nopath); cmd.assert() .code(predicate::eq(2)) .failure() .stderr(predicate::str::starts_with("Failed to read to-file")); let mut cmd = Command::cargo_bin("diffutils")?; - cmd.arg("foo.txt").arg("foo.txt"); + cmd.arg(&nopath).arg(&nopath); cmd.assert() .code(predicate::eq(2)) .failure() @@ -234,3 +239,49 @@ fn read_from_stdin() -> Result<(), Box> { Ok(()) } + +#[test] +fn compare_file_to_directory() -> Result<(), Box> { + let tmp_dir = tempdir()?; + + let directory = tmp_dir.path().join("d"); + let _ = std::fs::create_dir(&directory); + + let a_path = tmp_dir.path().join("a"); + let mut a = File::create(&a_path).unwrap(); + a.write_all(b"a\n").unwrap(); + + let da_path = directory.join("a"); + let mut da = File::create(&da_path).unwrap(); + da.write_all(b"da\n").unwrap(); + + let mut cmd = Command::cargo_bin("diffutils")?; + cmd.arg("-u").arg(&directory).arg(&a_path); + cmd.assert().code(predicate::eq(1)).failure(); + + let output = cmd.output().unwrap().stdout; + assert_diff_eq!( + output, + format!( + "--- {}\tTIMESTAMP\n+++ {}\tTIMESTAMP\n@@ -1 +1 @@\n-da\n+a\n", + da_path.display(), + a_path.display() + ) + ); + + let mut cmd = Command::cargo_bin("diffutils")?; + cmd.arg("-u").arg(&a_path).arg(&directory); + cmd.assert().code(predicate::eq(1)).failure(); + + let output = cmd.output().unwrap().stdout; + assert_diff_eq!( + output, + format!( + "--- {}\tTIMESTAMP\n+++ {}\tTIMESTAMP\n@@ -1 +1 @@\n-a\n+da\n", + a_path.display(), + da_path.display() + ) + ); + + Ok(()) +}