Improve spacing in HelpText

[asherber]

Resolves #491

The goal was to make vertical spacing consistent between sections. In HelpText.ToString(), things got slightly complicated because it occurred to me that some users might already be starting PreOptions with their own newline, so I didn't want to add another.

I added a test and also changed some existing tests to look at spacing (using ToLines() instead of ToNotEmptyLines()).

Some example help screens, showing before and after, are in the following comment.

Edit: (If it's intentional/desirable for PreOptions to start right after the copyright, with no space, then I can amend this PR to take that bit out. But I still think there should be a space before Usage.)

[asherber]

Usage only

// BEFORE
CommandLine 2.6.0
Copyright (c) 2005 - 2018 Giacomo Stelluti Scala & Contributors
USAGE:
Delete the frobber:
  CommandLine -o Delete

  -o           Operation to perform.
  --help       Display this help screen.
  --version    Display version information.
      
// AFTER
CommandLine 0.0.0
Copyright (c) 2005 - 2018 Giacomo Stelluti Scala & Contributors

USAGE:
Delete the frobber:
  CommandLine -o Delete

  -o           Operation to perform.
  --help       Display this help screen.
  --version    Display version information.

Error only

// BEFORE
CommandLine 2.6.0
Copyright (c) 2005 - 2018 Giacomo Stelluti Scala & Contributors

ERROR(S):
  Option 'bad' is unknown.

  -o           Operation to perform.
  --help       Display this help screen.
  --version    Display version information.

// AFTER
CommandLine 0.0.0
Copyright (c) 2005 - 2018 Giacomo Stelluti Scala & Contributors

ERROR(S):
  Option 'bad' is unknown.

  -o           Operation to perform.
  --help       Display this help screen.
  --version    Display version information.

Error and Usage

// BEFORE
CommandLine 2.6.0
Copyright (c) 2005 - 2018 Giacomo Stelluti Scala & Contributors

ERROR(S):
  Option 'bad' is unknown.
USAGE:
Delete the frobber:
  CommandLine -o Delete

  -o           Operation to perform.
  --help       Display this help screen.
  --version    Display version information.
  
// AFTER
CommandLine 0.0.0
Copyright (c) 2005 - 2018 Giacomo Stelluti Scala & Contributors

ERROR(S):
  Option 'bad' is unknown.

USAGE:
Delete the frobber:
  CommandLine -o Delete

  -o           Operation to perform.
  --help       Display this help screen.
  --version    Display version information.

Usage and pre-options

// BEFORE
CommandLine 2.6.0
Copyright (c) 2005 - 2018 Giacomo Stelluti Scala & Contributors
pre-options
USAGE:
Delete the frobber:
  CommandLine -o Delete

  -o           Operation to perform.
  --help       Display this help screen.
  --version    Display version information.

// AFTER
CommandLine 0.0.0
Copyright (c) 2005 - 2018 Giacomo Stelluti Scala & Contributors

pre-options

USAGE:
Delete the frobber:
  CommandLine -o Delete

  -o           Operation to perform.
  --help       Display this help screen.
  --version    Display version information.

Error and pre-options

// BEFORE
CommandLine 2.6.0
Copyright (c) 2005 - 2018 Giacomo Stelluti Scala & Contributors
pre-options

ERROR(S):
  Option 'bad' is unknown.

  -o           Operation to perform.
  --help       Display this help screen.
  --version    Display version information.
  
// AFTER
CommandLine 0.0.0
Copyright (c) 2005 - 2018 Giacomo Stelluti Scala & Contributors

pre-options

ERROR(S):
  Option 'bad' is unknown.

  -o           Operation to perform.
  --help       Display this help screen.
  --version    Display version information.

Usage, Error, and pre-options

// BEFORE
CommandLine 2.6.0
Copyright (c) 2005 - 2018 Giacomo Stelluti Scala & Contributors
pre-options

ERROR(S):
  Option 'bad' is unknown.
USAGE:
Delete the frobber:
  CommandLine -o Delete

  -o           Operation to perform.
  --help       Display this help screen.
  --version    Display version information.

// AFTER
CommandLine 0.0.0
Copyright (c) 2005 - 2018 Giacomo Stelluti Scala & Contributors

pre-options

ERROR(S):
  Option 'bad' is unknown.

USAGE:
Delete the frobber:
  CommandLine -o Delete

  -o           Operation to perform.
  --help       Display this help screen.
  --version    Display version information.

[moh-hassan]

Thanks @asherber for your work.
To avoid a breaking change for existing test cases in the next minor version, set this feature as an option in HelpText, something like:

                    AddSpaceBeforeUsage=true    //default =false

[asherber]

This PR includes changes to the unit tests so that they pass with the new output.

I also don't think this should be hidden behind an option flag, because it corrects the current undesirable spacing -- all users should get the new behavior without having to opt in to it.

[asherber]

Or are you talking about not breaking test cases for consumers of the library? I can see that point.

What do you think about the space before PreOptions? If it was intentional not to have a space between the header and PreOptions, then I'll take that out and add the flag as you suggest.

But for the PR as currently written (including space before PreOptions), then the flag should probably be something like UseNewSpacing. This would be my vote.

What do you think?

[moh-hassan]

It may be something like:

          AddNewlineBetHelpSections

That handle all newlines you added including: PreOptions, Usage, ...

[asherber]

Sorry this took me a while to get to -- I was on vacation. I hope this looks good to you now.

[moh-hassan]

Thanks for the change.
What's the purpose of the methods: SafeStartsWith and SafeEndsWith.
I find that it's used only when the argument (string s) is Environment.NewLine.

[asherber]

Moving that code into its own methods makes the code in HelpText.ToString() more readable. I guess I could have made them local functions instead, but it seemed like they might be useful somewhere else at some point.

[moh-hassan]

I guess it's used to check for existing Environment.NewLine. Is it?
It's better to add XML Documentation Comments to these methods.

[asherber]

Ah, you're right, I meant to do that and forgot. I'll add it this evening.

[DaRosenberg]

Really glad someone made a PR for this! Was the first thing that bothered me when I first started looking at this library 2 hours ago. :)

Let's get it merged!

[moh-hassan]

+1
Approved

[segunak]

I'd like to use the improvement being referenced here, is there a way for me to integrate this development branch improvement into my project through NuGet?

[moh-hassan]

@akintundejr4
This PR is in the milestone and planed to be in the next Release 2.7.
You can download the Dev nuget package from CI server here.