fix wrapped & indented option, command and argument descriptions

[Ephigenia]

Long Command, Option and Argument descriptions are not wrapped to the available TTY columns which make it hard to read the descriptions if they contain a lot of information. This change wraps the descriptions to the available space and also indents them so that they align correctly.

Issues:

• Format help descriptions that are too long #579
• Indent multi-line option description in help #759
• Regression in multiline descriptions #396

Example Output:

Usage: mite-project-update [options] [projectId]

Updates a specific project

Arguments:

  projectId                    Id of the project of which the note should be altered Cupidatat enim dolore anim culpa
                               ullamco laborum. Ut do quis aliqua et in sit ex irure in elit aliquip in. Anim
                               exercitation ut commodo deserunt non aute et ad occaecat ut magna laborum mollit.

Options:
  -V, --version                output the version number
  --archived <true|false>      define the new archived state of the project
  --budget <budget>            Defines the budget either in minutes or cents, alternatively also a duration value can
                               be used. F.e. 23:42 for 23 hours and 42 minutes.
  --budget-type <budget_type>  Defines the budget type that should be used. Only accepts one of the following options:
                               minutes_per_month, minutes, cents, cents_per_month
  --hourly-rate <hourlyRate>   optional value in cents to set for new hourly rate
  --name <name>                Alters the name of the project to the given value
  --note <note>                Alters the note of the project to the given value
  --update-entries             Works only in compbination with hourly-rate. When used also updates all already created
                               time entries of the project with the new hourly-rate
  -h, --help                   output usage information

[shadowspawn]

Nice clear description, thanks @Ephigenia

[shadowspawn]

I have done a first pass and hit some implementation details to fix or discuss.

Bug: I sometimes visually get extra lines with no text or just a word when wrapping occurs, and not sure if there is an off-by-one or if due to not accounting for the trailing whitespace before the split. Or both.

Bug: when there is not enough room, the pull request puts one word per line fully indented. I think when the space gets too small we should give up on word-wrapping. I have not experimented to see what a suitable minimum width would be.

For reference, I looked into how white space is handled with word-wrap in other situations. css has lots of options, and I think this pull request might be effectively pre-wrap, which I think is appropriate. https://developer.mozilla.org/en-US/docs/Web/CSS/white-space

Musing: are there cases where user has hand-formatted text we should leave alone? Can we recognise them? I wondered about leaving alone text with a line-break, but I think people will often want the next line indented so not that easy. I think maybe always on?

[Ephigenia]

Thanks for the feedback. I‘ll work on it a bit more.

[shadowspawn]

The regular expression is pretty complex. I made three changes to try making the match not run over the width unless needed: dropped first match by one, and make both white space matches match single instead of multiple. (Only experimented in regular expression tester!)

var regex = new RegExp('.{1,' + (width - 1) + '}([\\s\u200B]|$)|[^\\s\u200B]+?([\\s\u200B]|$)', 'g');

[Ephigenia]

@shadowspawn included your changes which look good to me. Feel free to check again.

[Ephigenia]

@shadowspawn I’ve fixed the typo which was causing the CI job to fail. I think the PR is now ready for a final review & merge.

[shadowspawn]

Thanks. I will have another look, but might be after the v3 release we are working on.

[shadowspawn]

I did some testing of RegExp on testing sites, and in stand-alone code. Looking hopeful. I haven't retested in Commander code yet.

[shadowspawn]

Musing: are there cases where user has hand-formatted text we should leave alone? Can we recognise them? I wondered about leaving alone text with a line-break, but I think people will often want the next line indented so not that easy. I think maybe always on?

Are there cases? Yes, yes there are. And in fact caused a previous implementation of indented multiline descriptions to be pulled! Came across this: #396

Oh, and #759 was specifically about multiline too.

Reminder to self: check multiline behaviours when revisit!

[Ephigenia]

I don't know how to handle the case mentioned in #396 where the indention and line breaks are added manually. Sure there can be an option to disable the automatic formatting of the help message. That should be part of a solution to #396 or a new feature and not part of this PR?

