Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
calebcartwright opened this issue Oct 16, 2020 · 2 comments · May be fixed by #169
Open

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

calebcartwright opened this issue Oct 16, 2020 · 2 comments · May be fixed by #169
Assignees
Projects
Milestone

Comments

@calebcartwright
Copy link
Member

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 calebcartwright self-assigned this Oct 16, 2020
@calebcartwright calebcartwright added this to To do in 1.0 Release via automation Oct 16, 2020
@calebcartwright calebcartwright added this to the 1.x Release milestone Oct 16, 2020
@Scandiravian
Copy link
Contributor

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 🙂

@calebcartwright
Copy link
Member Author

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 🙏

@Scandiravian Scandiravian linked a pull request Oct 17, 2021 that will close this issue
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
1.0 Release
  
To do
Development

Successfully merging a pull request may close this issue.

2 participants