Skip to content

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Added a feature to avoid linebreaks between option when displaying help #242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/CommandLine/Parser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,20 +183,20 @@ private static Result<IEnumerable<Token>, Error> Tokenize(
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)
Copy link
Member

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)
Copy link
Member

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.

{
parserResult.WithNotParsed(
errors =>
Maybe.Merge(errors.ToMaybe(), helpWriter.ToMaybe())
.Do((_, writer) => writer.Write(HelpText.AutoBuild(parserResult, maxDisplayWidth)))
.Do((_, writer) => writer.Write(HelpText.AutoBuild(parserResult, settings, maxDisplayWidth)))
);

return parserResult;
Expand Down
5 changes: 5 additions & 0 deletions src/CommandLine/ParserSettings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,5 +173,10 @@ private void Dispose(bool disposing)
disposed = true;
}
}
/// <summary>
/// This flag indicate if there is a line break between each option display in help
/// </summary>
public bool AdditionalNewLineAfterOption { get; set; } = true;

}
}
13 changes: 8 additions & 5 deletions src/CommandLine/Text/HelpText.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,19 +202,21 @@ public SentenceBuilder SentenceBuilder
/// <param name='onExample'>A delegate used to customize <see cref="CommandLine.Text.Example"/> model used to render text block of usage examples.</param>
/// <param name="verbsIndex">If true the output style is consistent with verb commands (no dashes), otherwise it outputs options.</param>
/// <param name="maxDisplayWidth">The maximum width of the display.</param>
/// <param name="parserSettings">Settings of the parser. Some settings are for help display.</param>
/// <remarks>The parameter <paramref name="verbsIndex"/> is not ontly a metter of formatting, it controls whether to handle verbs or options.</remarks>
public static HelpText AutoBuild<T>(
ParserResult<T> parserResult,
Func<HelpText, HelpText> onError,
Func<Example, Example> onExample,
ParserSettings parserSettings,
bool verbsIndex = false,
int maxDisplayWidth = DefaultMaximumLength)
{
var auto = new HelpText
{
Heading = HeadingInfo.Empty,
Copyright = CopyrightInfo.Empty,
AdditionalNewLineAfterOption = true,
AdditionalNewLineAfterOption = parserSettings.AdditionalNewLineAfterOption,
AddDashesToOption = !verbsIndex,
MaximumDisplayWidth = maxDisplayWidth
};
Expand Down Expand Up @@ -276,12 +278,13 @@ public static HelpText AutoBuild<T>(
/// </summary>
/// <param name='parserResult'>The <see cref="CommandLine.ParserResult{T}"/> containing the instance that collected command line arguments parsed with <see cref="CommandLine.Parser"/> class.</param>
/// <param name="maxDisplayWidth">The maximum width of the display.</param>
/// <param name="parserSettings">Settings of the parser. Some settings are for help display.</param>
/// <returns>
/// An instance of <see cref="CommandLine.Text.HelpText"/> class.
/// </returns>
/// <remarks>This feature is meant to be invoked automatically by the parser, setting the HelpWriter property
/// of <see cref="CommandLine.ParserSettings"/>.</remarks>
public static HelpText AutoBuild<T>(ParserResult<T> parserResult, int maxDisplayWidth = DefaultMaximumLength)
public static HelpText AutoBuild<T>(ParserResult<T> parserResult, ParserSettings parserSettings, int maxDisplayWidth = DefaultMaximumLength)
{
if (parserResult.Tag != ParserResultType.NotParsed)
throw new ArgumentException("Excepting NotParsed<T> type.", "parserResult");
Expand All @@ -292,13 +295,13 @@ public static HelpText AutoBuild<T>(ParserResult<T> parserResult, int maxDisplay
return new HelpText(HeadingInfo.Default){MaximumDisplayWidth = maxDisplayWidth }.AddPreOptionsLine(Environment.NewLine);

if (!errors.Any(e => e.Tag == ErrorType.HelpVerbRequestedError))
return AutoBuild(parserResult, current => DefaultParsingErrorsHandler(parserResult, current), e => e, maxDisplayWidth: maxDisplayWidth);
return AutoBuild(parserResult, current => DefaultParsingErrorsHandler(parserResult, current), e => e, maxDisplayWidth: maxDisplayWidth, parserSettings:parserSettings);

var err = errors.OfType<HelpVerbRequestedError>().Single();
var pr = new NotParsed<object>(TypeInfo.Create(err.Type), Enumerable.Empty<Error>());
return err.Matched
? AutoBuild(pr, current => DefaultParsingErrorsHandler(pr, current), e => e, maxDisplayWidth: maxDisplayWidth)
: AutoBuild(parserResult, current => DefaultParsingErrorsHandler(parserResult, current), e => e, true, maxDisplayWidth);
? AutoBuild(pr, current => DefaultParsingErrorsHandler(pr, current), e => e, maxDisplayWidth: maxDisplayWidth, parserSettings: parserSettings)
: AutoBuild(parserResult, current => DefaultParsingErrorsHandler(parserResult, current), e => e, parserSettings, true, maxDisplayWidth);
}

/// <summary>
Expand Down
18 changes: 9 additions & 9 deletions tests/CommandLine.Tests/Unit/Text/HelpTextTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Copy link
Member

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.


// Verify outcome
var lines = helpText.ToString().ToNotEmptyLines().TrimStringArray();
Expand Down Expand Up @@ -350,7 +350,7 @@ public void Invoke_AutoBuild_for_Verbs_with_specific_verb_returns_appropriate_fo
});

// Exercize system
var helpText = HelpText.AutoBuild(fakeResult);
var helpText = HelpText.AutoBuild(fakeResult, new ParserSettings());

// Verify outcome
var lines = helpText.ToString().ToNotEmptyLines().TrimStringArray();
Expand Down Expand Up @@ -383,7 +383,7 @@ public void Invoke_AutoBuild_for_Verbs_with_specific_verb_returns_appropriate_fo
});

