Skip to content

Commit a2ea4b1

Browse files
angelobreuerKeboo
angelobreuer
and
Keboo
authored
Fix user-specific resources not used (#1340)
* Fix user-specific resources not used Currently, System.CommandLine does only make partial use of the System.CommandLine.Resources passed by the user. Instead, it used the default instance (System.CommandLine.Resources.Instance). This commit fixes most occurrences where access is made to the shared instance instead of the user-passed resources. * Fix tests * Address PR feedback - Expose Resources property on InvocationContext - Make resources parameter required in HelpBuilder ctor Co-authored-by: Kevin B <[email protected]> * Fix test compilation * Set help option description when building parser * Set version description option when building parser * Trigger CI Co-authored-by: Kevin B <[email protected]>
1 parent df57bcb commit a2ea4b1

14 files changed

+84
-52
lines changed

‎src/System.CommandLine.Tests/Help/HelpBuilderTests.cs

+4-3
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ private HelpBuilder GetHelpBuilder(int maxWidth)
4141
{
4242
return new HelpBuilder(
4343
console: _console,
44+
Resources.Instance,
4445
maxWidth
4546
);
4647
}
@@ -136,7 +137,7 @@ public void Usage_section_shows_arguments_if_there_are_arguments_for_command_whe
136137
var rootCommand = new RootCommand();
137138
rootCommand.AddCommand(command);
138139

139-
new HelpBuilder(_console, LargeMaxWidth).Write(command);
140+
new HelpBuilder(_console, Resources.Instance, LargeMaxWidth).Write(command);
140141

141142
var expected =
142143
$"Usage:{NewLine}" +
@@ -1632,7 +1633,7 @@ public void Help_describes_default_values_for_subcommand_with_multiple_defaultab
16321633
[InlineData(int.MinValue)]
16331634
public void Constructor_ignores_non_positive_max_width(int maxWidth)
16341635
{
1635-
var helpBuilder = new HelpBuilder(_console, maxWidth);
1636+
var helpBuilder = new HelpBuilder(_console, Resources.Instance, maxWidth);
16361637
Assert.Equal(int.MaxValue, helpBuilder.MaxWidth);
16371638
}
16381639

@@ -1641,7 +1642,7 @@ private class CustomHelpBuilderThatAddsTextAfterDefaultText : HelpBuilder
16411642
private readonly string _theTextToAdd;
16421643

16431644
public CustomHelpBuilderThatAddsTextAfterDefaultText(IConsole console, string theTextToAdd)
1644-
: base(console)
1645+
: base(console, Resources.Instance)
16451646
{
16461647
_theTextToAdd = theTextToAdd;
16471648
}

‎src/System.CommandLine.Tests/Invocation/InvocationPipelineTests.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ public async Task When_help_builder_factory_is_specified_it_is_used_to_create_th
288288
Func<BindingContext, IHelpBuilder> helpBuilderFactory = context =>
289289
{
290290
factoryWasCalled = true;
291-
return createdHelpBuilder = new HelpBuilder(context.Console);
291+
return createdHelpBuilder = new HelpBuilder(context.Console, context.ParseResult.Parser.Configuration.Resources);
292292
};
293293

294294
var command = new Command("help-command");

‎src/System.CommandLine/Binding/ArgumentConverter.cs

+21-15
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@ internal static class ArgumentConverter
3737
internal static ArgumentConversionResult ConvertObject(
3838
IArgument argument,
3939
Type type,
40-
object? value)
40+
object? value,
41+
Resources resources)
4142
{
4243
if (argument.Arity.MaximumNumberOfValues == 0)
4344
{
@@ -49,15 +50,15 @@ internal static ArgumentConversionResult ConvertObject(
4950
case string singleValue:
5051
if (type.IsEnumerable() && !type.HasStringTypeConverter())
5152
{
52-
return ConvertStrings(argument, type, new[] { singleValue });
53+
return ConvertStrings(argument, type, new[] { singleValue },resources);
5354
}
5455
else
5556
{
56-
return ConvertString(argument, type, singleValue);
57+
return ConvertString(argument, type, singleValue, resources);
5758
}
5859

5960
case IReadOnlyList<string> manyValues:
60-
return ConvertStrings(argument, type, manyValues);
61+
return ConvertStrings(argument, type, manyValues, resources);
6162
}
6263

6364
return None(argument);
@@ -66,7 +67,8 @@ internal static ArgumentConversionResult ConvertObject(
6667
private static ArgumentConversionResult ConvertString(
6768
IArgument argument,
6869
Type? type,
69-
string value)
70+
string value,
71+
Resources resources)
7072
{
7173
type ??= typeof(string);
7274

@@ -82,7 +84,7 @@ private static ArgumentConversionResult ConvertString(
8284
}
8385
catch (Exception)
8486
{
85-
return Failure(argument, type, value);
87+
return Failure(argument, type, value, resources);
8688
}
8789
}
8890
}
@@ -105,13 +107,14 @@ private static ArgumentConversionResult ConvertString(
105107
return Success(argument, instance);
106108
}
107109

108-
return Failure(argument, type, value);
110+
return Failure(argument, type, value, resources);
109111
}
110112