I’m not sure how this case could be detected automatically. Maybe when the first line of the help message starts with a number of white-space characters?

Fixing #759 also would require detecting the number of whitespaces used in the first line to detect the depth of indention added manually. Also here I guess disabling the auto-format of the help message would help more.

Besides that, I’ve merged the changes against the 3.0.0/master

[shadowspawn]

I am still seeing problems with lines too long. In this example (manually formatted to match what I see on screen), the words "ka" and "kahs" both caused the terminal line to wrap.

$ node -e "console.log(process.stdout.columns)"
70
$ node index.js --help
Usage: index [options] [command]

xyz

Options:
  -x -xxx     kjsahdkajshkahd kajhsd akhds kashd kajhs dkha dkh aksd k
a 
              dkha kdh kasd ka kahs dkh sdkh askdh aksd kashdk ahsd ka
hs 
              dkha skdh 
  -h, --help  output usage information

const program = require("commander");

program
  .description("xyz")
  .option("-x -xxx", "kjsahdkajshkahd kajhsd akhds kashd kajhs dkha dkh aksd ka dkha kdh kasd ka kahs dkh sdkh askdh aksd kashdk ahsd kahs dkha skdh ")
  .command("alpha", "sjhsjhsjhs akjshkjashd kashd kajshdk ashd kashdkashd kash kahs kdh kshdk ashdk ashd kashdk ashd kash akshdaks hd kjahs d")
  // .command("betabetabteabteabetabetabteabteabetabetabteabteabetabetabteabteabetabetabteabteabetabetabteabteabetabetabteabtea", "sjhsjhsjhs akjshkjashd kashd kajshdk ashd kashdkashd kash kahs kdh kshdk ashdk ashd kashdk ashd kash akshdaks hd kjahs d")
  ;

program.parse(process.argv);

[shadowspawn]

The other two issues are as mentioned already:

1. To make this a default behaviour, we need to be confident it works well in most cases. We need a story for what happens in programs where people have manually formatted their help.

A thought is not formatting if there is consecutive whitespace of any sort in the text, including return followed by tab, and multiple spaces?

2. If there is not enough space to usefully wrap, then skip wrapping. Otherwise when the space for the wrapped text is narrow enough this code puts one word per line.

[shadowspawn]

I'm not going to be able to help much more, as I am working on other issues.

(The backwards compatibility is a hard problem, but matters a lot for a default behaviour.)

[Ephigenia]

Thanks for the hint and the code examples with the not working indention and line breaks. I’ll find a way to fix that. I’ll also try to make the code backwards compatible.

[Ephigenia]

My latest commits should make it easier to keep commander command and option descriptions which contain manually formatted descriptions backwards compatible. Also, there was an issue with the calculation of available width for the description which was fixed. Some tests were also added to see the input and result of the description cases.

[Ephigenia]

Just noticed that travis doesn’t complete successfully. I’ll check that.

[shadowspawn]

I did lots of testing and reading tonight, and only found one minor issue with the default value (when present) being added.

[shadowspawn]

The default value should be added to the description for optionalWrap, rather than added afterwards. I suggest a temporary variable to help with readability.

e.g.

var desc = option.description +
       ((!option.negate && option.defaultValue !== undefined) ? ' (default: ' + JSON.stringify(option.defaultValue) + ')' : '')

[shadowspawn]

(Marking as semver:major as changes help output significantly, although arguably "just" a feature.)

[shadowspawn]

I like the optionalWrap approach @Ephigenia, and the additional tests. Thanks.

[shadowspawn]

Picking this up to rework after change to Jest

[Ephigenia]

@shadowspawn sorry I didn’t find time in the past weeks to check this out. I’ll try to support you with this in the next week.

[shadowspawn]

No problem. I'll invite you to review, but optional.

[shadowspawn]

Implementation moved to #1051.

[shadowspawn]

#1051 has been merged to develop and will be released in v4.0.0. Comments still welcome if you wish.

Thank you for your contributions.

[shadowspawn]

v4.0.0 has been released.

Thank you for your contributions.