// Exercize system
var helpText = HelpText.AutoBuild(fakeResult, maxDisplayWidth: 100);
var helpText = HelpText.AutoBuild(fakeResult, new ParserSettings(), maxDisplayWidth: 100);

// Verify outcome
var lines = helpText.ToString().ToNotEmptyLines().TrimStringArray();
Expand Down Expand Up @@ -415,7 +415,7 @@ public void Invoke_AutoBuild_for_Verbs_with_unknown_verb_returns_appropriate_for
new Error[] { new HelpVerbRequestedError(null, null, false) });

// Exercize system
var helpText = HelpText.AutoBuild(fakeResult);
var helpText = HelpText.AutoBuild(fakeResult, new ParserSettings());

// Verify outcome
var lines = helpText.ToString().ToNotEmptyLines().TrimStringArray();
Expand Down Expand Up @@ -501,7 +501,7 @@ public void Invoke_AutoBuild_for_Options_with_Usage_returns_appropriate_formatte
});

// Exercize system
var helpText = HelpText.AutoBuild(fakeResult);
var helpText = HelpText.AutoBuild(fakeResult, new ParserSettings());

// Verify outcome
var text = helpText.ToString();
Expand Down Expand Up @@ -554,7 +554,7 @@ public void Default_set_to_sequence_should_be_properly_printed()

// Exercize system
handlers.ChangeCulture();
var helpText = HelpText.AutoBuild(fakeResult);
var helpText = HelpText.AutoBuild(fakeResult, new ParserSettings());
handlers.ResetCulture();

// Verify outcome
Expand Down Expand Up @@ -585,7 +585,7 @@ public void AutoBuild_when_no_assembly_attributes()
{
onErrorCalled = true;
return ht;
}, ex => ex);
}, ex => ex, new ParserSettings());

onErrorCalled.Should().BeTrue();
actualResult.Copyright.Should().Be(expectedCopyright);
Expand Down Expand Up @@ -617,7 +617,7 @@ public void AutoBuild_with_assembly_title_and_version_attributes_only()
{
onErrorCalled = true;
return ht;
}, ex => ex);
}, ex => ex, new ParserSettings());

onErrorCalled.Should().BeTrue();
actualResult.Heading.Should().Be(string.Format("{0} {1}", expectedTitle, expectedVersion));
Expand Down Expand Up @@ -648,7 +648,7 @@ public void AutoBuild_with_assembly_company_attribute_only()
{
onErrorCalled = true;
return ht;
}, ex => ex);
}, ex => ex, new ParserSettings());

onErrorCalled.Should().BeFalse(); // Other attributes have fallback logic
actualResult.Copyright.Should().Be(string.Format("Copyright (C) {0} {1}", DateTime.Now.Year, expectedCompany));
Expand Down