111113
public static ArgumentConversionResult ConvertStrings(
112114
IArgument argument,
113115
Type type,
114116
IReadOnlyList<string> tokens,
117+
Resources resources,
115118
ArgumentResult? argumentResult = null)
116119
{
117120
Type itemType;
@@ -137,7 +140,7 @@ public static ArgumentConversionResult ConvertStrings(
137140
{
138141
var token = tokens[i];
139142

140-
var result = ConvertString(argument, itemType, token);
143+
var result = ConvertString(argument, itemType, token, resources);
141144

142145
switch (result)
143146
{
@@ -204,9 +207,10 @@ internal static bool HasStringTypeConverter(this Type type) =>
204207
private static FailedArgumentConversionResult Failure(
205208
IArgument argument,
206209
Type expectedType,
207-
string value)
210+
string value,
211+
Resources resources)
208212
{
209-
return new FailedArgumentTypeConversionResult(argument, expectedType, value);
213+
return new FailedArgumentTypeConversionResult(argument, expectedType, value, resources);
210214
}
211215

212216
internal static ArgumentConversionResult ConvertIfNeeded(
@@ -219,19 +223,21 @@ internal static ArgumentConversionResult ConvertIfNeeded(
219223
SuccessfulArgumentConversionResult successful when !toType.IsInstanceOfType(successful.Value) =>
220224
ConvertObject(conversionResult.Argument,
221225
toType,
222-
successful.Value),
226+
successful.Value,
227+
symbolResult.Resources),
223228
SuccessfulArgumentConversionResult successful when toType == typeof(object) &&
224229
conversionResult.Argument.Arity.MaximumNumberOfValues > 1 &&
225230
successful.Value is string =>
226231
ConvertObject(conversionResult.Argument,
227232
typeof(IEnumerable<string>),
228-
successful.Value),
233+
successful.Value,
234+
symbolResult.Resources),
229235
NoArgumentConversionResult _ when toType == typeof(bool) =>
230236
Success(conversionResult.Argument,
231237
true),
232238
NoArgumentConversionResult _ when conversionResult.Argument.Arity.MinimumNumberOfValues > 0 =>
233239
new MissingArgumentConversionResult(conversionResult.Argument,
234-
Resources.Instance.RequiredArgumentMissing(symbolResult)),
240+
symbolResult.Resources.RequiredArgumentMissing(symbolResult)),
235241
NoArgumentConversionResult _ when conversionResult.Argument.Arity.MaximumNumberOfValues > 1 =>
236242
Success(conversionResult.Argument,
237243
Array.Empty<string>()),
@@ -274,8 +280,8 @@ public static bool TryConvertArgument(ArgumentResult argumentResult, out object?
274280
{
275281
// 0 is an implicit bool, i.e. a "flag"
276282
0 => Success(argumentResult.Argument, true),
277-
1 => ConvertObject(argument, argument.ValueType, argumentResult.Tokens[0].Value),
278-
_ => ConvertStrings(argument, argument.ValueType, argumentResult.Tokens.Select(t => t.Value).ToArray(), argumentResult)
283+
1 => ConvertObject(argument, argument.ValueType, argumentResult.Tokens[0].Value, argumentResult.Resources),
284+
_ => ConvertStrings(argument, argument.ValueType, argumentResult.Tokens.Select(t => t.Value).ToArray(), argumentResult.Resources, argumentResult)
279285
};
280286

281287
return value is SuccessfulArgumentConversionResult;

‎src/System.CommandLine/Binding/BindingContext.cs

+3-1
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ internal bool TryGetValueSource(
9595
internal bool TryBindToScalarValue(
9696
IValueDescriptor valueDescriptor,
9797
IValueSource valueSource,
98+
Resources resources,
9899
out BoundValue? boundValue)
99100
{
100101
if (valueSource.TryGetValue(valueDescriptor, this, out var value))
@@ -109,7 +110,8 @@ internal bool TryBindToScalarValue(
109110
var parsed = ArgumentConverter.ConvertObject(
110111
valueDescriptor as IArgument ?? new Argument(valueDescriptor.ValueName),
111112
valueDescriptor.ValueType,
112-
value);
113+
value,
114+
resources);
113115

114116
if (parsed is SuccessfulArgumentConversionResult successful)
115117
{

‎src/System.CommandLine/Binding/FailedArgumentTypeConversionResult.cs

+9-7
Original file line numberDiff line numberDiff line change
@@ -10,32 +10,34 @@ internal class FailedArgumentTypeConversionResult : FailedArgumentConversionResu
1010
internal FailedArgumentTypeConversionResult(
1111
IArgument argument,
1212
Type expectedType,
13-
string value) :
14-
base(argument, FormatErrorMessage(argument, expectedType, value))
13+
string value,
14+
Resources resources) :
15+
base(argument, FormatErrorMessage(argument, expectedType, value, resources))
1516
{
1617
}
1718

1819
private static string FormatErrorMessage(
1920
IArgument argument,
2021
Type expectedType,
21-
string value)
22+
string value,
23+
Resources resources)
2224
{
2325
if (argument is Argument a &&
2426
a.Parents.Count == 1)
2527
{
2628
var firstParent = (IIdentifierSymbol) a.Parents[0];
2729
var alias = firstParent.Aliases.First();
28-
30+
2931
switch(firstParent)
3032
{
3133
case ICommand _:
32-
return Resources.Instance.ArgumentConversionCannotParseForCommand(value, alias, expectedType);
34+
return resources.ArgumentConversionCannotParseForCommand(value, alias, expectedType);
3335
case IOption _:
34-
return Resources.Instance.ArgumentConversionCannotParseForOption(value, alias, expectedType);
36+
return resources.ArgumentConversionCannotParseForOption(value, alias, expectedType);
3537
}
3638
}
3739

38-
return Resources.Instance.ArgumentConversionCannotParse(value, expectedType);
40+
return resources.ArgumentConversionCannotParse(value, expectedType);
3941
}
4042
}
4143
}

‎src/System.CommandLine/Binding/ModelBinder.cs

+2
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ private bool ShortCutTheBinding()
115115
var valueSource = GetValueSource(bindingSources, bindingContext, ValueDescriptor, EnforceExplicitBinding);
116116
return bindingContext.TryBindToScalarValue(ValueDescriptor,
117117
valueSource,
118+
bindingContext.ParseResult.CommandResult.Resources,
118119
out var boundValue)
119120
? (true, boundValue?.Value, true)
120121
: (false,(object?) null, false);
@@ -263,6 +264,7 @@ internal static (BoundValue? boundValue, bool usedNonDefault) GetBoundValue(
263264
if (bindingContext.TryBindToScalarValue(
264265
valueDescriptor,
265266
valueSource,
267+
bindingContext.ParseResult.CommandResult.Resources,
266268
out var boundValue))
267269
{
268270
return (boundValue, true);

‎src/System.CommandLine/Builder/CommandLineBuilder.cs

+13
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,24 @@ public CommandLineBuilder(Command? rootCommand = null)
2929
internal Action<IHelpBuilder>? ConfigureHelp { get; set; }
3030

3131
internal HelpOption? HelpOption { get; set; }
32+
internal Option<bool>? VersionOption { get; set; }
3233

3334
internal Resources? Resources { get; set; }
3435

3536
public Parser Build()
3637
{
38+
var resources = Resources ?? Resources.Instance;
39+
40+
if (HelpOption is not null)
41+
{
42+
HelpOption.Description = resources.HelpOptionDescription();
43+
}
44+
45+
if (VersionOption is not null)
46+
{
47+
VersionOption.Description = resources.VersionOptionDescription();
48+
}
49+
3750
var rootCommand = Command;
3851

3952
var parser = new Parser(

‎src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs

+9-9
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ public static CommandLineBuilder UseDebugDirective(
230230
string debuggableProcessNames = GetEnvironmentVariable(environmentVariableName);
231231
if (string.IsNullOrWhiteSpace(debuggableProcessNames))
232232
{
233-
context.Console.Error.WriteLine(Resources.Instance.DebugDirectiveExecutableNotSpecified(environmentVariableName, process.ProcessName));
233+
context.Console.Error.WriteLine(context.Resources.DebugDirectiveExecutableNotSpecified(environmentVariableName, process.ProcessName));
234234
context.ExitCode = errorExitCode ?? 1;
235235
return;
236236
}
@@ -240,15 +240,15 @@ public static CommandLineBuilder UseDebugDirective(
240240
if (processNames.Contains(process.ProcessName, StringComparer.Ordinal))
241241
{
242242
var processId = process.Id;
243-
context.Console.Out.WriteLine(Resources.Instance.DebugDirectiveAttachToProcess(processId, process.ProcessName));
243+
context.Console.Out.WriteLine(context.Resources.DebugDirectiveAttachToProcess(processId, process.ProcessName));
244244
while (!Debugger.IsAttached)
245245
{
246246
await Task.Delay(500);
247247
}
248248
}
249249
else
250250
{
251-
context.Console.Error.WriteLine(Resources.Instance.DebugDirectiveProcessNotIncludedInEnvironmentVariable(process.ProcessName, environmentVariableName, debuggableProcessNames));
251+
context.Console.Error.WriteLine(context.Resources.DebugDirectiveProcessNotIncludedInEnvironmentVariable(process.ProcessName, environmentVariableName, debuggableProcessNames));
252252
context.ExitCode = errorExitCode ?? 1;
253253
return;
254254
}
@@ -329,7 +329,7 @@ void Default(Exception exception, InvocationContext context)
329329
context.Console.ResetTerminalForegroundColor();
330330
context.Console.SetTerminalForegroundRed();
331331

332-
context.Console.Error.Write(Resources.Instance.ExceptionHandlerHeader());
332+
context.Console.Error.Write(context.Resources.ExceptionHandlerHeader());
333333
context.Console.Error.WriteLine(exception.ToString());
334334

335335
context.Console.ResetTerminalForegroundColor();
@@ -524,14 +524,13 @@ public static CommandLineBuilder UseVersionOption(
524524
{
525525
var command = builder.Command;
526526

527-
if (command.Children.GetByAlias("--version") != null)
527+
if (builder.VersionOption is not null)
528528
{
529529
return builder;
530530
}
531-
531+
532532
var versionOption = new Option<bool>(
533533
"--version",
534-
description: Resources.Instance.VersionOptionDescription(),
535534
parseArgument: result =>
536535
{
537536
var commandChildren = result.FindResultFor(command)?.Children;
@@ -548,10 +547,10 @@ public static CommandLineBuilder UseVersionOption(
548547
{
549548
continue;
550549
}
551-
550+
552551
if (IsNotImplicit(symbolResult))
553552
{
554-
result.ErrorMessage = Resources.Instance.VersionOptionCannotBeCombinedWithOtherArguments("--version");
553+
result.ErrorMessage = result.Resources.VersionOptionCannotBeCombinedWithOtherArguments("--version");
555554
return false;
556555
}
557556
}
@@ -561,6 +560,7 @@ public static CommandLineBuilder UseVersionOption(
561560

562561
versionOption.DisallowBinding = true;
563562

563+
builder.VersionOption = versionOption;
564564
command.AddOption(versionOption);
565565

566566
builder.AddMiddleware(async (context, next) =>

‎src/System.CommandLine/CommandLineConfiguration.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ private static IHelpBuilder DefaultHelpBuilderFactory(BindingContext context)
112112
maxWidth = systemConsole.GetWindowWidth();
113113
}
114114

115-
return new HelpBuilder(context.Console, maxWidth);
115+
return new HelpBuilder(context.Console, context.ParseResult.CommandResult.Resources, maxWidth);
116116
}
117117

118118
private void AddGlobalOptionsToChildren(Command parentCommand)

0 commit comments

Comments
 (0)