Repository: swellaby/rusty-hook (rust)

Issue:
Option to skip/control which hook files are created during init
## Description
<!-- Clearly describe what you are suggesting/requesting. -->
Add a feature that allows for controlling which hook files are instantiated

## Value
<!-- Clearly and succinctly explain the benefits of what you are suggesting/requesting. -->
Currently, the repo initialization process (triggered with either `rusty-hook init` or using compiling with rusty-hook as a dev dep) adds a hook file for all the supportable client side hooks to easily support user configurability. However, there are probably some advanced user scenarios where they'd like to control which hook files are/aren't created.

For example, there are other Rust tools that support creating their own hook files (for example [git-journal](https://github.com/saschagrunert/git-journal#commit-message-preparation-and-verification) which supports commit message hooks) and we should support users that may want to use rusty-hook in conjunction with those others. There's also the potential for some users to want really fine grained control (create the `pre-commit` file but none of the others)

## Implementation Thoughts
I think the most common usage of this feature would be a skipping of one or two hook files (like `commit-msg` and _perhaps_ `prepare-commit-msg`) so my initial inclination would be to support this via a new config option skip list, for example:

```toml
hook_file_skip_list = [
  "commit-msg"
]
```

If there's sufficient demand down the road to support an opt-in type feature over an opt out, we could add an additional config at that time for an include/allow list and figure out precedence if and when we get there (implementing this as a skip/opt-out won't preclude doing more work in the future).

Currently, the init process that sets up the hook files in the repo just iterates through the full list of hooks:
https://github.com/swellaby/rusty-hook/blob/39fbca3b90c6f37bfdacd23bc4d25b9a0da554f0/src/hooks.rs#L75-L80

(side note `create_hook_files` has a lot of duplication that could be consolidated into a separate closure or function)

That could be updated pretty easily to accept a list of hook files to skip, and then use the subset of `HOOK_NAMES` that weren't skipped (I suppose technically the disjunctive union provided the skip list doesn't have any junk entries)

Would then need to shuffle up the init order a bit to run the config creation check first, and then check for the `hook_file_skip_list` presence/value to pass along to `setup_hooks` function flow
https://github.com/swellaby/rusty-hook/blob/39fbca3b90c6f37bfdacd23bc4d25b9a0da554f0/src/rusty_hook.rs#L10-L36


Pull request:
:umbrella:  Add option hook files to skip
## Changes
<!-- Summary of changes included in this PR -->
- Filter Hook files by option
- new command with skip e.g. `cargo run init --skip-hook-list="commit-msg,push-to-checkout"`

## Related Issues
<!-- List any related/impacted issues -->
- Closes #119

## Uncovered
- Update documentation


Patch:
diff --git a/build.rs b/build.rs
index ac11a7b..c40b15d 100644
--- a/build.rs
+++ b/build.rs
@@ -2,8 +2,8 @@
 #[path = "src/rusty_hook.rs"]
 mod rusty_hook;
 
-use std::env;
 use std::process::exit;
