Make it easier to configure HelpText

[NeilMacMullen]

The purpose of this change is to simplify configuration of the help-text output.

The HelpText class has lots of useful flags for controlling the way output is displayed but they are currently cumbersome to access. This PR allows you to write code such as...

  var parser= Parser.Default
                      .SetHelpTextConfiguration(h => 
                        {
                           helptext .AddEnumValuesToHelpText = true;
                           helptext .AddDashesToOption= true;
                         }
                         );
          
)

In addition, the old MaximumDisplayWidth and TextWriter properties have been moved into the HelpTextConfiguration and exposed via a fluent API in Parser to avoid clients having to access too many of the class internals so you can write..

 var p = Parser.Default
                .SetDisplayWidth(80,WidthPolicy.FitToScreen)
                .SetTextWriter(help)
                .SetHelpTextConfiguration(h => h.AddEnumValuesToHelpText = true);
 

The old config accessors are still provided as a backwards compatibility measure. but should be deprecated IMHO.

[NeilMacMullen]

I understand you can also configure HelpText using the pattern described in #224 but I think this approach is cleaner.

• It preserves the ability to inherit the screen width calculated by Parser
• It avoids exposing so many internals to the client (it's annoying that the client still needs to know about HelpText but hoisting all of those configuration flags into Parser would be too disruptive).
• It avoids the requirement to construct a custom TextWriter, call AutoBuild (a nasty internal exposure) and then dump it on the screen just to control the formatting.

[NeilMacMullen]

After reading some of the old issues, it does look like calling this was the recommended way to get custom output so a backwards compatibility version is probably needed unless the major version is bumped.

[NeilMacMullen]

@moh-hassan @nemec Sorry to tag you but @ericnewton76 suggested you were also involved in reviewing/approving PRs. This one would probably need to be merged with #461 (or vice-versa).

[moh-hassan]

Thanks @NeilMacMullen for your effort on this PR.

• Can you try to do an integration test with PR Propagate parser-settings to HelpText (fix #414 and #455) #461 and resolve the confliction.
• Give an example of using the custom parser with your custom help.

[NeilMacMullen]

@moh-hassan This PR now includes the changes made in #461. As a result, #461 should not be merged.

Readme.md has been updated to explain the new (preferred) configuration scheme.

Old methods preserved for backwards compatibility have been marked as Obsolete.

[NeilMacMullen]

I've just realised that the design would be a little better if the Parser.Setxxx mutators returned a new Parser otherwise Parser.Default is mutated (which is probably why tests are sometimes failing as they are being run in paralllel. I'll fix that up tomorrow.

[moh-hassan]

I agree that returning a new Parser otherwise Parser.Default is a better design.
It's best to keep the Parser.Default ASIS and all fluent interface methods with Parser.

[NeilMacMullen]

@moh-hassan I've been thinking about the PR a lot. To be honest I don't think it is possible to clean up the configuration mechanism properly while also maintaining backwards compatibility. Problems with the current design include:

• Failure to cleanly separate settings that affect parsing from settings that affect help generation
• Too much exposure to internals in order to perform simple configuration (e.g. turning on enum values requires you to access AutoBuild, create a new TextWriter etc)
• Too many configuration paths and inconsistent results (Parser.Default != new Parser(c=>{}).
• An over-reliance on a functional approach (I like functional programming but it's a bit overkill for simple configuration

So I'm going to reluctantly drop this. I'd be happy do the work to create a better configuration mechanism but it would have to include breaking changes.

[moh-hassan]

I understand your concerns and creating a better configuration mechanism can be done in a major build version which may be a Fluent Interface and can benefit of the new feature of C# 8.
We can discuss the changes and Breaking Change is taken into consideration as the developer know how to migrate from API V2 to the new API V3.
You can consider your PR here as WIP (work in progress) and you may re-engineer it based on the bug fixes of other Pr(s) or close it and latter you may open a new one as you wish.

[NeilMacMullen]

@moh-hassan Thanks. I'm inclined to knock off a few of the more minor outstanding bugs/issues first. I understand that you are doing this in your spare time :-) but was wondering if you have a particular roadmap in mind? I.e. is there a deadline to get minor/breaking PRs in submitted?

[moh-hassan]

The roadmap in mind is fixing some open hot issues and new features and coordinate with @ericnewton76 for the next release.

[moh-hassan]

I tried to run integration test with PR #356 (help localization) after rebasing with develop, the build success and the test fail for 9 (nine) test cases.

The common error related to TextWrapper class:

Object reference not set to an instance of an object.
System.NullReferenceException
Object reference not set to an instance of an object.
at CommandLine.Text.TextWrapper..ctor(String input) in F:\commandline-2.5\CommandlineFork_2.5\src\CommandLine\Text\TextWrapper.cs:line 21

The integeration branch localize3 and CI appveyor result here.

The xunit test report:
test-pr356-report.zip
I'm not sure what is the root reason of the fail of 9 tests.
Can you have a look to these tests, and it's nice if you help in fixing these test cases so we can merge it?

[NeilMacMullen]

@moh-hassan It's because the localisation PR has changed the default value of BaseAttribute.HelpText from string.empty to null.

It can be fixed by initialising LocalizableAttributeProperty._value to string.Empty

[asherber]

Something like this is definitely needed. Right now it seems that the only choices are go with the auto help, or roll your own inside of WithNotParsed(), including outputting the help text yourself.

I think exposing some of this on ParserSettings might be the way to go.

[moh-hassan]

Thanks @NeilMacMullen
That help in fixing the test fail. Now it's working :)

I did a modification by Seting Helptext property default to string.Empty