Skip to content

Commit 7013b47

Browse files
Fluent style obscures that there is a single PipelineResult that is mutated.
1 parent 4443107 commit 7013b47

File tree

12 files changed

+44
-58
lines changed

12 files changed

+44
-58
lines changed

src/System.CommandLine.Subsystems.Tests/AlternateSubsystems.cs

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,10 @@ internal class AlternateSubsystems
1010
{
1111
internal class AlternateVersion : VersionSubsystem
1212
{
13-
protected override PipelineResult Execute(PipelineResult pipelineResult)
13+
protected override void Execute(PipelineResult pipelineResult)
1414
{
1515
pipelineResult.ConsoleHack.WriteLine($"***{CliExecutable.ExecutableVersion}***");
1616
pipelineResult.SetSuccess();
17-
return pipelineResult;
1817
}
1918
}
2019

@@ -29,13 +28,12 @@ public VersionThatUsesHelpData(CliSymbol symbol)
2928

3029
private CliSymbol Symbol { get; }
3130

32-
protected override PipelineResult Execute(PipelineResult pipelineResult)
31+
protected override void Execute(PipelineResult pipelineResult)
3332
{
3433
TryGetAnnotation(Symbol, HelpAnnotations.Description, out string? description);
3534
pipelineResult.ConsoleHack.WriteLine(description);
3635
pipelineResult.AlreadyHandled = true;
3736
pipelineResult.SetSuccess();
38-
return pipelineResult;
3937
}
4038
}
4139

@@ -45,23 +43,23 @@ internal class VersionWithInitializeAndTeardown : VersionSubsystem
4543
internal bool ExecutionWasRun;
4644
internal bool TeardownWasRun;
4745

48-
protected override CliConfiguration Initialize(InitializationContext context)
46+
protected override void Initialize(InitializationContext context)
4947
{
48+
base.Initialize(context);
5049
// marker hack needed because ConsoleHack not available in initialization
5150
InitializationWasRun = true;
52-
return base.Initialize(context);
5351
}
5452

55-
protected override PipelineResult Execute(PipelineResult pipelineResult)
53+
protected override void Execute(PipelineResult pipelineResult)
5654
{
5755
ExecutionWasRun = true;
58-
return base.Execute(pipelineResult);
56+
base.Execute(pipelineResult);
5957
}
6058

61-
protected override PipelineResult TearDown(PipelineResult pipelineResult)
59+
protected override void TearDown(PipelineResult pipelineResult)
6260
{
6361
TeardownWasRun = true;
64-
return base.TearDown(pipelineResult);
62+
base.TearDown(pipelineResult);
6563
}
6664
}
6765

src/System.CommandLine.Subsystems/CompletionSubsystem.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,9 @@ protected internal override bool GetIsActivated(ParseResult? parseResult)
1818
? false
1919
: false;
2020

21-
protected internal override PipelineResult Execute(PipelineResult pipelineResult)
21+
protected internal override void Execute(PipelineResult pipelineResult)
2222
{
2323
pipelineResult.ConsoleHack.WriteLine("Not yet implemented");
2424
pipelineResult.SetSuccess();
25-
return pipelineResult;
2625
}
2726
}

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,14 @@ public class DiagramSubsystem( IAnnotationProvider? annotationProvider = null)
1313
//protected internal override bool GetIsActivated(ParseResult? parseResult)
1414
// => parseResult is not null && option is not null && parseResult.GetValue(option);
1515

16-
protected internal override PipelineResult Execute(PipelineResult pipelineResult)
16+
protected internal override void Execute(PipelineResult pipelineResult)
1717
{
1818
// Gather locations
1919
//var locations = pipelineResult.ParseResult.LocationMap
2020
// .Concat(Map(pipelineResult.ParseResult.Configuration.PreProcessedLocations));
2121

2222
pipelineResult.ConsoleHack.WriteLine("Output diagram");
2323
pipelineResult.SetSuccess();
24-
return pipelineResult;
2524
}
2625

2726

src/System.CommandLine.Subsystems/Directives/DirectiveSubsystem.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ public DirectiveSubsystem(string name, SubsystemKind kind, IAnnotationProvider?
1919
Id = id ?? name;
2020
}
2121

22-
protected internal override CliConfiguration Initialize(InitializationContext context)
22+
protected internal override void Initialize(InitializationContext context)
2323
{
2424
for (int i = 0; i < context.Args.Count; i++)
2525
{
@@ -50,8 +50,6 @@ protected internal override CliConfiguration Initialize(InitializationContext co
5050
break;
5151
}
5252
}
53-
54-
return context.Configuration;
5553
}
5654

