Skip to content

Commit 59201ca

Browse files
KathleenDollardKathleenDollard
committed
Changed ValueSubsystem to ValueProvider
ValueProvider does not execute or have any characteristics of a subsystem This commit included the other PRs from this morning to get it building and testing. Messy.
1 parent 66e9399 commit 59201ca

File tree

12 files changed

+273
-264
lines changed

12 files changed

+273
-264
lines changed

‎src/System.CommandLine.Subsystems.Tests/ValueSubsystemTests.cs

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,25 +10,25 @@ namespace System.CommandLine.Subsystems.Tests;
1010

1111
public class ValueSubsystemTests
1212
{
13-
[Fact]
14-
public void ValueSubsystem_is_activated_by_default()
15-
{
16-
CliRootCommand rootCommand = [
17-
new CliCommand("x")
18-
{
19-
new CliOption<string>("--opt1")
20-
}];
21-
var configuration = new CliConfiguration(rootCommand);
22-
var subsystem = new ValueSubsystem();
23-
var input = "x --opt1 Kirk";
24-
var args = CliParser.SplitCommandLine(input).ToList();
13+
//[Fact]
14+
//public void ValueSubsystem_is_activated_by_default()
15+
//{
16+
// CliRootCommand rootCommand = [
17+
// new CliCommand("x")
18+
// {
19+
// new CliOption<string>("--opt1")
20+
// }];
21+
// var configuration = new CliConfiguration(rootCommand);
22+
// var subsystem = new ValueProvider();
23+
// var input = "x --opt1 Kirk";
24+
// var args = CliParser.SplitCommandLine(input).ToList();
2525

26-
Subsystem.Initialize(subsystem, configuration, args);
27-
var parseResult = CliParser.Parse(rootCommand, args[0], configuration);
28-
var isActive = Subsystem.GetIsActivated(subsystem, parseResult);
26+
// Subsystem.Initialize(subsystem, configuration, args);
27+
// var parseResult = CliParser.Parse(rootCommand, args[0], configuration);
28+
// var isActive = Subsystem.GetIsActivated(subsystem, parseResult);
2929

30-
isActive.Should().BeTrue();
31-
}
30+
// isActive.Should().BeTrue();
31+
//}
3232
/* Hold these tests until we determine if ValueSubsystem is replaceable
3333
[Fact]
3434
public void ValueSubsystem_returns_values_that_are_entered()

‎src/System.CommandLine.Subsystems/Directives/DiagramSubsystem.cs

Lines changed: 68 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -76,78 +76,78 @@ private static void Diagram(
7676
break;
7777
*/
7878

79-
// TODO: This logic is deeply tied to internal types/properties. These aren't things we probably want to expose like SymbolNode. See #2349 for alternatives
80-
/*
81-
case ArgumentResult argumentResult:
79+
// TODO: This logic is deeply tied to internal types/properties. These aren't things we probably want to expose like SymbolNode. See #2349 for alternatives
80+
/*
81+
case ArgumentResult argumentResult:
82+
{
83+
var includeArgumentName =
84+
argumentResult.Argument.FirstParent!.Symbol is CliCommand { HasArguments: true, Arguments.Count: > 1 };
85+
86+
if (includeArgumentName)
8287
{
83-
var includeArgumentName =
84-
argumentResult.Argument.FirstParent!.Symbol is CliCommand { HasArguments: true, Arguments.Count: > 1 };
88+
builder.Append("[ ");
89+
builder.Append(argumentResult.Argument.Name);
90+
builder.Append(' ');
91+
}
8592
86-
if (includeArgumentName)
93+
if (argumentResult.Argument.Arity.MaximumNumberOfValues > 0)
94+
{
95+
ArgumentConversionResult conversionResult = argumentResult.GetArgumentConversionResult();
96+
switch (conversionResult.Result)
8797
{
88-
builder.Append("[ ");
89-
builder.Append(argumentResult.Argument.Name);
90-
builder.Append(' ');
98+
case ArgumentConversionResultType.NoArgument:
99+
break;
100+
case ArgumentConversionResultType.Successful:
101+
switch (conversionResult.Value)
102+
{
103+
case string s:
104+
builder.Append($"<{s}>");
105+
break;
106+
107+
case IEnumerable items:
108+
builder.Append('<');
109+
builder.Append(
110+
string.Join("> <",
111+
items.Cast<object>().ToArray()));
112+
builder.Append('>');
113+
break;
114+
115+
default:
116+
builder.Append('<');
117+
builder.Append(conversionResult.Value);
118+
builder.Append('>');
119+
break;
120+
}
121+
122+
break;
123+
124+
default: // failures
125+
builder.Append('<');
126+
builder.Append(string.Join("> <", symbolResult.Tokens.Select(t => t.Value)));
127+
builder.Append('>');
128+
129+
break;
91130
}
131+
}
92132
93-
if (argumentResult.Argument.Arity.MaximumNumberOfValues > 0)
94-
{
95-
ArgumentConversionResult conversionResult = argumentResult.GetArgumentConversionResult();
96-
switch (conversionResult.Result)
97-
{
98-
case ArgumentConversionResultType.NoArgument:
99-
break;
100-
case ArgumentConversionResultType.Successful:
101-
switch (conversionResult.Value)
102-
{
103-
case string s:
104-
builder.Append($"<{s}>");
105-
break;
106-
107-
case IEnumerable items:
108-
builder.Append('<');
109-
builder.Append(
110-
string.Join("> <",
111-
items.Cast<object>().ToArray()));
112-
builder.Append('>');
113-
break;
114-
115-
default:
116-
builder.Append('<');
117-
builder.Append(conversionResult.Value);
118-
builder.Append('>');
119-
break;
120-
}
121-
122-
break;
123-
124-
default: // failures
125-
builder.Append('<');
126-
builder.Append(string.Join("> <", symbolResult.Tokens.Select(t => t.Value)));
127-
builder.Append('>');
128-
129-
break;
130-
}
131-
}
133+
if (includeArgumentName)
134+
{
135+
builder.Append(" ]");
136+
}
132137
133-
if (includeArgumentName)
134-
{
135-
builder.Append(" ]");
136-
}
138+
break;
139+
}
137140
138-
break;
139-
}
141+
default:
142+
{
143+
OptionResult? optionResult = symbolResult as OptionResult;
140144
141-
default:
145+
if (optionResult is { Implicit: true })
142146
{
143-
OptionResult? optionResult = symbolResult as OptionResult;
147+
builder.Append('*');
148+
}
144149
145-
if (optionResult is { Implicit: true })
146-
{
147-
builder.Append('*');
148-
}
149-
150-
builder.Append("[ ");
150+
builder.Append("[ ");
151151
152152
if (optionResult is not null)
153153
{
@@ -167,16 +167,16 @@ private static void Diagram(
167167
continue;
168168
}
169169
170-
builder.Append(' ');
171-
172-
Diagram(builder, child, parseResult);
173-
}
170+
builder.Append(' ');
174171
175-
builder.Append(" ]");
176-
break;
172+
Diagram(builder, child, parseResult);
177173
}
174+
175+
builder.Append(" ]");
176+
break;
178177
}
179178
}
180179
}
180+
}
181181
*/
182182
}

