feat: ability to specify hook shell

[zaucy]

Defaults to no shell, meaning a direct execute occurs instead. This is a breaking change that would require the following to be added to cog.toml.

shell = { executable = "sh", args = ["-c"] }

I've updated the README to reflect what would need to be written if you did not specify a shell. As you can see not all hooks would require a shell and direct execution is just fine! (which is my main use case personally)

My last pr (#283) required too many changes to the tests. This one requires a lot less.

I didn't add CI for windows, but tests are currently failing on Windows due to how the run_cmd! macro is being used AFAICT.

[codecov]

Codecov Report

Merging #304 (16d1896) into main (17f98dc) will decrease coverage by 0.32%.
The diff coverage is 81.48%.

❗ Current head 16d1896 differs from pull request most recent head f20b3cc. Consider uploading reports for the commit f20b3cc to get more accurate results

@@            Coverage Diff             @@
##             main     #304      +/-   ##
==========================================
- Coverage   85.57%   85.26%   -0.32%     
==========================================
  Files          45       45              
  Lines        6380     6330      -50     
==========================================
- Hits         5460     5397      -63     
- Misses        920      933      +13     

Files	Coverage Δ
src/command/bump/mod.rs	84.98% <100.00%> (+0.05%)	⬆️
src/git/hook.rs	89.89% <ø> (-9.45%)	⬇️
src/hook/mod.rs	97.07% <88.37%> (-0.97%)	⬇️
src/settings/mod.rs	74.24% <44.44%> (-1.15%)	⬇️

... and 7 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[oknozor]

Hey @zaucy, thank you for the PR.
It would be great indeed to be able to specify the shell.
I think it would be preferable to provide a default implementation for the HookShell struct:

#[derive(Debug, Deserialize, Serialize, Eq, PartialEq)]
#[serde(default)]
pub struct HookShell {
    pub executable: String,
    pub args: Vec<String>,
}

impl Default for HookShell {
    fn default() -> Self {
        if cfg!(windows) {
            Self {
                executable: "cmd.exe".to_string(),
                args: vec!["/C".to_string()],
            } 
        } else {
            Self {
                executable: "sh".to_string(),
                args: vec!["-c".to_string()],
            }
        }
    }
}

Doing this would avoid a breaking change and setting the shell would be needed only to override the default.
I understand you need direct execution on Windows and I am not sure if this would work for you (I don't have a windows machine to test this). Would the default impl suffice here ?

[zaucy]

That would be better than having no support.

I would prefer direct execute for all platforms. I tend to work with Windows, Linux, and macOS and direct is more consistent for me. sh -c and cmd.exe /C have their own rules for escaping characters etc. and sh isn't guaranteed to be the same on all platforms.

Even tho I do believe direct execute is a better default, maybe we could add an option to do direct execute? Then sh / cmd.exe is the default like you suggested.

[cocogitto-bot]

❌ Found 3 compliant commit and 1 non-compliant commits in 81d54be.

Commit 16d1896 by @oknozor is not conform to the conventional commit specification :

• message: Merge branch 'main' into feat/hook-shell
• cause:

  Missing commit type separator `:`
  
  Caused by:
       --> 1:6
        |
      1 | Merge branch 'main' into feat/hook-shell
        |      ^---
        |
        = expected scope or type_separator
  

[oknozor]

Hey @zaucy can you squash those commits so the CI passes ?

[zaucy]

@oknozor done! Let me know if you'd like any changes.