From ed638b657cc220c45eead65644c5eccdec84b5d6 Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Fri, 13 Feb 2026 23:39:16 +1000 Subject: [PATCH 1/3] Add a couple unit tests for `ArgParser` --- src/BizHawk.Client.Common/ArgParser.cs | 10 +++---- .../ArgParserTests.cs | 27 +++++++++++++++++++ 2 files changed, 32 insertions(+), 5 deletions(-) create mode 100644 src/BizHawk.Tests.Client.Common/ArgParserTests.cs diff --git a/src/BizHawk.Client.Common/ArgParser.cs b/src/BizHawk.Client.Common/ArgParser.cs index c7e8393ac56..102638f8d1a 100644 --- a/src/BizHawk.Client.Common/ArgParser.cs +++ b/src/BizHawk.Client.Common/ArgParser.cs @@ -177,17 +177,17 @@ private static void EnsureConsole() /// exit code, or if should not exit /// parsing failure, or invariant broken - public static int? ParseArguments(out ParsedCLIFlags parsed, string[] args) + public static int? ParseArguments(out ParsedCLIFlags parsed, string[] args, bool fromUnitTest = false) { parsed = default; - if (args.Length is not 0) Console.Error.WriteLine($"parsing command-line flags: {string.Join(" ", args)}"); + if (!fromUnitTest && args.Length is not 0) Console.Error.WriteLine($"parsing command-line flags: {string.Join(" ", args)}"); var rootCommand = GetRootCommand(); var result = CommandLineParser.Parse(rootCommand, args); if (result.Errors.Count is not 0) { // generate useful commandline error output EnsureConsole(); - result.Invoke(); + if (!fromUnitTest) result.Invoke(); // show first error in modal dialog (done in `catch` block in `Program`) throw new ArgParserException($"failed to parse command-line arguments: {result.Errors[0].Message}"); } @@ -195,13 +195,13 @@ private static void EnsureConsole() { // means e.g. `./EmuHawkMono.sh --help` was passed, run whatever behaviour it normally has EnsureConsole(); - return result.Invoke(); + return fromUnitTest ? 0 : result.Invoke(); } if (result.GetValue(OptionQueryAppVersion)) { // means e.g. `./EmuHawkMono.sh --version` was passed, so print that and exit immediately EnsureConsole(); - Console.WriteLine(VersionInfo.GetEmuVersion()); + if (!fromUnitTest) Console.WriteLine(VersionInfo.GetEmuVersion()); return 0; } diff --git a/src/BizHawk.Tests.Client.Common/ArgParserTests.cs b/src/BizHawk.Tests.Client.Common/ArgParserTests.cs new file mode 100644 index 00000000000..2dc16e8965f --- /dev/null +++ b/src/BizHawk.Tests.Client.Common/ArgParserTests.cs @@ -0,0 +1,27 @@ +using BizHawk.Client.Common; + +namespace BizHawk.Tests.Client.Common +{ + [TestClass] + public sealed class ArgParserTests + { + [DataRow("--config=config.json", "--help")] + [DataRow("--config=config.json", "--version")] + [TestMethod] + public void TestConfigWithHelpOrVersion(params string[] args) + => Assert.AreEqual(0, ArgParser.ParseArguments(out _, args, fromUnitTest: true)); + + [DataRow("rom.nes", "--nonexistent")] + [DataRow("--nonexistent", "rom.nes")] + [DataRow("--nonexistent", "--", "rom.nes")] + [TestMethod] + public void TestWithNonexistent(params string[] args) + { + int? exitCode = null; + var e = Assert.ThrowsExactly(() => exitCode = ArgParser.ParseArguments(out _, args, fromUnitTest: true)); + Assert.AreNotEqual(0, exitCode ?? 1); + Assert.Contains(substring: "Unrecog", e.Message); +// Assert.Contains(substring: "--nonexistent", e.Message); + } + } +} From d0e78b1d0cc2bb3c67c57badeb9a7ed569a3fc2f Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Sat, 14 Feb 2026 13:43:12 +1000 Subject: [PATCH 2/3] Improve `ArgParser`'s handling of unrecognised flags unfortunately this makes the usage line in `--help` even worse, will fix in later commit --- src/BizHawk.Client.Common/ArgParser.cs | 17 ++++++++++++----- .../ArgParserTests.cs | 2 +- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/BizHawk.Client.Common/ArgParser.cs b/src/BizHawk.Client.Common/ArgParser.cs index 102638f8d1a..baa6a53bc9e 100644 --- a/src/BizHawk.Client.Common/ArgParser.cs +++ b/src/BizHawk.Client.Common/ArgParser.cs @@ -15,9 +15,8 @@ namespace BizHawk.Client.Common /// Parses command-line flags into a struct. public static class ArgParser { - private static readonly Argument ArgumentRomFilePath = new("rom") + private static readonly Argument ArgumentRomFilePath = new("rom") { - DefaultValueFactory = _ => null, Description = "path; if specified, the file will be loaded the same way as it would be from `File` > `Open...`; this argument can and should be given LAST despite what it says at the top of --help", }; @@ -134,7 +133,6 @@ private static RootCommand GetRootCommand() RootCommand root = new($"{ (string.IsNullOrEmpty(VersionInfo.CustomBuildString) ? "EmuHawk" : VersionInfo.CustomBuildString) }, a multi-system emulator frontend\n{VersionInfo.GetEmuVersion()}"); - root.Add(ArgumentRomFilePath); root.Options.RemoveAll(option => option is VersionOption); // we have our own version command // `--help` uses this order, so keep alphabetised by flag @@ -163,6 +161,7 @@ private static RootCommand GetRootCommand() root.Add(/* --userdata */ OptionUserdataUnparsedPairs); root.Add(/* --version */ OptionQueryAppVersion); + root.Add(ArgumentRomFilePath); return root; } @@ -205,6 +204,15 @@ private static void EnsureConsole() return 0; } + var unmatchedArguments = result.GetValue(ArgumentRomFilePath) ?? [ ]; + if (unmatchedArguments.Length >= 2) + { + var foundFlagLike = unmatchedArguments.FirstOrDefault(static s => s.StartsWith("--")); + throw new ArgParserException(foundFlagLike is null + ? "Multiple rom paths provided. (Did you delete half of a flag? Or forget the \"--\" of one?)" + : $"Unrecognised flag(s): \"{foundFlagLike}\""); + } + var autoDumpLength = result.GetValue(OptionAVDumpEndAtFrame); HashSet? currAviWriterFrameList = null; if (result.GetValue(OptionAVDumpFrameList) is string list) @@ -268,8 +276,7 @@ private static void EnsureConsole() openExtToolDll: result.GetValue(OptionOpenExternalTool), socketProtocol: result.GetValue(OptionSocketServerUseUDP) ? ProtocolType.Udp : ProtocolType.Tcp, userdataUnparsedPairs: userdataUnparsedPairs, - cmdRom: result.GetValue(ArgumentRomFilePath) - ); + cmdRom: unmatchedArguments.LastOrDefault()); // `unmatchedArguments.Length` must be 0 or 1 at this point, but in case we change how that's handled, 'last' here preserves the behaviour from the old hand-rolled parser return null; } diff --git a/src/BizHawk.Tests.Client.Common/ArgParserTests.cs b/src/BizHawk.Tests.Client.Common/ArgParserTests.cs index 2dc16e8965f..b32482665f3 100644 --- a/src/BizHawk.Tests.Client.Common/ArgParserTests.cs +++ b/src/BizHawk.Tests.Client.Common/ArgParserTests.cs @@ -21,7 +21,7 @@ public void TestWithNonexistent(params string[] args) var e = Assert.ThrowsExactly(() => exitCode = ArgParser.ParseArguments(out _, args, fromUnitTest: true)); Assert.AreNotEqual(0, exitCode ?? 1); Assert.Contains(substring: "Unrecog", e.Message); -// Assert.Contains(substring: "--nonexistent", e.Message); + Assert.Contains(substring: "--nonexistent", e.Message); } } } From 2a9a11180a3bf2c1552a816c2a14c42a3e61da7f Mon Sep 17 00:00:00 2001 From: YoshiRulz Date: Sat, 14 Feb 2026 14:47:05 +1000 Subject: [PATCH 3/3] Intercept output of `--help` to correct the usage line --- src/BizHawk.Client.Common/ArgParser.cs | 35 ++++++++++++++++++- .../ArgParserTests.cs | 13 +++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/src/BizHawk.Client.Common/ArgParser.cs b/src/BizHawk.Client.Common/ArgParser.cs index baa6a53bc9e..b2910494d17 100644 --- a/src/BizHawk.Client.Common/ArgParser.cs +++ b/src/BizHawk.Client.Common/ArgParser.cs @@ -2,7 +2,10 @@ using System.Collections.Generic; using System.CommandLine; +using System.CommandLine.Help; +using System.CommandLine.Invocation; using System.CommandLine.Parsing; +using System.IO; using System.Linq; using System.Net.Sockets; @@ -15,9 +18,30 @@ namespace BizHawk.Client.Common /// Parses command-line flags into a struct. public static class ArgParser { + private sealed class HelpSedAction(HelpAction stockAction) : SynchronousCommandLineAction + { + public override int Invoke(ParseResult parseResult) + { + var outerOutput = parseResult.InvocationConfiguration.Output; + using StringWriter innerOutput = new(); + parseResult.InvocationConfiguration.Output = innerOutput; + var result = stockAction.Invoke(parseResult); + var dollarZero = OSTailoredCode.IsUnixHost +#if true + ? "./EmuHawkMono.sh" +#else //TODO for .NET Core: see https://github.com/dotnet/runtime/issues/101837 + ? Environment.GetCommandLineArgs()[0].SubstringAfterLast('/') +#endif + : Environment.GetCommandLineArgs()[0].SubstringAfterLast('\\'); + outerOutput.Write(innerOutput.ToString().Replace("EmuHawk", dollarZero) + .Replace("[...] [options]", "[option...] [rom]")); + return result; + } + } + private static readonly Argument ArgumentRomFilePath = new("rom") { - Description = "path; if specified, the file will be loaded the same way as it would be from `File` > `Open...`; this argument can and should be given LAST despite what it says at the top of --help", + Description = "path; if specified, the file will be loaded the same way as it would be from `File` > `Open...`", }; private static readonly Option OptionAVDumpAudioSync = new("--audiosync") @@ -134,6 +158,8 @@ private static RootCommand GetRootCommand() (string.IsNullOrEmpty(VersionInfo.CustomBuildString) ? "EmuHawk" : VersionInfo.CustomBuildString) }, a multi-system emulator frontend\n{VersionInfo.GetEmuVersion()}"); root.Options.RemoveAll(option => option is VersionOption); // we have our own version command + var helpOption = root.Options.OfType().First(); + helpOption.Action = new HelpSedAction((HelpAction) helpOption.Action!); // `--help` uses this order, so keep alphabetised by flag root.Add(/* --audiosync */ OptionAVDumpAudioSync); @@ -280,6 +306,13 @@ private static void EnsureConsole() return null; } + internal static void RunHelpActionForUnitTest(TextWriter output) + { + var result = CommandLineParser.Parse(GetRootCommand(), [ "--help" ]); + result.InvocationConfiguration.Output = output; + result.Invoke(); + } + public sealed class ArgParserException : Exception { public ArgParserException(string message) : base(message) {} diff --git a/src/BizHawk.Tests.Client.Common/ArgParserTests.cs b/src/BizHawk.Tests.Client.Common/ArgParserTests.cs index b32482665f3..fcd20737cea 100644 --- a/src/BizHawk.Tests.Client.Common/ArgParserTests.cs +++ b/src/BizHawk.Tests.Client.Common/ArgParserTests.cs @@ -1,3 +1,6 @@ +using System.IO; +using System.Linq; + using BizHawk.Client.Common; namespace BizHawk.Tests.Client.Common @@ -11,6 +14,16 @@ public sealed class ArgParserTests public void TestConfigWithHelpOrVersion(params string[] args) => Assert.AreEqual(0, ArgParser.ParseArguments(out _, args, fromUnitTest: true)); + [TestMethod] + public void TestHelpSaysPassFlagsFirst() + { + using StringWriter output = new(); + ArgParser.RunHelpActionForUnitTest(output); + var outputLines = output.ToString().Split('\n'); + var usageLine = outputLines[outputLines.Index().First(tuple => tuple.Item.Contains("Usage:")).Index + 1].ToUpperInvariant(); + Assert.IsTrue(usageLine.IndexOf("OPTION") < usageLine.IndexOf("ROM")); + } + [DataRow("rom.nes", "--nonexistent")] [DataRow("--nonexistent", "rom.nes")] [DataRow("--nonexistent", "--", "rom.nes")]