Improve formatting of long "via" annotations #1237
Conversation
|
Looks nice! I wonder what @vphilippon would think? |
atugushev
added
enhancement
|
I like that! ( Beside that, 👍 from me! Edit: I wrote the coma suggestion without actually thinking about the potential risk of adding some git diff conflict with that. I'll let you be the judge of that :) |
|
My preference is for no comma. IMO, it doesn't add much to the formatting and I think the no-comma result looks clean and readable enought without it.
This is also a concern for me. When the last dependency in the list changes, now two lines will change instead of one. this causes unnecessary diff noise. So another reason I prefer no comma. I don't consider the above too big a deal though, so if it is considered a blocker, let me know and I'll change it. |
|
from my issue #1036 the version with commas: But I think this PR resolves all the problems I was having with the single line |
|
looking at the test output again, I really like how it looks, and I think I prefer comma-less |
|
@vphilippon thanks for your feedback! I like also "no comma" annotations. Less diff and less symbols. |
Makes for more robust tests as the complete output is tested. Helps show and catch subtle or unexpected changes in output.
Some of the test data exceeds the 88 width limit, so this has been bumped to 100. Black will continue to format non-string statements at a max width of 88.
The pip requirement does not continue on the next line so the line should not end in a trailing slash.
This makes the annotation output more consistent. No longer place annotations inline when hashes aren't generated. Now, always place annotations on a new line. When the annotation comment is very long, this helps make the result more readable as it is less likely to continue far off to the right.
When there is more than one "via" annotation, they are now formatted one per line. This prevents very long lines in requirements.txt output. The very long lines often scroll beyond the width of an editor, making them difficult to read. Worse, when these annotations change, diffs are hard to read as the actual difference is hidden far off to the right. By placing one annotation per line, diffs will be much more obvious. This follows the philosophy of many automatic formatters such as Black. To simplify the formatting and to make it more consistent, annotations are now always on the line after the requirement, whether hashes were generated or not. Fixes #1036
|
Thanks for the feedback and reviews all! |
|
FYI, this new formatting breaks the requirements-txt-fixer pre-commit hook. |
|
@jdufresne Just upgraded and love this change, thanks! |
|
This is a breaking change... And broke my workflow 😞 |
Sounds like a reference to |
| @@ -63,7 +63,7 @@ markers = | |||
| network: mark tests that require internet access | |||
|
|
|||
| [flake8] | |||
| max-line-length = 88 | |||
| max-line-length = 100 | |||
There was a problem hiding this comment.
this looks like a bad standard :(
Sounds like this should've been a CLI flag and not an opinionated reformatting |
Obviously. There are always good reasons to make changes, but the very good reasons not to make changes are often ignored. Such as you will probably be breaking something. |
Includes a humongous diff thanks to jazzband/pip-tools#1237, which places each "via" package on a separate line
Includes a humongous diff thanks to jazzband/pip-tools#1237, which places each "via" package on a separate line
Given that it's normal to commit requirements.txt files into a project repo (that's how you pin, after all), it's more than a matter of breaking tools trying to consume the file. Suddenly we have a large diff in our project's requirements.txt since the file is entirely rewritten, and we'd like to decide when to incur that diff (e.g. by way of a friendly command line option for this formatting change) rather than have the timing be imposed upon us.
Except, your project may need Python 3.9 support or other features which are orthogonal to this formatting change. |
When there is more than one "via" annotation, they are now formatted one
per line. This prevents very long lines in requirements.txt output. The
very long lines often scroll beyond the width of an editor, making them
difficult to read. Worse, when these annotations change, diffs are hard
to read as the actual difference is hidden far off to the right. By
placing one annotation per line, diffs will be much more obvious. This
follows the philosophy of many automatic formatters such as Black.
To simplify the formatting and to make it more consistent, annotations
are now always on the line after the requirement, whether hashes were
generated or not.
Fixes #1036
Contributor checklist