Skip to content

Commit 4443107

Browse files
Remove CliExit and add SetSuccess/NotRun
The SetSuccess and NotRun methods are intended to abstract what those conditions mean from the actual properties that are set, probably making them private after discussio. I have more confidence in SetSuccess than NotRun
1 parent 5f31f7c commit 4443107

File tree

14 files changed

+65
-80
lines changed

14 files changed

+65
-80
lines changed

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

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,11 @@ internal class AlternateSubsystems
1010
{
1111
internal class AlternateVersion : VersionSubsystem
1212
{
13-
protected override CliExit Execute(PipelineResult pipelineResult)
13+
protected override PipelineResult Execute(PipelineResult pipelineResult)
1414
{
1515
pipelineResult.ConsoleHack.WriteLine($"***{CliExecutable.ExecutableVersion}***");
16-
pipelineResult.AlreadyHandled = true;
17-
return CliExit.SuccessfullyHandled(pipelineResult.ParseResult);
16+
pipelineResult.SetSuccess();
17+
return pipelineResult;
1818
}
1919
}
2020

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

3030
private CliSymbol Symbol { get; }
3131

32-
protected override CliExit Execute(PipelineResult pipelineResult)
32+
protected override PipelineResult Execute(PipelineResult pipelineResult)
3333
{
3434
TryGetAnnotation(Symbol, HelpAnnotations.Description, out string? description);
3535
pipelineResult.ConsoleHack.WriteLine(description);
3636
pipelineResult.AlreadyHandled = true;
37-
return CliExit.SuccessfullyHandled(pipelineResult.ParseResult);
37+
pipelineResult.SetSuccess();
38+
return pipelineResult;
3839
}
3940
}
4041

@@ -51,16 +52,16 @@ protected override CliConfiguration Initialize(InitializationContext context)
5152
return base.Initialize(context);
5253
}
5354

54-
protected override CliExit Execute(PipelineResult pipelineResult)
55+
protected override PipelineResult Execute(PipelineResult pipelineResult)
5556
{
5657
ExecutionWasRun = true;
5758
return base.Execute(pipelineResult);
5859
}
5960

