diff --git a/TrayApp.Test/CommandLineParserTests.cs b/TrayApp.Test/CommandLineParserTests.cs index dbe3ee8..c685c21 100644 --- a/TrayApp.Test/CommandLineParserTests.cs +++ b/TrayApp.Test/CommandLineParserTests.cs @@ -186,6 +186,40 @@ public void TryParse_WithAnApplicationValueOptionAndNoValue_Fails() Assert.Contains("needs a value", error, StringComparison.Ordinal); } + [TestMethod] + [DataRow("--display")] + [DataRow("-d")] + [DataRow("--status")] + [DataRow("-t")] + public void TryParse_WithAnApplicationValueOptionFollowedByASwitch_FailsInsteadOfTakingTheSwitchAsItsValue(string next) + { + AppOption reason = AppOption.Value(["-r", "--reason"], "text", "Why.", _ => { }); + AppOption display = AppOption.Flag(["-d", "--display"], "Keep the display on.", () => { }); + + Assert.IsFalse(CommandLineParser.TryParse(["--reason", next], [reason, display], out _, out string error)); + Assert.AreEqual("'--reason' needs a value.", error); + } + + [TestMethod] + public void TryParse_WithADurationFollowedByASwitch_SaysTheValueIsMissing() + { + Assert.IsFalse(CommandLineParser.TryParse(["--for", "--status"], NoOptions, out _, out string error)); + Assert.AreEqual("'--for' needs a value.", error); + } + + [TestMethod] + [DataRow("-")] + [DataRow("-5")] + [DataRow("some text")] + public void TryParse_WithAValueThatIsNotAKnownSwitch_TakesIt(string value) + { + AppOption reason = AppOption.Value(["-r", "--reason"], "text", "Why.", _ => { }); + + Assert.IsTrue(CommandLineParser.TryParse(["--reason", value], [reason], out CommandLineOptions options, out _)); + Assert.AreEqual(1, options.Matches.Count); + Assert.AreEqual(value, options.Matches[0].Value); + } + [TestMethod] public void TryParse_WithEverything_KeepsTheOrderOfApplicationOptions() { diff --git a/TrayApp.Test/TrayAppBuilderTests.cs b/TrayApp.Test/TrayAppBuilderTests.cs index 0ec23c6..a738bfa 100644 --- a/TrayApp.Test/TrayAppBuilderTests.cs +++ b/TrayApp.Test/TrayAppBuilderTests.cs @@ -277,6 +277,21 @@ public async Task RunAsync_WithABadCommandLine_ExitsTwo() Assert.AreEqual(2, exitCode); } + [TestMethod] + public async Task RunAsync_WithAValueOptionMissingItsValueBeforeStatus_ExitsTwo() + { + bool started = false; + + int exitCode = await TrayAppBuilder.Create("demo") + .Option(["-r", "--reason"], "text", "Why.", _ => { }) + .OnStart(() => started = true) + .RunAsync(["--no-tray", "--for", "0.05s", "--reason", "--status"]) + .ConfigureAwait(false); + + Assert.AreEqual(2, exitCode); + Assert.IsFalse(started); + } + [TestMethod] public async Task RunAsync_WithStatus_ReportsWithoutStarting() { diff --git a/TrayApp/Cli/CommandLineParser.cs b/TrayApp/Cli/CommandLineParser.cs index ced4b65..b872d15 100644 --- a/TrayApp/Cli/CommandLineParser.cs +++ b/TrayApp/Cli/CommandLineParser.cs @@ -61,7 +61,7 @@ public static bool TryParse( for (int index = 0; index < args.Count; index++) { string argument = args[index]; - ReadResult result = ReadStandardOption(args, ref index, argument, state, out error); + ReadResult result = ReadStandardOption(args, ref index, argument, appOptions, state, out error); if (result == ReadResult.Failed || (result == ReadResult.NotRecognised && !TryReadAppOption(args, ref index, argument, appOptions, state, out error))) @@ -87,6 +87,7 @@ private static ReadResult ReadStandardOption( IReadOnlyList args, ref int index, string argument, + IReadOnlyList appOptions, ParseState state, out string error) { @@ -115,7 +116,7 @@ private static ReadResult ReadStandardOption( return ReadResult.Read; case "-f" or "--for": - return ReadDuration(args, ref index, argument, state, out error); + return ReadDuration(args, ref index, argument, appOptions, state, out error); default: return ReadResult.NotRecognised; @@ -126,10 +127,11 @@ private static ReadResult ReadDuration( IReadOnlyList args, ref int index, string argument, + IReadOnlyList appOptions, ParseState state, out string error) { - if (!TryTakeValue(args, ref index, argument, out string text, out error)) + if (!TryTakeValue(args, ref index, argument, appOptions, out string text, out error)) { return ReadResult.Failed; } @@ -170,7 +172,7 @@ private static bool TryReadAppOption( string? value = null; - if (matched.TakesValue && !TryTakeValue(args, ref index, argument, out value, out error)) + if (matched.TakesValue && !TryTakeValue(args, ref index, argument, appOptions, out value, out error)) { return false; } @@ -186,9 +188,18 @@ private static bool TryReadAppOption( return true; } - private static bool TryTakeValue(IReadOnlyList args, ref int index, string option, out string value, out string error) + private static bool TryTakeValue( + IReadOnlyList args, + ref int index, + string option, + IReadOnlyList appOptions, + out string value, + out string error) { - if (index + 1 >= args.Count) + // A switch after a value option means the value was forgotten. Taking the switch as the value would + // drop it without a word, so "--reason --status" would start a run instead of reporting. Anything + // else is a value, even if it starts with '-', so "-" and "-5" still get through. + if (index + 1 >= args.Count || IsKnownSwitch(args[index + 1], appOptions)) { value = string.Empty; error = $"'{option}' needs a value."; @@ -201,6 +212,9 @@ private static bool TryTakeValue(IReadOnlyList args, ref int index, stri return true; } + private static bool IsKnownSwitch(string argument, IReadOnlyList appOptions) => + StandardAliases.Contains(argument, StringComparer.Ordinal) || appOptions.Any(option => option.Matches(argument)); + /// /// What has been read so far, so that reading one argument is a small method rather than another arm of /// a loop that owns seven locals.