5755
protected internal override bool GetIsActivated(ParseResult? parseResult)

src/System.CommandLine.Subsystems/Directives/ResponseSubsystem.cs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,8 @@ namespace System.CommandLine.Directives;
99
public class ResponseSubsystem()
1010
: CliSubsystem("Response", SubsystemKind.Response, null)
1111
{
12-
protected internal override CliConfiguration Initialize(InitializationContext context)
13-
{
14-
context.Configuration.ResponseFileTokenReplacer = Replacer;
15-
return context.Configuration;
16-
}
12+
protected internal override void Initialize(InitializationContext context)
13+
=> context.Configuration.ResponseFileTokenReplacer = Replacer;
1714

1815
public static (List<string>? tokens, List<string>? errors) Replacer(string responseSourceName)
1916
{

src/System.CommandLine.Subsystems/ErrorReportingSubsystem.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,14 @@ protected internal override bool GetIsActivated(ParseResult? parseResult)
2323
=> parseResult is not null && parseResult.Errors.Any();
2424

2525
// TODO: properly test execute directly when parse result is usable in tests
26-
protected internal override PipelineResult Execute(PipelineResult pipelineResult)
26+
protected internal override void Execute(PipelineResult pipelineResult)
2727
{
2828
var _ = pipelineResult.ParseResult
2929
?? throw new ArgumentException("The parse result has not been set", nameof(pipelineResult));
3030

3131
Report(pipelineResult.ConsoleHack, pipelineResult.ParseResult.Errors);
3232

3333
pipelineResult.SetSuccess();
34-
return pipelineResult;
3534
}
3635

3736
public void Report(ConsoleHack consoleHack, IReadOnlyList<ParseError> errors)

src/System.CommandLine.Subsystems/HelpSubsystem.cs

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,22 +24,17 @@ public class HelpSubsystem(IAnnotationProvider? annotationProvider = null)
2424
Arity = ArgumentArity.Zero
2525
};
2626

27-
protected internal override CliConfiguration Initialize(InitializationContext context)
28-
{
29-
context.Configuration.RootCommand.Add(HelpOption);
30-
31-
return context.Configuration;
32-
}
27+
protected internal override void Initialize(InitializationContext context)
28+
=> context.Configuration.RootCommand.Add(HelpOption);
3329

3430
protected internal override bool GetIsActivated(ParseResult? parseResult)
3531
=> parseResult is not null && parseResult.GetValue(HelpOption);
3632

37-
protected internal override PipelineResult Execute(PipelineResult pipelineResult)
33+
protected internal override void Execute(PipelineResult pipelineResult)
3834
{
3935
// TODO: Match testable output pattern
4036
pipelineResult.ConsoleHack.WriteLine("Help me!");
4137
pipelineResult.SetSuccess();
42-
return pipelineResult;
4338
}
4439

4540
public bool TryGetDescription (CliSymbol symbol, out string? description)

src/System.CommandLine.Subsystems/Pipeline.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,8 @@ public PipelineResult Execute(CliConfiguration configuration, string rawInput, C
5757
public PipelineResult Execute(CliConfiguration configuration, string[] args, string rawInput, ConsoleHack? consoleHack = null)
5858
{
5959
var pipelineResult = Execute(Parse(configuration, args), rawInput, consoleHack);
60-
return TearDownSubsystems(pipelineResult);
60+
TearDownSubsystems(pipelineResult);
61+
return pipelineResult;
6162
}
6263

6364
public PipelineResult Execute(ParseResult parseResult, string rawInput, ConsoleHack? consoleHack = null)
@@ -95,18 +96,17 @@ protected virtual void InitializeSubsystems(InitializationContext context)
9596
/// Perform any cleanup operations
9697
/// </summary>
9798
/// <param name="pipelineResult">The context of the current execution</param>
98-
protected virtual PipelineResult TearDownSubsystems(PipelineResult pipelineResult)
99+
protected virtual void TearDownSubsystems(PipelineResult pipelineResult)
99100
{
100101
// TODO: Work on this design as the last pipelineResult wins and they may not all be well behaved
101102
var subsystems = Subsystems.Reverse();
102103
foreach (var subsystem in subsystems)
103104
{
104105
if (subsystem is not null)
105106
{
106-
pipelineResult = subsystem.TearDown(pipelineResult);
107+
subsystem.TearDown(pipelineResult);
107108
}
108109
}
109-
return pipelineResult;
110110
}
111111

112112
protected virtual void ExecuteSubsystems(PipelineResult pipelineResult)
@@ -117,7 +117,7 @@ protected virtual void ExecuteSubsystems(PipelineResult pipelineResult)
117117
{
118118
if (subsystem is not null)
119119
{
120-
pipelineResult = subsystem.ExecuteIfNeeded(pipelineResult);
120+
subsystem.ExecuteIfNeeded(pipelineResult);
121121
}
122122
}
123123
}

src/System.CommandLine.Subsystems/Subsystems/CliSubsystem.cs

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,8 @@ protected internal bool TryGetAnnotation<TValue>(CliSymbol symbol, AnnotationId<
6767
/// </summary>
6868
/// <param name="pipelineResult">The context contains data like the ParseResult, and allows setting of values like whether execution was handled and the CLI should terminate </param>
6969
/// <returns>A PipelineResult object with information such as whether the CLI should terminate</returns>
70-
protected internal virtual PipelineResult Execute(PipelineResult pipelineResult)
71-
{
72-
pipelineResult.NotRun(pipelineResult.ParseResult);
73-
return pipelineResult;
74-
}
70+
protected internal virtual void Execute(PipelineResult pipelineResult)
71+
=> pipelineResult.NotRun(pipelineResult.ParseResult);
7572

7673
internal PipelineResult ExecuteIfNeeded(PipelineResult pipelineResult)
7774
=> ExecuteIfNeeded(pipelineResult.ParseResult, pipelineResult);
@@ -110,11 +107,11 @@ internal PipelineResult ExecuteIfNeeded(ParseResult? parseResult, PipelineResult
110107
/// <returns>True if parsing should continue</returns> // there might be a better design that supports a message
111108
// TODO: Because of this and similar usage, consider combining CLI declaration and config. ArgParse calls this the parser, which I like
112109
// TODO: Why does Intitialize return a configuration?
113-
protected internal virtual CliConfiguration Initialize(InitializationContext context)
114-
=> context.Configuration;
110+
protected internal virtual void Initialize(InitializationContext context)
111+
{}
115112

116113
// TODO: Determine if this is needed.
117-
protected internal virtual PipelineResult TearDown(PipelineResult pipelineResult)
118-
=> pipelineResult;
114+
protected internal virtual void TearDown(PipelineResult pipelineResult)
115+
{}
119116

120117
}

src/System.CommandLine.Subsystems/Subsystems/Subsystem.cs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ public class Subsystem
88
public static void Initialize(CliSubsystem subsystem, CliConfiguration configuration, IReadOnlyList<string> args)
99
=> subsystem.Initialize(new InitializationContext(configuration, args));
1010

11-
public static PipelineResult Execute(CliSubsystem subsystem, PipelineResult pipelineResult)
11+
public static void Execute(CliSubsystem subsystem, PipelineResult pipelineResult)
1212
=> subsystem.Execute(pipelineResult);
1313

1414
public static bool GetIsActivated(CliSubsystem subsystem, ParseResult parseResult)
@@ -18,12 +18,19 @@ public static PipelineResult ExecuteIfNeeded(CliSubsystem subsystem, ParseResult
1818
=> subsystem.ExecuteIfNeeded(new PipelineResult(parseResult, rawInput, null, consoleHack));
1919

2020
public static PipelineResult Execute(CliSubsystem subsystem, ParseResult parseResult, string rawInput, ConsoleHack? consoleHack = null)
21-
=> subsystem.Execute(new PipelineResult(parseResult, rawInput, null, consoleHack));
22-
21+
{
22+
var pipelineResult = new PipelineResult(parseResult, rawInput,null, consoleHack);
23+
subsystem.Execute(pipelineResult);
24+
return pipelineResult;
25+
}
2326

2427
internal static PipelineResult ExecuteIfNeeded(CliSubsystem subsystem, ParseResult parseResult, string rawInput, ConsoleHack? consoleHack, PipelineResult? pipelineResult = null)
25-
=> subsystem.ExecuteIfNeeded(pipelineResult ?? new PipelineResult(parseResult, rawInput, null, consoleHack));
28+
{
29+
pipelineResult ??= new PipelineResult(parseResult, rawInput, null, consoleHack);
30+
subsystem.ExecuteIfNeeded(pipelineResult );
31+
return pipelineResult;
32+
}
2633

27-
internal static PipelineResult ExecuteIfNeeded(CliSubsystem subsystem, PipelineResult pipelineResult)
34+
internal static void ExecuteIfNeeded(CliSubsystem subsystem, PipelineResult pipelineResult)
2835
=> subsystem.ExecuteIfNeeded(pipelineResult);
2936
}

0 commit comments

Comments
 (0)