From 491be41e623e6a29a94f89714ebddfaa6636428e Mon Sep 17 00:00:00 2001 From: placeholder_name Date: Tue, 11 Nov 2025 00:06:21 +0530 Subject: [PATCH 1/6] fix(cli_tools): Integration of `help -h` with `MessageOutput` - added an internal `_HelpCommandDummy` - checks all cases via `_isHelpUsageRequested` - preserves upstream Exit Code --- .../better_command_runner.dart | 57 ++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/packages/cli_tools/lib/src/better_command_runner/better_command_runner.dart b/packages/cli_tools/lib/src/better_command_runner/better_command_runner.dart index 4e31369..5394459 100644 --- a/packages/cli_tools/lib/src/better_command_runner/better_command_runner.dart +++ b/packages/cli_tools/lib/src/better_command_runner/better_command_runner.dart @@ -48,6 +48,32 @@ typedef SetLogLevel = void Function({ /// A function type for tracking events. typedef OnAnalyticsEvent = void Function(String event); +/// A dummy to replicate the usage-text of upstream private `HelpCommand` +final class _HelpCommandDummy extends Command { + _HelpCommandDummy({required this.runner}); + + static const label = 'help'; + + static const Null exitCode = null; + + @override + final name = _HelpCommandDummy.label; + + @override + final BetterCommandRunner runner; + + @override + String get description => + 'Display help information for ${runner.executableName}.'; + + @override + String get invocation => '${runner.executableName} $name [command]'; + + @override + Never run() => throw StateError( + 'This class is meant to only obtain the Usage Text for `$name` command'); +} + /// An extension of [CommandRunner] with additional features. /// /// This class extends the [CommandRunner] class from the `args` package and adds @@ -72,7 +98,7 @@ class BetterCommandRunner /// The environment variables used for configuration resolution. final Map envVariables; - /// The gloabl option definitions. + /// The global option definitions. late final List _globalOptions; Configuration? _globalConfiguration; @@ -346,6 +372,10 @@ class BetterCommandRunner await _onBeforeRunCommand?.call(this); try { + if (_isHelpUsageRequested(topLevelResults)) { + messageOutput?.logUsage(_HelpCommandDummy(runner: this).usage); + return _HelpCommandDummy.exitCode; + } return await super.runCommand(topLevelResults); } on UsageException catch (e) { messageOutput?.logUsageException(e); @@ -367,6 +397,31 @@ class BetterCommandRunner ); } + static bool _isHelpUsageRequested(final ArgResults topLevelResults) { + final topLevelCommand = topLevelResults.command; + if (topLevelCommand == null) { + return false; + } + final topLevelCommandName = topLevelCommand.name; + if (topLevelCommandName != _HelpCommandDummy.label) { + return false; + } + final helpCommand = topLevelCommand; + // check whether it's allowed to get the usage-text for `help` + if (!helpCommand.options.contains(_HelpCommandDummy.label)) { + throw StateError('Upstream `package:args` has a breaking change'); + } + // case: `mock help -h` + if (helpCommand.flag(_HelpCommandDummy.label)) { + return true; + } + // case: `mock help help` + if ((helpCommand.arguments.contains(_HelpCommandDummy.label))) { + return true; + } + return false; + } + static CommandRunnerLogLevel _determineLogLevel(final Configuration config) { if (config.findValueOf(argName: BetterCommandRunnerFlags.verbose) == true) { return CommandRunnerLogLevel.verbose; From ab842e95ff344abcec3f94ab07788dfa37ee7f6a Mon Sep 17 00:00:00 2001 From: placeholder_name Date: Tue, 11 Nov 2025 00:56:50 +0530 Subject: [PATCH 2/6] test(cli_tools): Behavior of `help -h` with `MessageOutput` - checks multiple expected behavior - checks the Exit Code - checks multiple exception cases - ensures compatibility with upstream behavior --- .../help_command_usage_text_test.dart | 168 ++++++++++++++++++ 1 file changed, 168 insertions(+) create mode 100644 packages/cli_tools/test/better_command_runner/help_command_usage_text_test.dart diff --git a/packages/cli_tools/test/better_command_runner/help_command_usage_text_test.dart b/packages/cli_tools/test/better_command_runner/help_command_usage_text_test.dart new file mode 100644 index 0000000..96b55a7 --- /dev/null +++ b/packages/cli_tools/test/better_command_runner/help_command_usage_text_test.dart @@ -0,0 +1,168 @@ +import 'dart:async' show ZoneSpecification, runZoned; + +import 'package:args/command_runner.dart' show CommandRunner, UsageException; +import 'package:cli_tools/better_command_runner.dart' + show BetterCommandRunner, MessageOutput; +import 'package:test/test.dart'; + +void main() => _runTests(); + +const _exeName = 'mock'; +const _exeDescription = 'A mock command to test `help -h` with MessageOutput.'; +const _moMockText = 'SUCCESS: `MessageOutput.usageLogger`'; +const _moExceptionText = 'SUCCESS: `MessageOutput.usageExceptionLogger`'; +const _invalidOption = '-z'; + +CommandRunner _buildUpstreamRunner() => CommandRunner( + _exeName, + _exeDescription, + ); + +BetterCommandRunner _buildBetterRunner() => BetterCommandRunner( + _exeName, + _exeDescription, + messageOutput: MessageOutput( + usageLogger: (final usage) { + print(_moMockText); + print(usage); + }, + usageExceptionLogger: (final exception) { + print(_moExceptionText); + print(exception.message); + print(exception.usage); + }, + ), + ); + +void _runTests() { + group('Given a BetterCommandRunner with custom MessageOutput', () { + for (final args in [ + ['help', '-h'], + ['help', 'help'], + ['help', '-h', 'help'], + ['help', '-h', 'help', '-h'], + ['help', '-h', 'help', '-h', 'help'], + ['help', '-h', 'help', '-h', 'help', '-h'], + ]) { + _testsForValidHelpUsageRequests(args); + } + for (final args in [ + ['help', '-h', _invalidOption], + ['help', 'help', _invalidOption], + ['help', _invalidOption, 'help'], + ['help', 'help', '-h', _invalidOption], + ['help', _invalidOption, 'help', '-h', _invalidOption], + ]) { + _testsForInvalidHelpUsageRequests(args); + } + }); +} + +void _testsForValidHelpUsageRequests(final List args) { + group('when $args is received', () { + final betterRunnerOutput = StringBuffer(); + final upstreamRunnerOutput = StringBuffer(); + late final Object? betterRunnerExitCode; + late final Object? upstreamRunnerExitCode; + setUpAll(() async { + betterRunnerExitCode = await runZoned( + () async => await _buildBetterRunner().run(args), + zoneSpecification: ZoneSpecification( + print: (final _, final __, final ___, final String line) { + betterRunnerOutput.writeln(line); + }), + ); + upstreamRunnerExitCode = await runZoned( + () async => await _buildUpstreamRunner().run(args), + zoneSpecification: ZoneSpecification( + print: (final _, final __, final ___, final String line) { + upstreamRunnerOutput.writeln(line); + }), + ); + }); + test('then MessageOutput is not bypassed', () { + expect( + betterRunnerOutput.toString(), + contains(_moMockText), + ); + }); + test('then it can subsume upstream HelpCommand output', () { + expect( + betterRunnerOutput.toString(), + stringContainsInOrder([ + _moMockText, + upstreamRunnerOutput.toString(), + ]), + ); + }); + test('then Exit Code (null) matches that of upstream HelpCommand', () { + expect(betterRunnerExitCode, equals(null)); + expect(betterRunnerExitCode, equals(upstreamRunnerExitCode)); + }); + }); +} + +void _testsForInvalidHelpUsageRequests(final List args) { + group('when $args is received', () { + final betterRunnerOutput = StringBuffer(); + final upstreamRunnerOutput = StringBuffer(); + late final UsageException? betterRunnerException; + late final UsageException? upstreamRunnerException; + setUpAll(() async { + try { + await runZoned( + () async => await _buildBetterRunner().run(args), + zoneSpecification: ZoneSpecification( + print: (final _, final __, final ___, final String line) { + betterRunnerOutput.writeln(line); + }, + ), + ); + } on UsageException catch (e) { + betterRunnerException = e; + } + try { + await runZoned( + () async => await _buildUpstreamRunner().run(args), + zoneSpecification: ZoneSpecification( + print: (final _, final __, final ___, final String line) { + upstreamRunnerOutput.writeln(line); + }, + ), + ); + } on UsageException catch (e) { + upstreamRunnerException = e; + } + }); + test('then it throws UsageException exactly like CommandRunner', () { + expect(betterRunnerException, isA()); + expect(upstreamRunnerException, isA()); + expect( + betterRunnerException!.message, + equals(upstreamRunnerException!.message), + ); + expect( + betterRunnerException!.usage, + equals(upstreamRunnerException!.usage), + ); + }); + test('then MessageOutput is not bypassed', () { + expect( + betterRunnerOutput.toString(), + stringContainsInOrder( + [ + _moExceptionText, + betterRunnerException!.message, + betterRunnerException!.usage, + ], + ), + ); + }); + test('then it can subsume upstream HelpCommand output', () { + expect( + betterRunnerOutput.toString(), + contains(upstreamRunnerOutput.toString()), + ); + }); + }); +} From d3bb0cf95b68814b9cb8e1a9b20aed8913be042f Mon Sep 17 00:00:00 2001 From: placeholder_name Date: Tue, 11 Nov 2025 01:26:03 +0530 Subject: [PATCH 3/6] refactor(#92): Better error message if a test fails Although non-essential, this covers an edge-case as highlighted by the Review Bot. --- .../better_command_runner/help_command_usage_text_test.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cli_tools/test/better_command_runner/help_command_usage_text_test.dart b/packages/cli_tools/test/better_command_runner/help_command_usage_text_test.dart index 96b55a7..259bc42 100644 --- a/packages/cli_tools/test/better_command_runner/help_command_usage_text_test.dart +++ b/packages/cli_tools/test/better_command_runner/help_command_usage_text_test.dart @@ -106,8 +106,8 @@ void _testsForInvalidHelpUsageRequests(final List args) { group('when $args is received', () { final betterRunnerOutput = StringBuffer(); final upstreamRunnerOutput = StringBuffer(); - late final UsageException? betterRunnerException; - late final UsageException? upstreamRunnerException; + UsageException? betterRunnerException; + UsageException? upstreamRunnerException; setUpAll(() async { try { await runZoned( From 60d509093338232cbd977ab286ce886d86e5054d Mon Sep 17 00:00:00 2001 From: pseudonym Date: Wed, 12 Nov 2025 02:08:57 +0530 Subject: [PATCH 4/6] refactor(#92): Better readability - improved a function name - improved its inline documentation --- .../src/better_command_runner/better_command_runner.dart | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/cli_tools/lib/src/better_command_runner/better_command_runner.dart b/packages/cli_tools/lib/src/better_command_runner/better_command_runner.dart index 5394459..5ef0ac4 100644 --- a/packages/cli_tools/lib/src/better_command_runner/better_command_runner.dart +++ b/packages/cli_tools/lib/src/better_command_runner/better_command_runner.dart @@ -372,7 +372,7 @@ class BetterCommandRunner await _onBeforeRunCommand?.call(this); try { - if (_isHelpUsageRequested(topLevelResults)) { + if (_isUsageOfHelpCommandRequested(topLevelResults)) { messageOutput?.logUsage(_HelpCommandDummy(runner: this).usage); return _HelpCommandDummy.exitCode; } @@ -397,13 +397,13 @@ class BetterCommandRunner ); } - static bool _isHelpUsageRequested(final ArgResults topLevelResults) { + static bool _isUsageOfHelpCommandRequested(final ArgResults topLevelResults) { + // check whether Help Command is chosen final topLevelCommand = topLevelResults.command; if (topLevelCommand == null) { return false; } - final topLevelCommandName = topLevelCommand.name; - if (topLevelCommandName != _HelpCommandDummy.label) { + if (topLevelCommand.name != _HelpCommandDummy.label) { return false; } final helpCommand = topLevelCommand; @@ -419,6 +419,7 @@ class BetterCommandRunner if ((helpCommand.arguments.contains(_HelpCommandDummy.label))) { return true; } + // aside: more cases may be added if necessary in future return false; } From 9b10c86e11f443f51a2673e7fc01984393d58dd8 Mon Sep 17 00:00:00 2001 From: pseudonym Date: Wed, 12 Nov 2025 03:08:02 +0530 Subject: [PATCH 5/6] refactor(#92): Better robustness handles an extremely unlikely scenario in a better manner --- .../src/better_command_runner/better_command_runner.dart | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/cli_tools/lib/src/better_command_runner/better_command_runner.dart b/packages/cli_tools/lib/src/better_command_runner/better_command_runner.dart index 5ef0ac4..43f3d92 100644 --- a/packages/cli_tools/lib/src/better_command_runner/better_command_runner.dart +++ b/packages/cli_tools/lib/src/better_command_runner/better_command_runner.dart @@ -398,7 +398,7 @@ class BetterCommandRunner } static bool _isUsageOfHelpCommandRequested(final ArgResults topLevelResults) { - // check whether Help Command is chosen + // check whether `help` command is chosen final topLevelCommand = topLevelResults.command; if (topLevelCommand == null) { return false; @@ -409,7 +409,11 @@ class BetterCommandRunner final helpCommand = topLevelCommand; // check whether it's allowed to get the usage-text for `help` if (!helpCommand.options.contains(_HelpCommandDummy.label)) { - throw StateError('Upstream `package:args` has a breaking change'); + // rare scenario (if upstream `package:args` has a breaking change) + // fortunately, corresponding test-cases shall fail as it + // - tests the current behavior (e.g. args = ['help', '-h']) + // - notifies the publisher(s) of this breaking change + return false; } // case: `mock help -h` if (helpCommand.flag(_HelpCommandDummy.label)) { From f0f371940c609f91c744d7f98bb517292713cb9b Mon Sep 17 00:00:00 2001 From: pseudonym Date: Mon, 17 Nov 2025 16:35:42 +0530 Subject: [PATCH 6/6] refactor(#92): Clean Code Resolves all the review comments by a package admin. --- .../better_command_runner.dart | 67 +++---------------- .../help_command_workaround.dart | 64 ++++++++++++++++++ 2 files changed, 72 insertions(+), 59 deletions(-) create mode 100644 packages/cli_tools/lib/src/better_command_runner/help_command_workaround.dart diff --git a/packages/cli_tools/lib/src/better_command_runner/better_command_runner.dart b/packages/cli_tools/lib/src/better_command_runner/better_command_runner.dart index 43f3d92..9ea518a 100644 --- a/packages/cli_tools/lib/src/better_command_runner/better_command_runner.dart +++ b/packages/cli_tools/lib/src/better_command_runner/better_command_runner.dart @@ -8,6 +8,8 @@ import 'package:config/config.dart'; import 'completion/completion_command.dart'; import 'completion/completion_tool.dart' show CompletionScript; +import 'help_command_workaround.dart' show HelpCommandWorkaround; + /// A function type for executing code before running a command. typedef OnBeforeRunCommand = Future Function(BetterCommandRunner runner); @@ -48,32 +50,6 @@ typedef SetLogLevel = void Function({ /// A function type for tracking events. typedef OnAnalyticsEvent = void Function(String event); -/// A dummy to replicate the usage-text of upstream private `HelpCommand` -final class _HelpCommandDummy extends Command { - _HelpCommandDummy({required this.runner}); - - static const label = 'help'; - - static const Null exitCode = null; - - @override - final name = _HelpCommandDummy.label; - - @override - final BetterCommandRunner runner; - - @override - String get description => - 'Display help information for ${runner.executableName}.'; - - @override - String get invocation => '${runner.executableName} $name [command]'; - - @override - Never run() => throw StateError( - 'This class is meant to only obtain the Usage Text for `$name` command'); -} - /// An extension of [CommandRunner] with additional features. /// /// This class extends the [CommandRunner] class from the `args` package and adds @@ -372,10 +348,13 @@ class BetterCommandRunner await _onBeforeRunCommand?.call(this); try { - if (_isUsageOfHelpCommandRequested(topLevelResults)) { - messageOutput?.logUsage(_HelpCommandDummy(runner: this).usage); - return _HelpCommandDummy.exitCode; + // an edge case regarding `help -h` + final helpProxy = HelpCommandWorkaround(runner: this); + if (helpProxy.isUsageOfHelpCommandRequested(topLevelResults)) { + messageOutput?.logUsage(helpProxy.usage); + return null; } + // normal cases return await super.runCommand(topLevelResults); } on UsageException catch (e) { messageOutput?.logUsageException(e); @@ -397,36 +376,6 @@ class BetterCommandRunner ); } - static bool _isUsageOfHelpCommandRequested(final ArgResults topLevelResults) { - // check whether `help` command is chosen - final topLevelCommand = topLevelResults.command; - if (topLevelCommand == null) { - return false; - } - if (topLevelCommand.name != _HelpCommandDummy.label) { - return false; - } - final helpCommand = topLevelCommand; - // check whether it's allowed to get the usage-text for `help` - if (!helpCommand.options.contains(_HelpCommandDummy.label)) { - // rare scenario (if upstream `package:args` has a breaking change) - // fortunately, corresponding test-cases shall fail as it - // - tests the current behavior (e.g. args = ['help', '-h']) - // - notifies the publisher(s) of this breaking change - return false; - } - // case: `mock help -h` - if (helpCommand.flag(_HelpCommandDummy.label)) { - return true; - } - // case: `mock help help` - if ((helpCommand.arguments.contains(_HelpCommandDummy.label))) { - return true; - } - // aside: more cases may be added if necessary in future - return false; - } - static CommandRunnerLogLevel _determineLogLevel(final Configuration config) { if (config.findValueOf(argName: BetterCommandRunnerFlags.verbose) == true) { return CommandRunnerLogLevel.verbose; diff --git a/packages/cli_tools/lib/src/better_command_runner/help_command_workaround.dart b/packages/cli_tools/lib/src/better_command_runner/help_command_workaround.dart new file mode 100644 index 0000000..68bb3a3 --- /dev/null +++ b/packages/cli_tools/lib/src/better_command_runner/help_command_workaround.dart @@ -0,0 +1,64 @@ +import 'package:args/args.dart' show ArgResults; +import 'package:args/command_runner.dart' show Command, CommandRunner; + +/// A dummy to replicate the usage-text of upstream private `HelpCommand`. +/// +/// It is intended to be used for an internal patch only and is +/// intentionally not part of the public API of this package. +final class HelpCommandWorkaround extends Command { + HelpCommandWorkaround({required this.runner}); + + /// Checks whether the main command seeks the + /// usage-text for `help` command. + /// + /// Specifically, for a program `mock`, it checks + /// whether [topLevelResults] is of the form: + /// * `mock help -h` + /// * `mock help help` + bool isUsageOfHelpCommandRequested(final ArgResults topLevelResults) { + // check whether `help` command is chosen + final topLevelCommand = topLevelResults.command; + if (topLevelCommand == null) { + return false; + } + if (topLevelCommand.name != name) { + return false; + } + final helpCommand = topLevelCommand; + // check whether it's allowed to get the usage-text for `help` + if (!helpCommand.options.contains(name)) { + // extremely rare scenario (e.g. if `package:args` has a breaking change) + // fortunately, corresponding test-cases shall fail as it + // - tests the current behavior (e.g. args = ['help', '-h']) + // - notifies the publisher(s) of this breaking change + return false; + } + // case: `mock help -h` + if (helpCommand.flag(name)) { + return true; + } + // case: `mock help help` + if ((helpCommand.arguments.contains(name))) { + return true; + } + // aside: more cases may be added if necessary in future + return false; + } + + @override + final CommandRunner runner; + + @override + final name = 'help'; + + @override + String get description => + 'Display help information for ${runner.executableName}.'; + + @override + String get invocation => '${runner.executableName} $name [command]'; + + @override + Never run() => throw UnimplementedError( + 'This class is meant to only obtain the Usage Text for `$name` command'); +}