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

Zero length stdin input causes black to fail (regression) #2337

Closed
andyphelps opened this issue Jun 16, 2021 · 6 comments · Fixed by #2346
Closed

Zero length stdin input causes black to fail (regression) #2337

andyphelps opened this issue Jun 16, 2021 · 6 comments · Fixed by #2346
Labels
C: configuration CLI and configuration F: comments The syntactic kind. Not in the language grammar, always on our minds. Best bugs. R: not a bug This is deliberate behavior of Black.

Comments

@andyphelps
Copy link

Describe the bug

Using latest version 21.6b0 piping an empty file into black as below causes "string index out of range". This is a regression from 21.5b2.

NOTE: referencing an empty file works as expected this is only when piping into - as example below.
i.e. this works:

touch __init__.py
python -m black __init__.py
All done! ✨ 🍰 ✨
1 file left unchanged.

To Reproduce

touch __init__.py
cat __init__.py | python -m black -
error: cannot format -: string index out of range
Oh no! 💥 💔 💥
1 file failed to reformat.

Expected behavior

Using stdin with empty file works as previous version without error.

Environment (please complete the following information):

  • Version:
  • OS and Python version:

Does this bug also happen on main?

Yes

Additional context

A bit of a tails case but we're using this approach within a CI script.

@JelleZijlstra JelleZijlstra added C: configuration CLI and configuration F: comments The syntactic kind. Not in the language grammar, always on our minds. Best bugs. labels Jun 16, 2021
@cooperlees
Copy link
Collaborator

cooperlees commented Jun 16, 2021

I feel the opposite here and that we were broken before and this is now fixed. We should error when nothing is piped in. It will allow people to optimize their calls sending empty strings and save those resources from starting black for no reason.

I feel your CI script should check if the file is empty before trying to pipe it into black.

Edit: That said, I can see expectations here due to other Unix CLI tool behavior tho. So we should do what majority thinks is right here.

  • maybe we only care with —check

@cooperlees cooperlees added the R: not a bug This is deliberate behavior of Black. label Jun 16, 2021
@felix-hilden
Copy link
Collaborator

That's a fair point, but not immediately obvious at least to me. The error happens when manually inputting an empty stream as well.

At the very least, I think "string index out of range" is not the error we want to be throwing. If we want to restrict to nonempty inputs, maybe it should read "cannot format empty stream" or something like that. Thoughts?

@JelleZijlstra
Copy link
Collaborator

I think we should allow this, since Unix tools generally accept empty input (e.g. cat, grep, awk).

@shadchin
Copy link

Bug here - https://github.com/psf/black/blob/main/src/black/__init__.py#L806

@ghost
Copy link

ghost commented Jun 22, 2021

May I work on this?

@JelleZijlstra
Copy link
Collaborator

Please do!

ichard26 pushed a commit that referenced this issue Jun 23, 2021
Commit history before merge:

* Accept empty stdin (close #2337)
* Update tests/test_black.py
* Add changelog
* Assert Black reformats an empty string to an empty string (#2337) (#2346)
* fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: configuration CLI and configuration F: comments The syntactic kind. Not in the language grammar, always on our minds. Best bugs. R: not a bug This is deliberate behavior of Black.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants