diff --git a/Cargo.lock b/Cargo.lock index fee7f7f1c8..a33aac8874 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -791,6 +791,7 @@ dependencies = [ "scopeguard", "scopetime", "serde", + "serial_test", "simplelog", "struct-patch", "syntect", @@ -1425,9 +1426,9 @@ checksum = "dc375e1527247fe1a97d8b7156678dfe7c1af2fc075c9a4db3690ecd2a148068" [[package]] name = "proc-macro2" -version = "1.0.51" +version = "1.0.56" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5d727cae5b39d21da60fa540906919ad737832fe0b1c165da3a34d6548c849d6" +checksum = "2b63bdb0cd06f1f4dedf69b254734f9b45af66e4a031e42a7480257d9898b435" dependencies = [ "unicode-ident", ] @@ -1443,9 +1444,9 @@ dependencies = [ [[package]] name = "quote" -version = "1.0.23" +version = "1.0.26" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8856d8364d252a14d474036ea1358d63c9e6965c8e5c1885c18f73d70bff9c7b" +checksum = "4424af4bf778aae2051a77b60283332f386554255d722233d09fbfc7e30da2fc" dependencies = [ "proc-macro2", ] diff --git a/Cargo.toml b/Cargo.toml index ce69c1f16d..d7a0788a7a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -60,7 +60,9 @@ which = "4.4" pprof = { version = "0.11", features = ["flamegraph"], optional = true } [dev-dependencies] +asyncgit = { path = "asyncgit", features = ["test_utils"] } pretty_assertions = "1.3" +serial_test = "1.0.0" tempfile = "3.4" [badges] diff --git a/asyncgit/Cargo.toml b/asyncgit/Cargo.toml index 57d6a31094..1063d0a913 100644 --- a/asyncgit/Cargo.toml +++ b/asyncgit/Cargo.toml @@ -14,6 +14,7 @@ keywords = ["git"] [dependencies] crossbeam-channel = "0.5" easy-cast = "0.5" +env_logger = "0.10" git2 = "0.17" log = "0.4" # git2 = { path = "../../extern/git2-rs", features = ["vendored-openssl"]} @@ -24,6 +25,7 @@ rayon-core = "1.11" scopetime = { path = "../scopetime", version = "0.1" } serde = { version = "1.0", features = ["derive"] } shellexpand = "3.1" +tempfile = "3.4" thiserror = "1.0" unicode-truncate = "0.2.0" url = "2.3" @@ -33,9 +35,9 @@ env_logger = "0.10" invalidstring = { path = "../invalidstring", version = "0.1" } pretty_assertions = "1.3" serial_test = "1.0" -tempfile = "3.4" [features] default = ["trace-libgit"] trace-libgit = [] vendor-openssl = ["openssl-sys"] +test_utils = [] diff --git a/asyncgit/src/lib.rs b/asyncgit/src/lib.rs index ecf85e0c71..f50aa66857 100644 --- a/asyncgit/src/lib.rs +++ b/asyncgit/src/lib.rs @@ -23,7 +23,7 @@ // #![deny(clippy::expect_used)] //TODO: consider cleaning some up and allow specific places #![allow(clippy::significant_drop_tightening)] - +#![allow(clippy::multiple_crate_versions)] pub mod asyncjob; mod blame; mod branches; diff --git a/asyncgit/src/sync/mod.rs b/asyncgit/src/sync/mod.rs index a7bf7d64ed..61aa4ba847 100644 --- a/asyncgit/src/sync/mod.rs +++ b/asyncgit/src/sync/mod.rs @@ -100,8 +100,19 @@ pub use utils::{ pub use git2::ResetType; -#[cfg(test)] -mod tests { +#[cfg(feature = "test_utils")] +/// test utilities - exported now +// see https://github.com/rust-lang/cargo/issues/8379 +pub mod tests { + // these are now not under 'test' so they get clippied with 'all-features' + // we dont care about tests that panic + #![allow(clippy::unwrap_used, clippy::missing_panics_doc)] + // minor niggles + #![allow(clippy::nursery)] + // this clippy is confused by the name 'read' + // should probably be changed to read_into + #![allow(clippy::read_zero_byte_vec)] + use super::{ commit, repository::repo, @@ -278,7 +289,7 @@ mod tests { // init log fn init_log() { - let _ = env_logger::builder() + let _b = env_logger::builder() .is_test(true) .filter_level(log::LevelFilter::Trace) .try_init(); diff --git a/deny.toml b/deny.toml index 71507058b8..45cce671d3 100644 --- a/deny.toml +++ b/deny.toml @@ -23,4 +23,5 @@ multiple-versions = "deny" skip-tree = [ { name = "windows-sys" }, { name = "hermit-abi" } + ] diff --git a/src/components/commit.rs b/src/components/commit.rs index 4cea6f4b90..875d0a35e9 100644 --- a/src/components/commit.rs +++ b/src/components/commit.rs @@ -531,7 +531,6 @@ impl Component for CommitComponent { self.input.set_text(msg); self.commit_msg_history_idx += 1; } - } else { } // stop key event propagation return Ok(EventState::Consumed); diff --git a/src/components/externaleditor.rs b/src/components/externaleditor.rs index fd9bd8b65a..8bfce54f9d 100644 --- a/src/components/externaleditor.rs +++ b/src/components/externaleditor.rs @@ -64,9 +64,12 @@ impl ExternalEditorComponent { bail!("file not found: {:?}", path); } - io::stdout().execute(LeaveAlternateScreen)?; - defer! { - io::stdout().execute(EnterAlternateScreen).expect("reset terminal"); + // so that the output is not messed up when running tests + if cfg!(not(test)) { + io::stdout().execute(LeaveAlternateScreen)?; + defer! { + io::stdout().execute(EnterAlternateScreen).expect("reset terminal"); + } } let environment_options = ["GIT_EDITOR", "VISUAL", "EDITOR"]; @@ -80,6 +83,7 @@ impl ExternalEditorComponent { .or_else(|| env::var(environment_options[2]).ok()) .unwrap_or_else(|| String::from("vi")); + log::trace!("external editor:{}", editor); // TODO: proper handling arguments containing whitespaces // This does not do the right thing if the input is `editor --something "with spaces"` @@ -112,11 +116,53 @@ impl ExternalEditorComponent { args.push(path.as_os_str()); - Command::new(command.clone()) - .current_dir(work_dir) - .args(args) - .status() - .map_err(|e| anyhow!("\"{}\": {}", command, e))?; + let exec_result = Command::new(&command) + .current_dir(&work_dir) + .args(&args) + .status(); + + if cfg!(windows) { + // if command failed to run on windows retry as a batch file (.bat, .cmd,...) + if exec_result.is_err() { + /* here args contains the arguments pulled from the configured editor string + "myeditor --color blue" -> + args[0] = "--color" + args[1] = "blue" + + now insert before these + "/C" + "myeditor" + */ + + args.insert(0, OsStr::new("/C")); + args.insert(1, OsStr::new(&command)); + let exec_result2 = Command::new("cmd") + .current_dir(work_dir) + .args(args) + .status(); + + match exec_result2 { + // failed to start (unlikely as cmd would have to be missing) + Err(e) => bail!("\"{}\": {}", command, e), + + // ran, did it complete OK? + Ok(stat) => { + // no result is treated as arbitrary failure code of 99 + let code = stat.code().unwrap_or(99); + if code != 0 { + bail!( + "\"{}\": cmd.exe returned {}", + command, + code + ) + } + } + }; + } + } else { + exec_result + .map_err(|e| anyhow!("\"{}\": {}", command, e))?; + } Ok(()) } @@ -192,3 +238,180 @@ impl Component for ExternalEditorComponent { Ok(()) } } +#[cfg(test)] +mod tests { + use crate::components::ExternalEditorComponent; + use anyhow::Result; + use asyncgit::sync::tests::repo_init; + #[cfg(windows)] + use asyncgit::sync::utils::read_file; + use asyncgit::sync::RepoPath; + use serial_test::serial; + use std::env; + use std::fs::File; + use std::io::Write; + use tempfile::TempDir; + + fn write_temp_file( + td: &TempDir, + file: &str, + content: &str, + ) -> Result<()> { + let binding = td.path().join(file); + let file_path = binding.to_str().unwrap(); + let mut file = File::create(file_path)?; + file.write_all(content.as_bytes())?; + Ok(()) + } + const TEST_FILE_NAME: &str = "test1.txt"; + const TEST_FILE_DATA: &str = "test file data"; + + fn setup_repo() -> (TempDir, RepoPath) { + let (td, repo) = repo_init().unwrap(); + let root = repo.path().parent().unwrap(); + let repo_path: RepoPath = + root.as_os_str().to_str().unwrap().into(); + + // create a dummy file to operate on + let txt = String::from(TEST_FILE_DATA); + write_temp_file(&td, TEST_FILE_NAME, &txt).unwrap(); + (td, repo_path) + } + + // these have to de serialzied because they set env variables to control which editor to use + + #[test] + #[serial] + fn editor_missing() { + let (td, repo_path) = setup_repo(); + let target_file_path = td.path().join(TEST_FILE_NAME); + env::set_var("GIT_EDITOR", "i_doubt_this_exists"); + let foo = ExternalEditorComponent::open_file_in_editor( + &repo_path, + &target_file_path, + ); + assert!(foo.is_err()); + } + + #[cfg(windows)] + mod win_test { + use super::*; + #[test] + #[serial] + fn editor_is_bat() { + let (td, repo_path) = setup_repo(); + let target_file_path = td.path().join(TEST_FILE_NAME); + env::set_var("GIT_EDITOR", "testbat"); + let bat = String::from("@echo off\ntype %1 >made.txt"); + write_temp_file(&td, "testbat.bat", &bat).unwrap(); + + let runit = ExternalEditorComponent::open_file_in_editor( + &repo_path, + &target_file_path, + ); + assert!(runit.is_ok()); + + let echo_file = td.path().join("made.txt"); + let read_text = read_file(echo_file.as_path()).unwrap(); + + assert_eq!( + read_text.lines().next(), + Some(TEST_FILE_DATA) + ); + } + #[test] + #[serial] + fn editor_is_bat_ext() { + let (td, repo_path) = setup_repo(); + let target_file_path = td.path().join(TEST_FILE_NAME); + + env::set_var("GIT_EDITOR", "testbat.bat"); + + let bat = String::from("@echo off\ntype %1 >made.txt"); + write_temp_file(&td, "testbat.bat", &bat).unwrap(); + + let runit = ExternalEditorComponent::open_file_in_editor( + &repo_path, + &target_file_path, + ); + assert!(runit.is_ok()); + + let echo_file = td.path().join("made.txt"); + let read_text = read_file(echo_file.as_path()).unwrap(); + assert_eq!( + read_text.lines().next(), + Some(TEST_FILE_DATA) + ); + } + #[test] + #[serial] + fn editor_is_bat_noext_arg() { + let (td, repo_path) = setup_repo(); + let target_file_path = td.path().join(TEST_FILE_NAME); + + env::set_var("GIT_EDITOR", "testbat --foo"); + + let bat = String::from("@echo off\ntype %2 >made.txt"); + write_temp_file(&td, "testbat.bat", &bat).unwrap(); + + let runit = ExternalEditorComponent::open_file_in_editor( + &repo_path, + &target_file_path, + ); + assert!(runit.is_ok()); + + let echo_file = td.path().join("made.txt"); + let read_text = read_file(echo_file.as_path()).unwrap(); + assert_eq!( + read_text.lines().next(), + Some(TEST_FILE_DATA) + ); + } + #[test] + #[serial] + fn editor_is_cmd() { + let (td, repo_path) = setup_repo(); + let target_file_path = td.path().join(TEST_FILE_NAME); + env::set_var("GIT_EDITOR", "testcmd"); + let bat = String::from("@echo off\ntype %1 >made.txt"); + write_temp_file(&td, "testcmd.cmd", &bat).unwrap(); + + let runit = ExternalEditorComponent::open_file_in_editor( + &repo_path, + &target_file_path, + ); + assert!(runit.is_ok()); + + let echo_file = td.path().join("made.txt"); + let read_text = read_file(echo_file.as_path()).unwrap(); + + assert_eq!( + read_text.lines().next(), + Some(TEST_FILE_DATA) + ); + } + #[test] + #[serial] + fn editor_is_cmd_arg() { + let (td, repo_path) = setup_repo(); + let target_file_path = td.path().join(TEST_FILE_NAME); + env::set_var("GIT_EDITOR", "testcmd --bar"); + let bat = String::from("@echo off\ntype %2 >made.txt"); + write_temp_file(&td, "testcmd.cmd", &bat).unwrap(); + + let runit = ExternalEditorComponent::open_file_in_editor( + &repo_path, + &target_file_path, + ); + assert!(runit.is_ok()); + + let echo_file = td.path().join("made.txt"); + let read_text = read_file(echo_file.as_path()).unwrap(); + + assert_eq!( + read_text.lines().next(), + Some(TEST_FILE_DATA) + ); + } + } +} diff --git a/src/components/help.rs b/src/components/help.rs index f2c45e3d64..e6a1c8eb30 100644 --- a/src/components/help.rs +++ b/src/components/help.rs @@ -135,7 +135,6 @@ impl Component for HelpComponent { self.move_selection(true); } else if key_match(e, self.key_config.keys.move_up) { self.move_selection(false); - } else { } }