60-
protected override CliExit TearDown(CliExit cliExit)
61+
protected override PipelineResult TearDown(PipelineResult pipelineResult)
6162
{
6263
TeardownWasRun = true;
63-
return base.TearDown(cliExit);
64+
return base.TearDown(pipelineResult);
6465
}
6566
}
6667

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public void Subsystem_runs_in_pipeline_only_when_requested(string input, bool sh
4343
var exit = pipeline.Execute(GetNewTestConfiguration(), input, console);
4444

4545
exit.ExitCode.Should().Be(0);
46-
exit.Handled.Should().Be(shouldRun);
46+
exit.AlreadyHandled.Should().Be(shouldRun);
4747
if (shouldRun)
4848
{
4949
console.GetBuffer().Trim().Should().Be(TestData.AssemblyVersionString);
@@ -61,7 +61,7 @@ public void Subsystem_runs_with_explicit_parse_only_when_requested(string input,
6161
var exit = pipeline.Execute(result, input, console);
6262

6363
exit.ExitCode.Should().Be(0);
64-
exit.Handled.Should().Be(shouldRun);
64+
exit.AlreadyHandled.Should().Be(shouldRun);
6565
if (shouldRun)
6666
{
6767
console.GetBuffer().Trim().Should().Be(TestData.AssemblyVersionString);
@@ -79,7 +79,7 @@ public void Subsystem_runs_initialize_and_teardown_when_requested(string input,
7979
var exit = pipeline.Execute(GetNewTestConfiguration(), input, console);
8080

8181
exit.ExitCode.Should().Be(0);
82-
exit.Handled.Should().Be(shouldRun);
82+
exit.AlreadyHandled.Should().Be(shouldRun);
8383
versionSubsystem.InitializationWasRun.Should().BeTrue();
8484
versionSubsystem.ExecutionWasRun.Should().Be(shouldRun);
8585
versionSubsystem.TeardownWasRun.Should().BeTrue();
@@ -109,7 +109,7 @@ public void Subsystem_works_without_pipeline(string input, bool shouldRun)
109109
var exit = Subsystem.Execute(versionSubsystem, parseResult, input, console);
110110
exit.Should().NotBeNull();
111111
exit.ExitCode.Should().Be(0);
112-
exit.Handled.Should().BeTrue();
112+
exit.AlreadyHandled.Should().BeTrue();
113113
console.GetBuffer().Trim().Should().Be(TestData.AssemblyVersionString);
114114
}
115115
}
@@ -132,7 +132,7 @@ public void Subsystem_works_without_pipeline_style2(string input, bool shouldRun
132132
var exit = Subsystem.ExecuteIfNeeded(versionSubsystem, parseResult, input, console);
133133

134134
exit.ExitCode.Should().Be(0);
135-
exit.Handled.Should().Be(shouldRun);
135+
exit.AlreadyHandled.Should().Be(shouldRun);
136136
console.GetBuffer().Trim().Should().Be(expectedVersion);
137137
}
138138

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public void When_the_version_option_is_specified_then_the_version_is_written_to_
2626
var exit = pipeline.Execute(configuration, "-v", consoleHack);
2727

2828
exit.ExitCode.Should().Be(0);
29-
exit.Handled.Should().BeTrue();
29+
exit.AlreadyHandled.Should().BeTrue();
3030
consoleHack.GetBuffer().Should().Be($"{version}{newLine}");
3131
}
3232

src/System.CommandLine.Subsystems/CliExit.cs

Lines changed: 0 additions & 35 deletions
This file was deleted.

src/System.CommandLine.Subsystems/CompletionSubsystem.cs

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

21-
protected internal override CliExit Execute(PipelineResult pipelineResult)
21+
protected internal override PipelineResult Execute(PipelineResult pipelineResult)
2222
{
2323
pipelineResult.ConsoleHack.WriteLine("Not yet implemented");
24-
return CliExit.SuccessfullyHandled(pipelineResult.ParseResult);
24+
pipelineResult.SetSuccess();
25+
return pipelineResult;
2526
}
2627
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,15 @@ 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 CliExit Execute(PipelineResult pipelineResult)
16+
protected internal override PipelineResult 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");
23-
return CliExit.SuccessfullyHandled(pipelineResult.ParseResult);
23+
pipelineResult.SetSuccess();
24+
return pipelineResult;
2425
}
2526

2627

src/System.CommandLine.Subsystems/ErrorReportingSubsystem.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,15 @@ 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 CliExit Execute(PipelineResult pipelineResult)
26+
protected internal override PipelineResult 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

33-
return CliExit.SuccessfullyHandled(pipelineResult.ParseResult);
33+
pipelineResult.SetSuccess();
34+
return pipelineResult;
3435
}
3536

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

