-
-
Notifications
You must be signed in to change notification settings - Fork 252
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
Dynamically wrap argument and option descriptions based on console width in the default help text generator. #248
Dynamically wrap argument and option descriptions based on console width in the default help text generator. #248
Conversation
Hey there, just letting you know I've seen this, but haven't reviewed it. I'm on vacation for 2 more weeks without a laptop, and I'll take a closer look when I'm home. |
Cheers Mate!
…On Mon, Jul 1, 2019, 7:14 AM Nate McMaster ***@***.***> wrote:
Hey there, just letting you know I've seen this, but haven't reviewed it.
I'm on vacation for 2 more weeks without a laptop, and I'll take a closer
look when I'm home.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#248?email_source=notifications&email_token=ABEI7XDDEJZU54NPNBSXWUDP5IGNDA5CNFSM4H4AWLR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY6ILHA#issuecomment-507282844>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABEI7XDLFYVFCBBJRAU2XK3P5IGNDANCNFSM4H4AWLRQ>
.
|
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.
Thanks for the PR! This one is particularly well done. I'm glad to see you have added tests, removed some duplicated code, and documented all new API.
I have a few suggestions on how I'd like to tweak this change before merging. What you have appears to be fully functional already, so if you don't have time to update your PR, I can merge and make the changes myself. Let me know either way!
Thanks,
Nate
@natemcmaster acking - will take a look tonight again Thanks for making the HelpTextGenerator extendable. I added this functionality into our app a few weeks ago that way. |
10f88ff
to
9b86617
Compare
@natemcmaster Great suggestions, thanks for the review. I've resolved all comments. |
I've also rebased onto master. |
Looks great! Thanks for updating with my feedback. This will be in the 2.4.0 RC release, which I'm hoping to release in a few weeks. |
Dynamic description wrapping in generated help text
This addresses my two concerns in issue #246.
New Behavior
\n
or\r\n
will result in a padded newline.