‎src/System.CommandLine.Subsystems/Pipeline.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public partial class Pipeline
2828
/// <param name="errorReporting">A help subsystem to replace the standard one. To add a subsystem, use <see cref="AddSubsystem"></param>
2929
/// <returns>A new pipeline.</returns>
3030
/// <remarks>
31-
/// Currently, the standard <see cref="ValueSubsystem"/>, <see cref="ValidationSubsystem"/> , and <see cref="ResponseSubsystem"/> cannot be replaced. <see cref="ResponseSubsystem"/> is disabled by default.
31+
/// The ValueProvider, ResponseSubystem, InvocationSubsystem, and ValidationSubsystem cannot be replaced.
3232
/// </remarks>
3333
public static Pipeline Create(HelpSubsystem? help = null,
3434
VersionSubsystem? version = null,
@@ -56,7 +56,7 @@ public static Pipeline CreateEmpty()
5656

5757
private Pipeline()
5858
{
59-
Value = new ValueSubsystem();
59+
//Value = new ValueProvider();
6060
Response = new ResponseSubsystem();
6161
Invocation = new InvocationSubsystem();
6262
Validation = new ValidationSubsystem();
@@ -172,20 +172,20 @@ public ErrorReportingSubsystem? ErrorReporting
172172

173173
// TODO: Consider whether replacing the validation subsystem is valuable
174174
/// <summary>
175-
/// Sets or gets the validation subsystem
175+
/// Gets the validation subsystem
176176
/// </summary>
177177
public ValidationSubsystem? Validation { get; }
178178

179179
// TODO: Consider whether replacing the invocation subsystem is valuable
180180
/// <summary>
181-
/// Sets or gets the invocation subsystem
181+
/// Gets the invocation subsystem
182182
/// </summary>
183183
public InvocationSubsystem? Invocation { get; }
184184

185-
/// <summary>
186-
/// Gets the value subsystem which manages entered and default values.
187-
/// </summary>
188-
public ValueSubsystem Value { get; }
185+
///// <summary>
186+
///// Gets the value subsystem which manages entered and default values.
187+
///// </summary>
188+
//public ValueProvider Value { get; }
189189

190190
/// <summary>
191191
/// Gets the response file subsystem

‎src/System.CommandLine.Subsystems/PipelineResult.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@ public class PipelineResult(ParseResult? parseResult, string rawInput, Pipeline?
99
{
1010
private readonly List<ParseError> errors = [];
1111
public ParseResult? ParseResult { get; } = parseResult;
12+
private ValueProvider valueProvider { get; } = new ValueProvider(parseResult);
1213
public string RawInput { get; } = rawInput;
14+
15+
// TODO: Consider behavior when pipeline is null - this is probably a core user accessing some subsystems
1316
public Pipeline Pipeline { get; } = pipeline ?? Pipeline.CreateEmpty();
1417
public ConsoleHack ConsoleHack { get; } = consoleHack ?? new ConsoleHack();
1518

‎src/System.CommandLine.Subsystems/Subsystems/PipelinePhase.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ private List<CliSubsystem> CreateAfterIfNeeded()
4141
return after;
4242
}
4343

44-
public IEnumerable<CliSubsystem> GetSubsystems()
44+
public IEnumerable<CliSubsystem> GetSubsystems()
4545
=> [
4646
.. (before is null ? [] : before),
4747
.. (CliSubsystem is null ? new List<CliSubsystem> { } : [CliSubsystem]),

‎src/System.CommandLine.Subsystems/ValueAnnotationExtensions.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public static void SetDefaultValue<TValue>(this CliOption<TValue> option, TValue
3939
/// <param name="option">The option</param>
4040
/// <returns>The option's default value annotation if any, otherwise <see langword="null"/></returns>
4141
/// <remarks>
42-
/// This is intended to be called by CLI authors. Subsystems should instead call <see cref="ValueSubsystem.GetValue{T}(CliOption)"/>,
42+
/// This is intended to be called by CLI authors. Subsystems should instead call <see cref="PipelineResult.GetValue{T}(CliOption)"/>,
4343
/// which calculates the actual default value, based on the default value annotation and default value calculation,
4444
/// whether directly stored on the symbol or from the subsystem's <see cref="IAnnotationProvider"/>.
4545
/// </remarks>
@@ -84,7 +84,7 @@ public static void SetDefaultValue<TValue>(this CliArgument<TValue> argument, TV
8484
/// <param name="argument">The argument</param>
8585
/// <returns>The argument's default value annotation if any, otherwise <see langword="null"/></returns>
8686
/// <remarks>
87-
/// This is intended to be called by CLI authors. Subsystems should instead call <see cref="ValueSubsystem.GetValue{T}(CliArgument)"/>,
87+
/// This is intended to be called by CLI authors. Subsystems should instead call <see cref="PipelineResult.GetValue{T}(CliArgument)"/>,
8888
/// which calculates the actual default value, based on the default value annotation and default value calculation,
8989
/// whether directly stored on the symbol or from the subsystem's <see cref="IAnnotationProvider"/>.
9090
/// </remarks>
@@ -128,7 +128,7 @@ public static void SetDefaultValueCalculation<TValue>(this CliOption<TValue> opt
128128
/// <param name="option">The option</param>
129129
/// <returns>The option's default value calculation if any, otherwise <see langword="null"/></returns>
130130
/// <remarks>
131-
/// This is intended to be called by CLI authors. Subsystems should instead call <see cref="ValueSubsystem.GetValue{T}(CliOption)"/>,
131+
/// This is intended to be called by CLI authors. Subsystems should instead call <see cref="PipelineResult.GetValue{T}(CliOption)"/>,
132132
/// which calculates the actual default value, based on the default value annotation and default value calculation,
133133
/// whether directly stored on the symbol or from the subsystem's <see cref="IAnnotationProvider"/>.
134134
/// </remarks>
@@ -173,7 +173,7 @@ public static void SetDefaultValueCalculation<TValue>(this CliArgument<TValue> a
173173
/// <param name="argument">The argument</param>
174174
/// <returns>The argument's default value calculation if any, otherwise <see langword="null"/></returns>
175175
/// <remarks>
176-
/// This is intended to be called by CLI authors. Subsystems should instead call <see cref="ValueSubsystem.GetValue{T}(CliArgument)"/>,
176+
/// This is intended to be called by CLI authors. Subsystems should instead call <see cref="PipelineResult.GetValue{T}(CliArgument)"/>,
177177
/// which calculates the actual default value, based on the default value annotation and default value calculation,
178178
/// whether directly stored on the symbol or from the subsystem's <see cref="IAnnotationProvider"/>.
179179
/// </remarks>

‎src/System.CommandLine.Subsystems/ValueSubsystem.cs renamed to ‎src/System.CommandLine.Subsystems/ValueProvider.cs

Lines changed: 6 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -6,36 +6,17 @@
66

77
namespace System.CommandLine;
88

9-
public class ValueSubsystem(IAnnotationProvider? annotationProvider = null)
10-
: CliSubsystem(ValueAnnotations.Prefix, SubsystemKind.Value, annotationProvider)
9+
internal class ValueProvider
1110
{
1211
private Dictionary<CliSymbol, object?> cachedValues = [];
1312
private ParseResult? parseResult = null;
1413

15-
// It is possible that another subsystems GetIsActivated method will access a value.
16-
// If this is called from a GetIsActivated method of a subsystem in the early termination group,
17-
// it will fail. That is not an expected scenario.
18-
/// <inheritdoc cref="CliSubsystem.GetIsActivated"/>
19-
/// <remarks>
20-
/// Note to inheritors: Call base for all ValueSubsystem methods that you override to ensure correct behavior
21-
/// </remarks>
22-
protected internal override bool GetIsActivated(ParseResult? parseResult)
14+
public ValueProvider(ParseResult parseResult)
2315
{
2416
this.parseResult = parseResult;
25-
return true;
2617
}
2718

28-
/// <inheritdoc cref="CliSubsystem.Execute"/>
29-
/// <remarks>
30-
/// Note to inheritors: Call base for all ValueSubsystem methods that you override to ensure correct behavior
31-
/// </remarks>
32-
protected internal override void Execute(PipelineResult pipelineResult)
33-
{
34-
parseResult ??= pipelineResult.ParseResult;
35-
base.Execute(pipelineResult);
36-
}
37-
38-
private void SetValue<T>(CliSymbol symbol, object? value)
19+
private void SetValue(CliSymbol symbol, object? value)
3920
{
4021
cachedValues[symbol] = value;
4122
}
@@ -53,10 +34,8 @@ private bool TryGetValue<T>(CliSymbol symbol, out T? value)
5334
return false;
5435
}
5536

56-
public T? GetValue<T>(CliOption option)
57-
=> GetValueInternal<T>(option);
58-
public T? GetValue<T>(CliArgument argument)
59-
=> GetValueInternal<T>(argument);
37+
public T? GetValue<T>(CliDataSymbol dataSymbol)
38+
=> GetValueInternal<T>(dataSymbol);
6039

6140
private T? GetValueInternal<T>(CliSymbol? symbol)
6241
{
@@ -84,7 +63,7 @@ not null when TryGetAnnotation(symbol, ValueAnnotations.DefaultValue, out T? exp
8463

8564
TValue? UseValue<TValue>(CliSymbol symbol, TValue? value)
8665
{
87-
SetValue<TValue>(symbol, value);
66+
SetValue(symbol, value);
8867
return value;
8968
}
9069
}

0 commit comments

Comments
 (0)