diff --git a/Cargo.lock b/Cargo.lock index 1b7eb76021..e1c5901631 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -362,7 +362,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6d2301688392eb071b0bf1a37be05c469d3cc4dbbd95df672fe28ab021e6a096" dependencies = [ "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -389,7 +389,7 @@ dependencies = [ "proc-macro2", "quote", "scratch", - "syn", + "syn 1.0.107", ] [[package]] @@ -406,7 +406,7 @@ checksum = "ebf883b7aacd7b2aeb2a7b338648ee19f57c140d4ee8e52c68979c6b2f7f2263" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -741,7 +741,7 @@ dependencies = [ "proc-macro-hack", "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -790,6 +790,7 @@ dependencies = [ "scopeguard", "scopetime", "serde", + "serial_test", "simplelog", "struct-patch", "syntect", @@ -1402,7 +1403,7 @@ dependencies = [ "proc-macro-error-attr", "proc-macro2", "quote", - "syn", + "syn 1.0.107", "version_check", ] @@ -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", ] @@ -1590,7 +1591,7 @@ checksum = "d7e29c4601e36bcec74a223228dce795f4cd3616341a4af93520ca1a837c087d" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -1606,9 +1607,9 @@ dependencies = [ [[package]] name = "serial_test" -version = "1.0.0" +version = "2.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "538c30747ae860d6fb88330addbbd3e0ddbe46d662d032855596d8a8ca260611" +checksum = "0e56dd856803e253c8f298af3f4d7eb0ae5e23a737252cd90bb4f3b435033b2d" dependencies = [ "dashmap", "futures", @@ -1620,13 +1621,13 @@ dependencies = [ [[package]] name = "serial_test_derive" -version = "1.0.0" +version = "2.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "079a83df15f85d89a68d64ae1238f142f172b1fa915d0d76b26a7cba1b659a69" +checksum = "91d129178576168c589c9ec973feedf7d3126c01ac2bf08795109aa35b69fb8f" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.15", ] [[package]] @@ -1753,7 +1754,7 @@ dependencies = [ "proc-macro-error", "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -1790,6 +1791,17 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "syn" +version = "2.0.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a34fcf3e8b60f57e6a14301a2e916d323af98b0ea63c599441eec8558660c822" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + [[package]] name = "syntect" version = "5.0.0" @@ -1872,7 +1884,7 @@ checksum = "5420d42e90af0c38c3290abcca25b9b3bdf379fc9f55c528f53a269d9c9a267e" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -2059,7 +2071,7 @@ dependencies = [ "once_cell", "proc-macro2", "quote", - "syn", + "syn 1.0.107", "wasm-bindgen-shared", ] @@ -2081,7 +2093,7 @@ checksum = "2aff81306fcac3c7515ad4e177f521b5c9a15f2b08f4e32d823066102f35a5f6" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 1.0.107", "wasm-bindgen-backend", "wasm-bindgen-shared", ] diff --git a/Cargo.toml b/Cargo.toml index 5d893d9abc..66da1a4f36 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -61,7 +61,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 = "2.0.0" tempfile = "3.4" [badges] @@ -94,3 +96,4 @@ codegen-units = 1 # makes their debug profile slow [profile.dev.package."tui"] opt-level = 3 + diff --git a/asyncgit/Cargo.toml b/asyncgit/Cargo.toml index 57d6a31094..a3ccdb7127 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,18 +25,18 @@ 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" [dev-dependencies] -env_logger = "0.10" invalidstring = { path = "../invalidstring", version = "0.1" } pretty_assertions = "1.3" -serial_test = "1.0" -tempfile = "3.4" +serial_test = "2.0.0" [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..26195f423a 100644 --- a/asyncgit/src/lib.rs +++ b/asyncgit/src/lib.rs @@ -23,6 +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; diff --git a/asyncgit/src/sync/mod.rs b/asyncgit/src/sync/mod.rs index a7bf7d64ed..56c0b65cf4 100644 --- a/asyncgit/src/sync/mod.rs +++ b/asyncgit/src/sync/mod.rs @@ -100,8 +100,18 @@ 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 +288,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..5abca03057 100644 --- a/deny.toml +++ b/deny.toml @@ -22,5 +22,6 @@ version = "1.0.3" multiple-versions = "deny" skip-tree = [ { name = "windows-sys" }, - { name = "hermit-abi" } + { name = "hermit-abi" }, + { name = "syn" } ] diff --git a/src/components/externaleditor.rs b/src/components/externaleditor.rs index 3c9c918602..08f38b2aca 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) + ); + } + } +}