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

pre-push: fix stdin line splitting when <local ref> has whitespace #2345

Merged
merged 1 commit into from Apr 14, 2022

Conversation

wwade
Copy link
Contributor

@wwade wwade commented Apr 14, 2022

From the pre-push.sample file:

Information about the commits which are being pushed is supplied as
lines to the standard input in the form:

<local ref> <local sha1> <remote ref> <remote sha1>

When <local ref> is not simply a branch name, but a more general
ref (see git-rev-parse(1)), it could contain whitespace, and that
breaks the split() call that expected only 3 spaces in the line.

Changed to use rsplit(maxsplit=3) since only the is
likely to have embedded whitespace.

Added a new test case for the same.

Fixes #2344

From the `pre-push.sample` file:

> Information about the commits which are being pushed is supplied as
> lines to the standard input in the form:
>
>   <local ref> <local sha1> <remote ref> <remote sha1>

When `<local ref>` is not simply a branch name, but a more general
ref (see git-rev-parse(1)), it could contain whitespace, and that
breaks the split() call that expected only 3 spaces in the line.

Changed to use `rsplit(maxsplit=3)` since only the <local ref> is
likely to have embedded whitespace.

Added a new test case for the same.
Copy link
Member

@asottile asottile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, you learn something wild and wacky about git every day it seems

@asottile asottile merged commit 2562c7f into pre-commit:main Apr 14, 2022
@wwade
Copy link
Contributor Author

wwade commented Apr 14, 2022

nice, you learn something wild and wacky about git every day it seems

at least every day 😄

also, I love this gif, thanks for that!

@asottile
Copy link
Member

heh -- there's a bunch more and a user script too -- https://github.com/chriskuehl/shipit https://www.youtube.com/watch?v=2rDNJvHznrM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

pre-push hook failed with "ValueError: too many values to unpack (expected 4)"
2 participants