Skip to content

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

Merged
asottile merged 1 commit intopre-commit:mainfrom
wwade:main
Apr 14, 2022
Merged

pre-push: fix stdin line splitting when <local ref> has whitespace#2345
asottile merged 1 commit intopre-commit:mainfrom
wwade:main

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