src/System.CommandLine.Subsystems/HelpSubsystem.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,12 @@ protected internal override CliConfiguration Initialize(InitializationContext co
3434
protected internal override bool GetIsActivated(ParseResult? parseResult)
3535
=> parseResult is not null && parseResult.GetValue(HelpOption);
3636

37-
protected internal override CliExit Execute(PipelineResult pipelineResult)
37+
protected internal override PipelineResult Execute(PipelineResult pipelineResult)
3838
{
3939
// TODO: Match testable output pattern
4040
pipelineResult.ConsoleHack.WriteLine("Help me!");
41-
return CliExit.SuccessfullyHandled(pipelineResult.ParseResult);
41+
pipelineResult.SetSuccess();
42+
return pipelineResult;
4243
}
4344

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

src/System.CommandLine.Subsystems/Pipeline.cs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -51,20 +51,20 @@ public ParseResult Parse(CliConfiguration configuration, IReadOnlyList<string> a
5151
return parseResult;
5252
}
5353

54-
public CliExit Execute(CliConfiguration configuration, string rawInput, ConsoleHack? consoleHack = null)
54+
public PipelineResult Execute(CliConfiguration configuration, string rawInput, ConsoleHack? consoleHack = null)
5555
=> Execute(configuration, CliParser.SplitCommandLine(rawInput).ToArray(), rawInput, consoleHack);
5656

57-
public CliExit Execute(CliConfiguration configuration, string[] args, string rawInput, ConsoleHack? consoleHack = null)
57+
public PipelineResult Execute(CliConfiguration configuration, string[] args, string rawInput, ConsoleHack? consoleHack = null)
5858
{
59-
var cliExit = Execute(Parse(configuration, args), rawInput, consoleHack);
60-
return TearDownSubsystems(cliExit);
59+
var pipelineResult = Execute(Parse(configuration, args), rawInput, consoleHack);
60+
return TearDownSubsystems(pipelineResult);
6161
}
6262

63-
public CliExit Execute(ParseResult parseResult, string rawInput, ConsoleHack? consoleHack = null)
63+
public PipelineResult Execute(ParseResult parseResult, string rawInput, ConsoleHack? consoleHack = null)
6464
{
6565
var pipelineResult = new PipelineResult(parseResult, rawInput, this, consoleHack ?? new ConsoleHack());
6666
ExecuteSubsystems(pipelineResult);
67-
return new CliExit(pipelineResult);
67+
return pipelineResult;
6868
}
6969

7070
// TODO: Consider whether this should be public. It would simplify testing, but would it do anything else
@@ -95,18 +95,18 @@ protected virtual void InitializeSubsystems(InitializationContext context)
9595
/// Perform any cleanup operations
9696
/// </summary>
9797
/// <param name="pipelineResult">The context of the current execution</param>
98-
protected virtual CliExit TearDownSubsystems(CliExit cliExit)
98+
protected virtual PipelineResult TearDownSubsystems(PipelineResult pipelineResult)
9999
{
100-
// TODO: Work on this design as the last cliExit wins and they may not all be well behaved
100+
// TODO: Work on this design as the last pipelineResult wins and they may not all be well behaved
101101
var subsystems = Subsystems.Reverse();
102102
foreach (var subsystem in subsystems)
103103
{
104104
if (subsystem is not null)
105105
{
106-
cliExit = subsystem.TearDown(cliExit);
106+
pipelineResult = subsystem.TearDown(pipelineResult);
107107
}
108108
}
109-
return cliExit;
109+
return pipelineResult;
110110
}
111111

112112
protected virtual void ExecuteSubsystems(PipelineResult pipelineResult)

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,12 @@ protected internal bool TryGetAnnotation<TValue>(CliSymbol symbol, AnnotationId<
6666
/// Executes the behavior of the subsystem. For example, help would write information to the console.
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>
69-
/// <returns>A CliExit object with information such as whether the CLI should terminate</returns>
70-
protected internal virtual CliExit Execute(PipelineResult pipelineResult)
71-
=> CliExit.NotRun(pipelineResult.ParseResult);
69+
/// <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+
}
7275

7376
internal PipelineResult ExecuteIfNeeded(PipelineResult pipelineResult)
7477
=> ExecuteIfNeeded(pipelineResult.ParseResult, pipelineResult);
@@ -111,7 +114,7 @@ protected internal virtual CliConfiguration Initialize(InitializationContext con
111114
=> context.Configuration;
112115

113116
// TODO: Determine if this is needed.
114-
protected internal virtual CliExit TearDown(CliExit cliExit)
115-
=> cliExit;
117+
protected internal virtual PipelineResult TearDown(PipelineResult pipelineResult)
118+
=> pipelineResult;
116119

117120
}

0 commit comments

Comments
 (0)