Support additional flags when running rebase

[danielkleinstein]

The motivation for this request is similar to #48 - we want to run precommit hooks when rebasing with git absorb.

There is a workaround for running precommit hooks when rebasing, outlined by this guide - the solution is to pass in -x "..." to git rebase.

When running git absorb ... --and-rebase we have no control over what flags to pass in to git rebase.

Is it possible to expand git absorb's parameters to receive something like a "--rebase-params" parameter that will be given to git rebase? This will enable a powerful alias like git absorb --and-rebase --base origin/main --rebase-params "...".

P.S. huge thanks for this great tool!! It's a major productivity boon for us

[tummychow]

see #66 (comment) for approaches i would consider acceptable. notably i would not accept a pr for --rebase-params, because that would require implementing a shell string splitter in git-absorb to break the string into individual arguments.

although frankly, if you're passing --base yourself, i see no reason to implement any of this. just... run git rebase yourself. there's no magic here, it's just git rebase -i --autosquash, and the latter can be enabled by default. the only reason --and-rebase exists is because it adds the correct base for you.

[danielkleinstein]

Thanks for the prompt reply!

The idea behind the request was indeed in line with what you wrote in #66 (comment):

use -- to pass arbitrary flags to git-rebase, again with the understanding that validating them is entirely the user's responsibility. eg git absorb --and-rebase -- --update-refs .... this only works because -- tells us not to parse the flags intended for git-rebase. if you intermingle the git-rebase flags with the git-absorb flags, you're giving your cli parser crate a very unpleasant problem to solve.

---

Regarding

notably i would not accept a pr for --rebase-params, because that would require implementing a shell string splitter in git-absorb to break the string into individual arguments.

Is it not compatible with git absorb's implementation to treat the parameter after "--rebase-params" as a single parameter? i.e. it'd be on the user to wrap multiple arguments in quotes.

This would be consistent with the behavior of many other CLI tools that need to pass in params to external CLI tools.

[tummychow]

Is it not compatible with git absorb's implementation to treat the parameter after "--rebase-params" as a single parameter? i.e. it'd be on the user to wrap multiple arguments in quotes.

This'd be consistent with the behavior of many other CLI tools that need to pass in params to external CLI tools.

and if i was in charge of those cli tools then they wouldn't do it either. the reason i'm opposed to it is because it's an implementation nightmare. splitting a single string into multiple strings is not a trivial problem, as shells themselves demonstrate. consider:

# i want to run:
git rebase -x "make test && echo \"all clear\""
# now as a git absorb command:
git absorb --and-rebase --rebase-params '-x "make test && echo \"all clear\""'

have fun parsing the content of -x "make test && echo \"all clear\"" (e: heck, i even forgot the backslashes the first time around - if you think this is easy then help yourself!). i strongly prefer using -- because that way you only have one layer of string splitting - in the shell prior to the command being invoked.

[danielkleinstein]

To clarify what I meant - you wouldn't need to parse the contents of the string - you would treat it as a single argument.

As a concrete example using the code from clap's readme:

~/t/t/t/debug (master)> ./tmp --name=Hello
Hello

~/t/t/t/debug (master)> ./tmp --name=Hello hello
error: unexpected argument 'hello' found

Usage: tmp [OPTIONS] --name <NAME>

For more information, try '--help'.

~/t/t/t/debug (master) [2]> ./tmp --name="-x \"make test && echo \"all clear\"\""
-x "make test && echo "all clear""

No parsing required - I'm proposing (in line with many CLI tools) that the onus be on the user to make sure their parameter is a single argument.

[tummychow]

but in your own example, the resulting behavior is incorrect:

~/t/t/t/debug (master) [2]> ./tmp --name="-x \"make test && echo \"all clear\"\""
-x "make test && echo "all clear""

this needs to be split into two arguments, -x and the stuff that comes afterward. you would have to pass:

# not to mention, passing --rebase-params -x is almost certainly going to confuse a cli parser
# you would have to pass it in the single argument form --rebase-params=-x
git absorb --and-rebase --rebase-params '-x' --rebase-params '"make test && echo \"all clear\""'

which i think we can agree has no redeeming qualities compared to:

git absorb --and-rebase -- -x "make test && echo \"all clear\""

[danielkleinstein]

Oh I see what you mean - it needs to be split into two arguments so it can be passed into git rebase as two arguments.

Mmm indeed the complexity probably isn't worth it.

[danielkleinstein]

Would you be receptive to a ---based PR?

[tummychow]

yep. in fact, as i mentioned in the linked comment, i think it is the only reasonable approach to achieve what you're asking for. luckily git absorb has no need for positional parameters (and hopefully stays that way...), so we can always assume that the -- arguments are for git rebase (and can return an error if --and-rebase was not passed, etc).