rusty-hook icon indicating copy to clipboard operation
rusty-hook copied to clipboard

CLI changes for v0.12.0 not compatible with all earlier scripts

Open calebcartwright opened this issue 3 years ago • 2 comments

As part of the recent batch of changes and bump to the unreleased v0.12.0, the CLI args were updated a bit as part of the switch to Clap.

Current versions of the hook scripts run rusty-hook run --hook "${hookName}" "${gitParams}" providing the git params as a positional argument that's empty in cases of hooks that have no args. This is problematic with clap which doesn't allow explicitly empty values by default.

The impact of this would be users currently using rusty-hook in multiple projects would experience rusty-hook failures after updating one project (and thus updating the rusty-hook cli to v0.12.0) in all other projects.

To address, we need to update the clap interface to accept that arg for legacy/BC purposes

calebcartwright avatar Oct 16 '20 01:10 calebcartwright

I tried modifying the RustyHookOpts to this

Run {
        #[clap(long)]
        hook: String,
        // Changed from #[clap(name = "git args", raw(true))]
        #[clap(name = "git args")]
        args: Option<String>,
    },

This allows the cli to be called as in v0.11 (rusty-hook run --hook "${hookName}" "${gitParams}")

I then made a small script to test out the solution

# test.sh
echo -e "1: $1\n2: $2\n3: $3\n"

And finally I configured the rusty hooks

[hooks]
commit-msg = "./test.sh %rh! 'hardcoded arg'"
# rusty-hook run --hook "commit-msg" "git_params"
# Output:
# 1: git
# 2: params
# 3: hardcoded arg

# rusty-hook run --hook "commit-msg" ""
# Output:
# 1: hardcoded arg
# 2: 
# 3: 

# rusty-hook run --hook "commit-msg"
# Output:
# 1: %rh!
# 2: hardcoded arg
# 3: 

From how I interpret the results, I think this would solve the issue of broken backwards compatibility. I'd like to get some feedback on whether this is a workable solution or there are any considerations, that I have forgotten to account for?

I can implement the changes and probably add some tests for the argument parsing as well during the weekend if this is acceptable :slightly_smiling_face:

Scandiravian avatar Oct 12 '21 16:10 Scandiravian

Yeah that's the direction I'd started heading in as well, but ended up stepping back from it both due to some other priorities and because I'm increasingly reconsidering whether it really makes sense to utilize clap in this context.

Clap is absolutely fabulous and greatly simplifies and streamlines the process of developing rich CLI tools, but in hindsight I just feel like it's too heavy for rusty-hook which is largely behind the scenes and rarely, if ever, used directly by a user. As such I really feel like we should go back to getopts and optimize for compile time over feature richness.

If you're up for looking into that it would be fantastic and most appreciated :pray:

calebcartwright avatar Oct 13 '21 01:10 calebcartwright