-
Notifications
You must be signed in to change notification settings - Fork 11
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
Align and wrap description, usage, flags, and commands #41
Align and wrap description, usage, flags, and commands #41
Conversation
@richard-viney this is awesome, thanks for tackling this! I'll hopefully have time to take a look in the next few days |
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.
🥳
Wow this is great! I really appreciate you picking up this ticket, I've left some preliminary style suggestions while I attempt to better understand the wrapping math that's being done here.
Some additional thoughts that I had at first glance:
- The maximum width should be user-configurable and defined as a field in the
Config
, this field can continue to default to 100 as you have here but i suspect some users will want to customize the max (with 80 likely being the most common default) - There will need to be at least 1 new test case to for the alignment and wrapping behaviour for long subcommand names and descriptions. All current subcommands in the help text tests have very short names and descriptions and will thus not exercise this behaviour
- I would really appreciate some inline comments detailing the logic & math for the wrapping and alignment, both in the
utils.gleam
as well as where the wrapping is being used inglint.gleam
- This will be super helpful for me when I inevitably need to remember what all of this is doing 😄
- no need to go overboard, but definitely don't worry about being too verbose
Ready for another look! |
The default max output width is now 80 chars to match what |
This looks great, i'm going to merge it now! Thanks @richard-viney 💜 |
Could also consider adding a blank line between each command and/or flag. Maybe it should be configurable?