Fail a value option followed by a switch instead of taking the switch as its value [patch]

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()
 	{

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()
 	{

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<string> args,
 		ref int index,
 		string argument,
+		IReadOnlyList<AppOption> 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<string> args,
 		ref int index,
 		string argument,
+		IReadOnlyList<AppOption> 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<string> args, ref int index, string option, out string value, out string error)
+	private static bool TryTakeValue(
+		IReadOnlyList<string> args,
+		ref int index,
+		string option,
+		IReadOnlyList<AppOption> 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<string> args, ref int index, stri
 		return true;
 	}
 
+	private static bool IsKnownSwitch(string argument, IReadOnlyList<AppOption> appOptions) =>
+		StandardAliases.Contains(argument, StringComparer.Ordinal) || appOptions.Any(option => option.Matches(argument));
+
 	/// <summary>
 	/// 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.