-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Wrap and indent help descriptions for options and commands #1051
Conversation
Co-authored-by: Ephigenia <love@ephigenia.de>
FYI: @Ephigenia |
No rush @abetomo, I requested review here accidentally. Thanks for other reviews already today! |
* | ||
* @param {String} str | ||
* @param {Number} width | ||
* @param {Number} indent |
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.
The document parameter is missing minWidth
.
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.
Prompted by this, I moved minWidth to an internal detail.
index.js
Outdated
function wrap(str, width, indent) { | ||
var regex = new RegExp('.{1,' + (width - 1) + '}([\\s\u200B]|$)|[^\\s\u200B]+?([\\s\u200B]|$)', 'g'); | ||
var lines = str.match(regex) || []; | ||
var result = lines.map(function(line, i) { |
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.
How about returning directly without assigning to a variable?
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.
Done
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.
Finish off #956 with Jest tests and last fixes. See #956 for description and discussion.
(I realised when adding tests for this I have never tried the somewhat undocumented argsDescription parameter to
.description
, and not currently being tested here.)