-
-
Notifications
You must be signed in to change notification settings - Fork 611
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
Ensure pip-compile --no-header <blank requirements.in> creates/overwrites requirements.txt #909
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for submitting this! I was thinking about:
@@ -230,6 +220,10 @@ class OutputWriter(object):
if not self.dry_run:
self.dst_file.write(unstyle(line).encode("utf-8"))
self.dst_file.write(os.linesep.encode("utf-8"))
+ else:
+ # Create an empty file, if there is no output
+ if not self.dry_run:
+ self.dst_file.write("")
What do you think?
@AndydeCleyre could you please rebase onto master to fix red CI checks? |
Codecov Report
@@ Coverage Diff @@
## master #909 +/- ##
==========================================
- Coverage 99.06% 98.58% -0.48%
==========================================
Files 35 35
Lines 2247 2264 +17
Branches 288 290 +2
==========================================
+ Hits 2226 2232 +6
- Misses 13 21 +8
- Partials 8 11 +3
Continue to review full report at Codecov.
|
Is a simple merge of master into here acceptable? GitHub pull requests don't love history rewrites. I may be misunderstanding your alternative suggested fix, but:
That said, always |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My suggestion was wrong, thanks for noticing. I'm okay with the current approach, but I have a few comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Could you rebase and squash before I merge this pull request? |
…d --no-header is supplied (Fixes jazzband#900) Add !is_empty (yielded) tracking to the writer's _iter_lines method, and yield a blank line (empty string) if (and only if) otherwise nothing at all would be yielded.
bd41a63
to
d53d3ff
Compare
Thanks, @AndydeCleyre! |
Before, in the case where the writer should generate a blank output document (e.g.
requirements.txt
), no new file would be created, and any existing file with that name would be left unchanged. With this change, a blank document will created and overwritten if it exists.This is implemented by having the writer keep track of whether it yields any lines, and if not, yielding an empty string (where before it would yield nothing).
A simpler solution would be to always yield an extra empty string at the end of
_iter_lines
, but that would result in a bit of awkward space at the bottom of all thetxt
s, and if that were fixed it would involve leaking the composition logic outside of_iter_lines
.Changelog-friendly one-liner:
pip-compile --no-header <blank requirements.in>
now creates/overwritesrequirements.txt
.Fixes #900.
@atugushev I hereby request a review. If this does not satisfy the "Requested a review from another contributor" requirement, please tell me what does. Thanks!
Contributor checklist