diff --git a/src/Repl.Core/Help/HelpTextBuilder.Rendering.cs b/src/Repl.Core/Help/HelpTextBuilder.Rendering.cs index 35ff2b3..caa47fd 100644 --- a/src/Repl.Core/Help/HelpTextBuilder.Rendering.cs +++ b/src/Repl.Core/Help/HelpTextBuilder.Rendering.cs @@ -208,7 +208,7 @@ or OptionSchemaTokenKind.ValueAlias parameterType = groupInfo.Property.PropertyType; description = groupInfo.Property.GetCustomAttribute()?.Description ?? string.Empty; var propDefault = groupInfo.Property.GetValue(groupInfo.DefaultInstance); - defaultValue = propDefault is not null && !IsDefaultForType(propDefault, parameterType) + defaultValue = propDefault is not null && !ParsingOptions.IsDefaultForType(propDefault, parameterType) ? $" [default: {propDefault}]" : string.Empty; } @@ -314,31 +314,6 @@ private static Type UnwrapAsyncReturnType(Type returnType) : returnType; } - private static bool IsDefaultForType(object value, Type type) - { - if (type == typeof(bool)) - { - return value is false; - } - - if (type == typeof(int)) - { - return value is 0; - } - - if (type == typeof(long)) - { - return value is 0L; - } - - if (type == typeof(double)) - { - return value is 0.0d; - } - - return false; - } - private static string ResolveOptionPlaceholder(Type parameterType) { var effectiveType = Nullable.GetUnderlyingType(parameterType) ?? parameterType; @@ -619,13 +594,17 @@ private static string[][] BuildGlobalOptionRows(ParsingOptions parsingOptions) .OrderBy(option => option.Name, StringComparer.OrdinalIgnoreCase) .Select(option => { - var aliases = option.Aliases.Count == 0 - ? string.Empty - : $", {string.Join(", ", option.Aliases)}"; + var aliases = option.Aliases.Count == 0 + ? string.Empty + : $", {string.Join(", ", option.Aliases)}"; + var description = string.IsNullOrWhiteSpace(option.Description) + ? "Custom global option." + : option.Description; + return new[] { $"{option.CanonicalToken}{aliases}", - "Custom global option.", + description, }; }); return [.. BuiltInGlobalOptionRows.Concat(customRows)]; diff --git a/src/Repl.Core/Parsing/GlobalOptionDefinition.cs b/src/Repl.Core/Parsing/GlobalOptionDefinition.cs index fd79451..6fd93bc 100644 --- a/src/Repl.Core/Parsing/GlobalOptionDefinition.cs +++ b/src/Repl.Core/Parsing/GlobalOptionDefinition.cs @@ -5,5 +5,6 @@ internal sealed record GlobalOptionDefinition( string CanonicalToken, IReadOnlyList Aliases, string? DefaultValue, + string? Description, Type ValueType, Type? OwnerType); diff --git a/src/Repl.Core/ParsingOptions.cs b/src/Repl.Core/ParsingOptions.cs index b0b25c5..00f4624 100644 --- a/src/Repl.Core/ParsingOptions.cs +++ b/src/Repl.Core/ParsingOptions.cs @@ -101,7 +101,25 @@ internal bool TryGetRouteConstraint(string name, out Func predicat /// Optional aliases. Values without prefix are normalized to --alias. /// Optional default value metadata. public void AddGlobalOption(string name, string[]? aliases = null, T? defaultValue = default) => - AddGlobalOptionCore(name, typeof(T), aliases, defaultValue?.ToString()); + AddGlobalOptionCore(name, typeof(T), aliases, FormatDefaultValue(defaultValue, typeof(T))); + + /// + /// Registers a custom global option with an explicit help description. + /// + /// + /// is the trailing required parameter so this overload never collides with + /// during overload resolution: a positional + /// null second argument (for example AddGlobalOption<string>("tenant", null)) binds + /// unambiguously to the aliases-only overload. The typed + /// path (UseGlobalOptions<T>()) remains the primary way to attach descriptions. + /// + /// Declared value type. + /// Canonical name without prefix (for example: "tenant"). + /// Aliases (pass null for none). Values without prefix are normalized to --alias. + /// Default value metadata (pass default for none). + /// Description shown in help output. + public void AddGlobalOption(string name, string[]? aliases, T? defaultValue, string description) => + AddGlobalOptionCore(name, typeof(T), aliases, FormatDefaultValue(defaultValue, typeof(T)), description); /// /// Registers a custom global option using a type or constraint name @@ -117,7 +135,27 @@ public void AddGlobalOption(string name, string[]? aliases = null, T? default public void AddGlobalOption(string name, string constraintOrTypeName, string[]? aliases = null, string? defaultValue = null) => AddGlobalOptionCore(name, ResolveConstraintOrTypeName(constraintOrTypeName, _customRouteConstraints), aliases, defaultValue); - internal void AddGlobalOptionCore(string name, Type valueType, string[]? aliases, string? defaultValue, Type? ownerType = null) + /// + /// Registers a custom global option (by type or constraint name) with an explicit help description. + /// + /// + /// is the trailing required parameter so this overload never collides with + /// during overload resolution: a positional + /// null third argument (for example AddGlobalOption("port", "int", null)) binds unambiguously + /// to the aliases-only overload. + /// + /// Canonical name without prefix (for example: "tenant"). + /// + /// Built-in type name ("string", "int", "long", "bool", "guid", "uri", "date", "datetime", "timespan") + /// or a registered custom route constraint name. Custom constraints resolve to string. + /// + /// Aliases (pass null for none). Values without prefix are normalized to --alias. + /// Default value as string (pass null for none). + /// Description shown in help output. + public void AddGlobalOption(string name, string constraintOrTypeName, string[]? aliases, string? defaultValue, string description) => + AddGlobalOptionCore(name, ResolveConstraintOrTypeName(constraintOrTypeName, _customRouteConstraints), aliases, defaultValue, description); + + internal void AddGlobalOptionCore(string name, Type valueType, string[]? aliases, string? defaultValue, string? description = null, Type? ownerType = null) { name = string.IsNullOrWhiteSpace(name) ? throw new ArgumentException("Global option name cannot be empty.", nameof(name)) @@ -141,6 +179,7 @@ internal void AddGlobalOptionCore(string name, Type valueType, string[]? aliases CanonicalToken: normalizedCanonical, Aliases: normalizedAliases, DefaultValue: defaultValue, + Description: description, ValueType: valueType, OwnerType: ownerType); } @@ -161,6 +200,37 @@ private static string BuildDuplicateGlobalOptionMessage(string name, Type? exist return $"A global option named '{name}' is already registered by {existingSource} and cannot also be registered by {newSource}."; } + internal static string? FormatDefaultValue(object? value, Type type) => + value is not null && !IsDefaultForType(value, type) + ? value.ToString() + : null; + + internal static bool IsDefaultForType(object value, Type type) + { + var effectiveType = Nullable.GetUnderlyingType(type) ?? type; + if (effectiveType == typeof(bool)) + { + return value is false; + } + + if (effectiveType == typeof(int)) + { + return value is 0; + } + + if (effectiveType == typeof(long)) + { + return value is 0L; + } + + if (effectiveType == typeof(double)) + { + return value is 0.0d; + } + + return false; + } + private static Type ResolveConstraintOrTypeName( string constraintOrTypeName, Dictionary> customConstraints) diff --git a/src/Repl.Defaults/GlobalOptionsExtensions.cs b/src/Repl.Defaults/GlobalOptionsExtensions.cs index af3c1f6..125db8a 100644 --- a/src/Repl.Defaults/GlobalOptionsExtensions.cs +++ b/src/Repl.Defaults/GlobalOptionsExtensions.cs @@ -34,9 +34,10 @@ public static class GlobalOptionsExtensions var optionAttr = property.GetCustomAttribute(); var name = optionAttr?.Name ?? ToKebabCase(property.Name); var aliases = optionAttr?.Aliases; - var defaultValue = property.GetValue(prototype)?.ToString(); + var defaultValue = ParsingOptions.FormatDefaultValue(property.GetValue(prototype), property.PropertyType); + var description = property.GetCustomAttribute()?.Description; - options.Parsing.AddGlobalOptionCore(name, property.PropertyType, aliases, defaultValue, typeof(T)); + options.Parsing.AddGlobalOptionCore(name, property.PropertyType, aliases, defaultValue, description, typeof(T)); } }); diff --git a/src/Repl.IntegrationTests/Given_CustomGlobalOptions.cs b/src/Repl.IntegrationTests/Given_CustomGlobalOptions.cs index f77b163..77e39ae 100644 --- a/src/Repl.IntegrationTests/Given_CustomGlobalOptions.cs +++ b/src/Repl.IntegrationTests/Given_CustomGlobalOptions.cs @@ -1,3 +1,5 @@ +using Repl.Parameters; + namespace Repl.IntegrationTests; [TestClass] @@ -45,5 +47,55 @@ public void When_RequestingRootHelp_Then_CustomGlobalOptionIsListedInGlobalOptio output.ExitCode.Should().Be(0); output.Text.Should().Contain("Global Options:"); output.Text.Should().Contain("--tenant, -t"); + output.Text.Should().Contain("Custom global option."); + } + + [TestMethod] + [Description("Regression guard: verifies typed global option descriptions are rendered in root help without adding default-value display.")] + public void When_RequestingRootHelpForTypedGlobalOptions_Then_DescriptionsAreListed() + { + var sut = ReplApp.Create() + .UseGlobalOptions(); + sut.Map("status", (DemoGlobals globals) => globals.Tenant); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["--help", "--no-logo"])); + + output.ExitCode.Should().Be(0); + output.Text.Should().Contain("--tenant, -t"); + output.Text.Should().Contain("Tenant id used for all commands."); + output.Text.Should().Contain("--verbose, -v"); + output.Text.Should().Contain("Enable verbose diagnostics for all commands."); + output.Text.Should().NotContain("[default:"); + } + + [TestMethod] + [Description("Regression guard: verifies explicit global option descriptions are rendered in root help.")] + public void When_RequestingRootHelpForExplicitGlobalOption_Then_DescriptionIsListed() + { + var sut = ReplApp.Create() + .Options(options => options.Parsing.AddGlobalOption( + "tenant", + aliases: ["-t"], + defaultValue: null, + description: "Tenant id used for all commands.")); + sut.Map("ping", () => "ok"); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["--help", "--no-logo"])); + + output.ExitCode.Should().Be(0); + output.Text.Should().Contain("--tenant, -t"); + output.Text.Should().Contain("Tenant id used for all commands."); + output.Text.Should().NotContain("[default:"); + } + + private sealed class DemoGlobals + { + [System.ComponentModel.Description("Tenant id used for all commands.")] + [ReplOption(Aliases = ["-t"])] + public string? Tenant { get; set; } = "default"; + + [System.ComponentModel.Description("Enable verbose diagnostics for all commands.")] + [ReplOption(Aliases = ["-v"])] + public bool Verbose { get; set; } } } diff --git a/src/Repl.Tests/Given_GlobalOptionsAccessor.cs b/src/Repl.Tests/Given_GlobalOptionsAccessor.cs index 2129a3f..34e066b 100644 --- a/src/Repl.Tests/Given_GlobalOptionsAccessor.cs +++ b/src/Repl.Tests/Given_GlobalOptionsAccessor.cs @@ -67,6 +67,49 @@ public void When_OptionNotProvidedButRegisteredDefault_Then_GetValueReturnsRegis sut.GetValue("port").Should().Be(3000); } + [TestMethod] + [Description("GetValue returns caller default when a value-typed option has no explicit registration default.")] + public void When_ValueTypeOptionNotProvidedAndNoRegisteredDefault_Then_GetValueReturnsCallerDefault() + { + var parsing = new ParsingOptions(); + parsing.AddGlobalOption("port"); + var sut = new GlobalOptionsSnapshot(parsing); + sut.Update(new Dictionary>(StringComparer.OrdinalIgnoreCase)); + + sut.GetValue("port", 8080).Should().Be(8080); + } + + [TestMethod] + [Description("GetValue returns caller default when a described value-typed option has no explicit registration default.")] + public void When_DescribedValueTypeOptionNotProvidedAndNoRegisteredDefault_Then_GetValueReturnsCallerDefault() + { + var parsing = new ParsingOptions(); + parsing.AddGlobalOption("port", aliases: null, defaultValue: default, description: "Port used by the server."); + var sut = new GlobalOptionsSnapshot(parsing); + sut.Update(new Dictionary>(StringComparer.OrdinalIgnoreCase)); + + sut.GetValue("port", 8080).Should().Be(8080); + } + + [TestMethod] + [Description("Regression (#34 review): a positional null second argument keeps binding the aliases-only overload. The description overloads take description as the trailing required parameter precisely so this call never becomes ambiguous (CS0121) with the description overload.")] + public void When_AddGlobalOptionCalledWithPositionalNull_Then_BindsAliasesOnlyOverload() + { + var parsing = new ParsingOptions(); + + // The positional null is the regression scenario itself: naming the argument would make + // resolution trivial and defeat the test. These calls must compile and bind unambiguously + // to the aliases-only overloads (description overloads take description as a trailing required arg). +#pragma warning disable MA0003 // Name the parameter — intentionally positional here; see comment above. + parsing.AddGlobalOption("tenant", null); + parsing.AddGlobalOption("port", "int", null); +#pragma warning restore MA0003 + + parsing.GlobalOptions["tenant"].Description.Should().BeNull(); + parsing.GlobalOptions["tenant"].Aliases.Should().BeEmpty(); + parsing.GlobalOptions["port"].Description.Should().BeNull(); + } + [TestMethod] [Description("HasValue returns false before parsing.")] public void When_NeverUpdated_Then_HasValueReturnsFalse()