Skip to content

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

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

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

Conversation

@wwade

@wwade wwade commented Apr 14, 2022

Copy link
Copy Markdown
Contributor

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.

@asottile asottile left a comment

Copy link
Copy Markdown
Member

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

wwade commented Apr 14, 2022

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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