+use std::{env, vec};
 
 fn main() {
     if ci_info::is_ci() {
@@ -16,6 +16,7 @@ fn main() {
         nias::get_file_writer(),
         nias::get_file_existence_checker(),
         Some(&target_directory),
+        vec![],
     ) {
         println!(
             "Fatal error encountered during initialization. Details: {}",
diff --git a/src/git.rs b/src/git.rs
index 73441dd..0b2559d 100644
--- a/src/git.rs
+++ b/src/git.rs
@@ -47,6 +47,7 @@ pub fn setup_hooks<F, G>(
     run_command: F,
     write_file: G,
     root_directory_path: &str,
+    hook_file_skip_list: &Vec<&str>,
 ) -> Result<(), String>
 where
     F: Fn(
@@ -61,7 +62,12 @@ where
         Ok(Some(path)) => path,
         _ => return Err(String::from("Failure determining git hooks directory")),
     };
-    hooks::create_hook_files(write_file, &root_directory_path, &hooks_directory)
+    hooks::create_hook_files(
+        write_file,
+        &root_directory_path,
+        &hooks_directory,
+        hook_file_skip_list,
+    )
 }
 
 #[cfg(test)]
diff --git a/src/hooks.rs b/src/hooks.rs
index 79a757e..1089194 100644
--- a/src/hooks.rs
+++ b/src/hooks.rs
@@ -67,12 +67,16 @@ pub fn create_hook_files<F>(
     write_file: F,
     root_directory_path: &str,
     hooks_directory: &str,
+    hook_file_skip_list: &Vec<&str>,
 ) -> Result<(), String>
 where
     F: Fn(&str, &str, bool) -> Result<(), String>,
 {
     let hook_file_contents = get_hook_file_contents();
-    for hook in HOOK_NAMES.iter() {
+    for hook in HOOK_NAMES
+        .iter()
+        .filter(|h| !hook_file_skip_list.contains(h))
+    {
         let path = get_file_path(root_directory_path, hooks_directory, hook);
         if write_file(&path, &hook_file_contents, true).is_err() {
             return Err(String::from(HOOK_CREATION_ERROR));
diff --git a/src/main.rs b/src/main.rs
index d0878ed..d0788de 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -10,7 +10,10 @@ mod rusty_hook;
 enum RustyHookOpts {
     /// Initialize rusty-hook's git hooks in the current directory.
     #[clap(author, version)]
-    Init,
+    Init {
+        #[clap(long)]
+        skip_hook_list: Option<String>,
+    },
     /// Print the current version of rusty-hook.
     #[clap(author, version, alias = "-v")]
     Version,
@@ -25,16 +28,21 @@ enum RustyHookOpts {
     },
 }
 
-fn init() {
+fn init(skip_hook_list: Option<String>) {
     if ci_info::is_ci() {
         println!("[rusty-hook] CI Environment detected. Skipping hook install");
         exit(0);
     }
 
+    let skip_hook_list = skip_hook_list
+        .as_deref()
+        .map_or(vec![], |s| s.split(",").collect());
+
     if let Err(err) = rusty_hook::init(
         nias::get_command_runner(),
         nias::get_file_writer(),
         nias::get_file_existence_checker(),
+        skip_hook_list,
     ) {
         eprintln!(
             "[rusty-hook] Fatal error encountered during initialization. Details: {}",
@@ -69,7 +77,7 @@ fn run(hook: String, args: Option<String>) {
 fn main() {
     let opts = RustyHookOpts::parse();
     match opts {
-        RustyHookOpts::Init => init(),
+        RustyHookOpts::Init { skip_hook_list } => init(skip_hook_list),
         RustyHookOpts::Version => println!(env!("CARGO_PKG_VERSION")),
         RustyHookOpts::Run { hook, args } => run(hook, args),
     }
diff --git a/src/rusty_hook.rs b/src/rusty_hook.rs
index 4365f9d..893b5f7 100644
--- a/src/rusty_hook.rs
+++ b/src/rusty_hook.rs
@@ -13,6 +13,7 @@ pub fn init_directory<F, G, H>(
     write_file: G,
     file_exists: H,
     target_directory: Option<&str>,
+    hook_file_skip_list: Vec<&str>,
 ) -> Result<(), String>
 where
     F: Fn(
@@ -28,8 +29,14 @@ where
         Ok(Some(path)) => path,
         _ => return Err(String::from("Failure determining git repo root directory")),
     };
-
-    if git::setup_hooks(&run_command, &write_file, &root_directory_path).is_err() {
+    if git::setup_hooks(
+        &run_command,
+        &write_file,
+        &root_directory_path,
+        &hook_file_skip_list,
+    )
+    .is_err()
+    {
         return Err(String::from("Unable to create git hooks"));
     };
 
@@ -41,7 +48,12 @@ where
     Ok(())
 }
 
-pub fn init<F, G, H>(run_command: F, write_file: G, file_exists: H) -> Result<(), String>
+pub fn init<F, G, H>(
+    run_command: F,
+    write_file: G,
+    file_exists: H,
+    hook_file_skip_list: Vec<&str>,
+) -> Result<(), String>
 where
     F: Fn(
         &str,
@@ -52,7 +64,13 @@ where
     G: Fn(&str, &str, bool) -> Result<(), String>,
     H: Fn(&str) -> Result<bool, ()>,
 {
-    init_directory(&run_command, &write_file, &file_exists, None)
+    init_directory(
+        &run_command,
+        &write_file,
+        &file_exists,
+        None,
+        hook_file_skip_list,
+    )
 }
 
 pub fn run<F, G, H, I>(


Test patch:
diff --git a/src/git_test.rs b/src/git_test.rs
index 49f7ec9..dd8b950 100644
--- a/src/git_test.rs
+++ b/src/git_test.rs
@@ -73,7 +73,7 @@ mod setup_hooks_tests {
         let exp_err = "Failure determining git hooks directory";
         let run_command = build_simple_command_runner(Err(None));
         let write_file = |_path: &str, _contents: &str, _x: bool| Ok(());
-        let result = setup_hooks(run_command, write_file, "");
+        let result = setup_hooks(run_command, write_file, "", &vec![]);
         assert_eq!(result, Err(String::from(exp_err)));
     }
 
@@ -82,7 +82,7 @@ mod setup_hooks_tests {
         let run_command =
             build_simple_command_runner(Ok(Some(String::from("/usr/repos/foo/.git/hooks"))));
         let write_file = |_path: &str, _contents: &str, _x: bool| Err(String::from(""));
-        let result = setup_hooks(run_command, write_file, "");
+        let result = setup_hooks(run_command, write_file, "", &vec![]);
         assert_eq!(result, Err(String::from(hooks::HOOK_CREATION_ERROR)));
     }
 
@@ -92,7 +92,7 @@ mod setup_hooks_tests {
         let git_hooks = ".git/hooks";
         let run_command = build_simple_command_runner(Ok(Some(String::from(git_hooks))));
         let write_file = |_p: &str, _c: &str, _x: bool| Ok(());
-        let result = setup_hooks(run_command, write_file, root_dir);
+        let result = setup_hooks(run_command, write_file, root_dir, &vec![]);
         assert_eq!(result, Ok(()));
     }
 }
diff --git a/src/hooks_test.rs b/src/hooks_test.rs
index 656f697..4050c9b 100644
--- a/src/hooks_test.rs
+++ b/src/hooks_test.rs
@@ -28,6 +28,7 @@ const EXP_HOOK_NAMES: [&str; 19] = [
     "post-rewrite",
     "sendemail-validate",
 ];
+const EXP_SKIPPED_HOOK: &str = "commit-msg";
 
 const EXP_CLI_SCRIPT_NAME: &str = "cli.sh";
 const EXP_SEMVER_SCRIPT_NAME: &str = "semver.sh";
@@ -123,7 +124,7 @@ mod create_hook_files_tests {
                 _ => Err(String::from("")),
             }
         };
-        let result = create_hook_files(write_file, "", "");
+        let result = create_hook_files(write_file, "", "", &vec![]);
         assert_eq!(result, Err(String::from(EXP_HOOK_CREATION_ERROR)));
     }
 
@@ -137,7 +138,7 @@ mod create_hook_files_tests {
                 _ => Ok(()),
             }
         };
-        let result = create_hook_files(write_file, "", "");
+        let result = create_hook_files(write_file, "", "", &vec![]);
         assert_eq!(result, Err(String::from(EXP_HOOK_CREATION_ERROR)));
     }
 
@@ -151,7 +152,7 @@ mod create_hook_files_tests {
                 _ => Ok(()),
             }
         };
-        let result = create_hook_files(write_file, "", "");
+        let result = create_hook_files(write_file, "", "", &vec![]);
         assert_eq!(result, Err(String::from(EXP_HOOK_CREATION_ERROR)));
     }
 
@@ -185,7 +186,42 @@ mod create_hook_files_tests {
             assert_eq!(true, make_executable);
             Ok(())
         };
-        let result = create_hook_files(write_file, root_dir, git_hooks);
+        let result = create_hook_files(write_file, root_dir, git_hooks, &vec![]);
+        assert_eq!(result, Ok(()));
+    }
+
+    #[test]
+    fn does_not_create_skipped_hook() {
+        let root_dir = "/usr/repos/foo";
+        let git_hooks = ".git/hooks";
+        let exp_contents = get_expected_hook_file_contents();
+        let exp_cli_contents = get_expected_cli_script_file_contents();
+        let exp_cli_path = &format!("{}/{}/{}", root_dir, git_hooks, EXP_CLI_SCRIPT_NAME);
+        let exp_semver_path = &format!("{}/{}/{}", root_dir, git_hooks, EXP_SEMVER_SCRIPT_NAME);
+        let exp_semver_contents = get_expected_semver_script_file_contents();
+        let write_file = |path: &str, contents: &str, make_executable: bool| {
+            let file_name = &&path[(path.rfind('/').unwrap() + 1)..];
+            match *file_name {
+                EXP_CLI_SCRIPT_NAME => {
+                    assert_eq!(exp_cli_path, path);
+                    assert_eq!(exp_cli_contents, contents);
+                }
+                EXP_SEMVER_SCRIPT_NAME => {
+                    assert_eq!(exp_semver_path, path);
+                    assert_eq!(exp_semver_contents, contents);
+                }
+                EXP_SKIPPED_HOOK => panic!("Should not create skipped hook"),
+                _ => {
+                    let exp_hook = EXP_HOOK_NAMES.iter().find(|&n| n == file_name).unwrap();
+                    let exp_path = &format!("{}/{}/{}", root_dir, git_hooks, exp_hook);
+                    assert_eq!(exp_path, path);
+                    assert_eq!(exp_contents, contents);
+                }
+            }
+            assert_eq!(true, make_executable);
+            Ok(())
+        };
+        let result = create_hook_files(write_file, root_dir, git_hooks, &vec![EXP_SKIPPED_HOOK]);
         assert_eq!(result, Ok(()));
     }
 }
diff --git a/src/rusty_hook_test.rs b/src/rusty_hook_test.rs
index 7834401..6b21cc6 100644
--- a/src/rusty_hook_test.rs
+++ b/src/rusty_hook_test.rs
@@ -39,7 +39,7 @@ mod init_directory_tests {
             panic!("Should not get here");
         };
         let file_exists = |_path: &str| panic!("Should not get here");
-        let result = init(run_command, write_file, file_exists);
+        let result = init(run_command, write_file, file_exists, vec![]);
         assert_eq!(result, Err(String::from(exp_err)));
     }
 
@@ -48,7 +48,7 @@ mod init_directory_tests {
         let run_command = build_simple_command_runner(Ok(Some(String::from(""))));
         let write_file = |_file_path: &str, _contents: &str, _x: bool| Err(String::from(""));
         let file_exists = |_path: &str| panic!("Should not get here");
-        let result = init(run_command, write_file, file_exists);
+        let result = init(run_command, write_file, file_exists, vec![]);
         assert_eq!(result, Err(String::from("Unable to create git hooks")));
     }
 
@@ -57,7 +57,7 @@ mod init_directory_tests {
         let run_command = build_simple_command_runner(Ok(Some(String::from(""))));
         let write_file = |_file_path: &str, _contents: &str, _x: bool| Ok(());
         let file_exists = |_path: &str| Err(());
-        let result = init(run_command, write_file, file_exists);
+        let result = init(run_command, write_file, file_exists, vec![]);
         assert_eq!(result, Err(String::from("Unable to create config file")));
     }
 
@@ -66,7 +66,7 @@ mod init_directory_tests {
         let run_command = build_simple_command_runner(Ok(Some(String::from(""))));
         let write_file = |_file_path: &str, _contents: &str, _x: bool| Ok(());
         let file_exists = |_path: &str| Ok(false);
-        let result = init(run_command, write_file, file_exists);
+        let result = init(run_command, write_file, file_exists, vec![]);
         assert_eq!(result, Ok(()));
     }
 }
@@ -91,7 +91,7 @@ mod init_tests {
         };
         let write_file = |_file_path: &str, _contents: &str, _x: bool| Ok(());
         let file_exists = |_path: &str| Ok(false);
-        let result = init(run_command, write_file, file_exists);
+        let result = init(run_command, write_file, file_exists, vec![]);
         assert_eq!(result, Ok(()));
     }
 }