-
Notifications
You must be signed in to change notification settings - Fork 1
Add description to optional parameters #32
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: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,32 @@ | ||
| using FluentValidation.TestHelper; | ||
|
|
||
| using ModernRonin.FluentArgumentParser.Definition; | ||
| using ModernRonin.FluentArgumentParser.Validation; | ||
|
|
||
| using NUnit.Framework; | ||
|
|
||
| using System; | ||
|
|
||
| namespace ModernRonin.FluentArgumentParser.Tests.Validation; | ||
|
|
||
| [TestFixture] | ||
| public class OptionalParameterValidatorTests | ||
| { | ||
| [Test] | ||
| public void HasDefaultBeenSet_must_be_true() => | ||
| new OptionalParameterValidator().TestValidate(new OptionalParameter { Type = typeof(string) }) | ||
| new OptionalParameterValidator() | ||
| .TestValidate(new OptionalParameter { Type = typeof(string) }) | ||
| .ShouldHaveValidationErrorFor(p => p.Default); | ||
|
|
||
| [Test] | ||
| public void Description_Should_Have_Error_When_Description_Empty_For_Reference_Type() => | ||
| new OptionalParameterValidator() | ||
| .TestValidate(new OptionalParameter { Type = typeof(string) }) | ||
| .ShouldHaveValidationErrorFor(p => p.Description); | ||
|
|
||
| [Test] | ||
| public void Description_Should_Not_Have_Error_When_Description_Empty_For_Value_Type() => | ||
| new OptionalParameterValidator() | ||
| .TestValidate(new OptionalParameter { Type = typeof(int) }) | ||
| .ShouldHaveValidationErrorFor(p => p.Description); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,9 +2,11 @@ | |
| using System.Collections.Generic; | ||
| using System.Linq; | ||
| using System.Text; | ||
|
|
||
| using ModernRonin.FluentArgumentParser.Definition; | ||
| using ModernRonin.FluentArgumentParser.Extensibility; | ||
| using ModernRonin.FluentArgumentParser.Parsing; | ||
|
|
||
| using MoreLinq.Extensions; | ||
|
|
||
| namespace ModernRonin.FluentArgumentParser.Help; | ||
|
|
@@ -184,7 +186,14 @@ Row indexableToRow(AnIndexableParameter indexable) | |
| Row optionalToRow(OptionalParameter opt) | ||
| { | ||
| var result = indexableToRow(opt); | ||
| result.RightLines.Add($"default: {opt.Default}"); | ||
| if (opt.Default == null) | ||
| { | ||
| result.RightLines.Add(opt.Description); | ||
| } | ||
| else | ||
| { | ||
| result.RightLines.Add($"Default: {opt.Default}"); | ||
| } | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this can be simplified to |
||
| return result; | ||
| } | ||
|
|
||
|
|
@@ -219,10 +228,10 @@ void VerbList(string preamble, IEnumerable<Verb> verbs, Action addDescription) | |
| addDescription(); | ||
| Line("Available commands:"); | ||
| Table(materialized.Select(v => new Row | ||
| { | ||
| LeftText = v.Name, | ||
| RightLines = { v.HelpText } | ||
| }) | ||
| { | ||
| LeftText = v.Name, | ||
| RightLines = { v.HelpText } | ||
| }) | ||
| .ToArray()); | ||
| LineFeed(); | ||
| Line("use help <command> for more detailed help on a specific command"); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -120,4 +120,23 @@ public ParameterBindingConfigurer<TProperty> WithHelp(string helpText) | |
| Parameter.HelpText = helpText; | ||
| return this; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// <para> | ||
| /// Set default description for optional parameter. | ||
| /// </para> | ||
| /// Default description is required for reference types with a default value of null | ||
| /// </summary> | ||
| /// <exception cref="InvalidOperationException"></exception> | ||
| public ParameterBindingConfigurer<TProperty> WithDefaultDescription(string text) | ||
| { | ||
| if (!(Parameter is OptionalParameter optional)) | ||
| { | ||
| throw new InvalidOperationException( | ||
| $"Default are only settable for optional parameters - maybe you forgot a call to {nameof(MakeOptional)}?"); | ||
| } | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would be good to have a test for the throw branch, too |
||
|
|
||
| optional.Description = text; | ||
| return this; | ||
| } | ||
| } | ||
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.
shouldn't it be
.ShouldNotHaveValidationErrors()or some such in this test-case?