Conversation
Fixes pallets#3362. wrap_text builds a TextWrapper without setting break_on_hyphens, so it keeps textwrap's default of True. That is right for prose, where splitting a hyphenated word across lines is normal typesetting, but wrong for a usage line, which is a list of option names. An option split at a hyphen leaves a fragment the reader cannot copy or type: Usage: program --enable-verbose-logging --output-file-path --max- retry-count --disable-cache-mode --config-file- Adds break_on_hyphens to wrap_text, defaulting to the current behaviour so no existing caller changes, and passes False from both branches of write_usage. Adds a regression test reproducing the report exactly, and one covering the long-prefix branch where the arguments move to their own line.
Member
Author
|
Let me give you the calculation, and then manually check the solution. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3362.
The problem
wrap_textbuilds itsTextWrapperwithout settingbreak_on_hyphens, so it keepstextwrap's default ofTrue. That is the right default for prose — splitting a hyphenated word across lines is normal typesetting — but wrong for a usage line, which is a list of option names.Reproducing the report on
main:Three options are split.
--max-on its own is not something a reader can copy or type, and it is not obvious from the fragment whether the newline is part of the name.After
Byte-identical to the expected output in the issue.
The change
wrap_textgains abreak_on_hyphensparameter defaulting toTrue, so no existing caller changes behaviour.write_usagepassesFalsefrom both of its branches — the one where arguments sit beside the prefix, and the one where a long prefix pushes them onto their own line.I kept it scoped to
write_usagerather than flipping the default globally, since prose help text is a different case and that would be a wider behavioural change than the issue calls for.Tests
Two regression tests: one reproducing the report exactly and asserting the full expected output, one covering the long-prefix branch. The first fails on
main.Note the second test uses
width=40deliberately — at narrower widths the option is wider than the line andbreak_long_wordssplits it, which is a separate mechanism and not what this fixes.Full suite: 1993 passed, 24 skipped, 1 xfailed.