-
Notifications
You must be signed in to change notification settings - Fork 482
Added a feature to avoid linebreaks between option when displaying help #242
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
base: develop
Are you sure you want to change the base?
Conversation
…lp AdditionalNewLineAfterOption
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.
In it's current state, you actually break the interface for HelpText.AutoBuild. You need to preserve the existing method parameters with a default ParserSettings instance.
You can always mark the older method as Obsolete if it makes sense.
@@ -183,20 +183,20 @@ public void Dispose() | |||
settings.EnableDashDash)(arguments, optionSpecs); | |||
} | |||
|
|||
private static ParserResult<T> MakeParserResult<T>(ParserResult<T> parserResult, ParserSettings settings) | |||
private /*static*/ ParserResult<T> MakeParserResult<T>(ParserResult<T> parserResult, ParserSettings settings) |
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.
It would be better to just remove the static modifier altogether. We can see the change in diffs.
{ | ||
return DisplayHelp( | ||
parserResult, | ||
settings.HelpWriter, | ||
settings.MaximumDisplayWidth); | ||
} | ||
|
||
private static ParserResult<T> DisplayHelp<T>(ParserResult<T> parserResult, TextWriter helpWriter, int maxDisplayWidth) | ||
private /*static*/ ParserResult<T> DisplayHelp<T>(ParserResult<T> parserResult, TextWriter helpWriter, int maxDisplayWidth) |
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.
It would be better to just remove the static modifier altogether. We can see the change in diffs.
@@ -315,7 +315,7 @@ public void Invoke_AutoBuild_for_Options_returns_appropriate_formatted_text() | |||
}); | |||
|
|||
// Exercize system | |||
var helpText = HelpText.AutoBuild(fakeResult); | |||
var helpText = HelpText.AutoBuild(fakeResult, new ParserSettings()); |
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.
All of these test updates should've alerted you to the fact that you broke the existing expected interface. This will cause problems for people simply upgrading from a patch release, we'd have to make it wait until a full version release.
As noted elsewhere, you should preserve this method and make your new method an overload with the old method calling the new method with that default ParserSettings
instance.
b211712
to
746885a
Compare
Hi
Linked with issue #224.
I try to make this to allow users to set option in ParserSettings to have or not linebreaks between option when displaying help.
Thanks for advance