Skip to content

Commit 6a6e104

Browse files
indraneel12placeholder_namepseudonym
authored
fix(cli_tools): Integration of help -h with MessageOutput (#92)
# Resolves #90 Consumers of this package can now truly expect [`MessageOutput`][mo] to work in the case of usage-text query for [`HelpCommand`][hc] (which does not inherit from `BetterCommand`) even while being tightly-coupled and strictly internal to the upstream dependency `package:args`. --- ## Old Behavior As shown in the referenced issue, `MessageOutput` fails to work when the usage-text for `help` is solicited by users. ## New Behavior The desired functionality is achieved, and all tests pass successfully. ### Custom Tests <img width="600" height="600" alt="Screenshot_20251111_010644" src="https://github.com/user-attachments/assets/eabc0459-72c4-40eb-a4f3-9d2a47150e06" /> ### All Tests <img width="600" height="600" alt="Screenshot_20251111_010710" src="https://github.com/user-attachments/assets/64de1297-8692-4859-be35-e160a56c7502" /> --- #### Features - Fixes for both `mock help -h` and `mock help help` - Preserves upstream `HelpCommand` behavior as well - Non-breaking change [mo]: https://pub.dev/documentation/cli_tools/latest/cli_tools/MessageOutput-class.html [hc]: https://github.com/dart-lang/core/blob/20ed966843c5930e00c480e2697b4afb7b3fd8e7/pkgs/args/lib/src/help_command.dart#L10 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Corrects an edge case so help/usage output for the help command is captured and reported consistently, preserving expected exit behavior. * **Tests** * Adds tests validating help-usage output ordering, logging of usage/exception cases, and parity with upstream behavior for valid and invalid inputs. * **Documentation** * Fixed a typo in internal comments. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: placeholder_name <[email protected]> Co-authored-by: pseudonym <[email protected]>
1 parent 902a5f6 commit 6a6e104

File tree

3 files changed

+242
-1
lines changed

3 files changed

+242
-1
lines changed

‎packages/cli_tools/lib/src/better_command_runner/better_command_runner.dart‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import 'package:config/config.dart';
88
import 'completion/completion_command.dart';
99
import 'completion/completion_tool.dart' show CompletionScript;
1010

11+
import 'help_command_workaround.dart' show HelpCommandWorkaround;
12+
1113
/// A function type for executing code before running a command.
1214
typedef OnBeforeRunCommand = Future<void> Function(BetterCommandRunner runner);
1315

@@ -72,7 +74,7 @@ class BetterCommandRunner<O extends OptionDefinition, T>
7274
/// The environment variables used for configuration resolution.
7375
final Map<String, String> envVariables;
7476

75-
/// The gloabl option definitions.
77+
/// The global option definitions.
7678
late final List<O> _globalOptions;
7779

7880
Configuration<O>? _globalConfiguration;
@@ -346,6 +348,13 @@ class BetterCommandRunner<O extends OptionDefinition, T>
346348
await _onBeforeRunCommand?.call(this);
347349

348350
try {
351+
// an edge case regarding `help -h`
352+
final helpProxy = HelpCommandWorkaround(runner: this);
353+
if (helpProxy.isUsageOfHelpCommandRequested(topLevelResults)) {
354+
messageOutput?.logUsage(helpProxy.usage);
355+
return null;
356+
}
357+
// normal cases
349358
return await super.runCommand(topLevelResults);
350359
} on UsageException catch (e) {
351360
messageOutput?.logUsageException(e);

‎packages/cli_tools/lib/src/better_command_runner/help_command_workaround.dart‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
import 'package:args/args.dart' show ArgResults;
2+
import 'package:args/command_runner.dart' show Command, CommandRunner;
3+
4+
/// A dummy to replicate the usage-text of upstream private `HelpCommand`.
5+
///
6+
/// It is intended to be used for an internal patch only and is
7+
/// intentionally not part of the public API of this package.
8+
final class HelpCommandWorkaround extends Command {
9+
HelpCommandWorkaround({required this.runner});
10+
11+
/// Checks whether the main command seeks the
12+
/// usage-text for `help` command.
13+
///
14+
/// Specifically, for a program `mock`, it checks
15+
/// whether [topLevelResults] is of the form:
16+
/// * `mock help -h`
17+
/// * `mock help help`
18+
bool isUsageOfHelpCommandRequested(final ArgResults topLevelResults) {
19+
// check whether `help` command is chosen
20+
final topLevelCommand = topLevelResults.command;
21+
if (topLevelCommand == null) {
22+
return false;
23+
}
24+
if (topLevelCommand.name != name) {
25+
return false;
26+
}
27+
final helpCommand = topLevelCommand;
28+
// check whether it's allowed to get the usage-text for `help`
29+
if (!helpCommand.options.contains(name)) {
30+
// extremely rare scenario (e.g. if `package:args` has a breaking change)
31+
// fortunately, corresponding test-cases shall fail as it
32+
// - tests the current behavior (e.g. args = ['help', '-h'])
33+
// - notifies the publisher(s) of this breaking change
34+
return false;
35+
}
36+
// case: `mock help -h`
37+
if (helpCommand.flag(name)) {
38+
return true;
39+
}
40+
// case: `mock help help`
41+
if ((helpCommand.arguments.contains(name))) {
42+
return true;
43+
}
44+
// aside: more cases may be added if necessary in future
45+
return false;
46+
}
47+
48+
@override
49+
final CommandRunner runner;
50+
51+
@override
52+
final name = 'help';
53+
54+
@override
55+
String get description =>
56+
'Display help information for ${runner.executableName}.';
57+
58+
@override
59+
String get invocation => '${runner.executableName} $name [command]';
60+
61+
@override
62+
Never run() => throw UnimplementedError(
63+
'This class is meant to only obtain the Usage Text for `$name` command');
64+
}

‎packages/cli_tools/test/better_command_runner/help_command_usage_text_test.dart‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
import 'dart:async' show ZoneSpecification, runZoned;
2+
3+
import 'package:args/command_runner.dart' show CommandRunner, UsageException;
4+
import 'package:cli_tools/better_command_runner.dart'
5+
show BetterCommandRunner, MessageOutput;
6+
import 'package:test/test.dart';
7+
8+
void main() => _runTests();
9+
10+
const _exeName = 'mock';
11+
const _exeDescription = 'A mock command to test `help -h` with MessageOutput.';
12+
const _moMockText = 'SUCCESS: `MessageOutput.usageLogger`';
13+
const _moExceptionText = 'SUCCESS: `MessageOutput.usageExceptionLogger`';
14+
const _invalidOption = '-z';
15+
16+
CommandRunner _buildUpstreamRunner() => CommandRunner(
17+
_exeName,
18+
_exeDescription,
19+
);
20+
21+
BetterCommandRunner _buildBetterRunner() => BetterCommandRunner(
22+
_exeName,
23+
_exeDescription,
24+
messageOutput: MessageOutput(
25+
usageLogger: (final usage) {
26+
print(_moMockText);
27+
print(usage);
28+
},
29+
usageExceptionLogger: (final exception) {
30+
print(_moExceptionText);
31+
print(exception.message);
32+
print(exception.usage);
33+
},
34+
),
35+
);
36+
37+
void _runTests() {
38+
group('Given a BetterCommandRunner with custom MessageOutput', () {
39+
for (final args in [
40+
['help', '-h'],
41+
['help', 'help'],
42+
['help', '-h', 'help'],
43+
['help', '-h', 'help', '-h'],
44+
['help', '-h', 'help', '-h', 'help'],
45+
['help', '-h', 'help', '-h', 'help', '-h'],
46+
]) {
47+
_testsForValidHelpUsageRequests(args);
48+
}
49+
for (final args in [
50+
['help', '-h', _invalidOption],
51+
['help', 'help', _invalidOption],
52+
['help', _invalidOption, 'help'],
53+
['help', 'help', '-h', _invalidOption],
54+
['help', _invalidOption, 'help', '-h', _invalidOption],
55+
]) {
56+
_testsForInvalidHelpUsageRequests(args);
57+
}
58+
});
59+
}
60+
61+
void _testsForValidHelpUsageRequests(final List<String> args) {
62+
group('when $args is received', () {
63+
final betterRunnerOutput = StringBuffer();
64+
final upstreamRunnerOutput = StringBuffer();
65+
late final Object? betterRunnerExitCode;
66+
late final Object? upstreamRunnerExitCode;
67+
setUpAll(() async {
68+
betterRunnerExitCode = await runZoned(
69+
() async => await _buildBetterRunner().run(args),
70+
zoneSpecification: ZoneSpecification(
71+
print: (final _, final __, final ___, final String line) {
72+
betterRunnerOutput.writeln(line);
73+
}),
74+
);
75+
upstreamRunnerExitCode = await runZoned(
76+
() async => await _buildUpstreamRunner().run(args),
77+
zoneSpecification: ZoneSpecification(
78+
print: (final _, final __, final ___, final String line) {
79+
upstreamRunnerOutput.writeln(line);
80+
}),
81+
);
82+
});
83+
test('then MessageOutput is not bypassed', () {
84+
expect(
85+
betterRunnerOutput.toString(),
86+
contains(_moMockText),
87+
);
88+
});
89+
test('then it can subsume upstream HelpCommand output', () {
90+
expect(
91+
betterRunnerOutput.toString(),
92+
stringContainsInOrder([
93+
_moMockText,
94+
upstreamRunnerOutput.toString(),
95+
]),
96+
);
97+
});
98+
test('then Exit Code (null) matches that of upstream HelpCommand', () {
99+
expect(betterRunnerExitCode, equals(null));
100+
expect(betterRunnerExitCode, equals(upstreamRunnerExitCode));
101+
});
102+
});
103+
}
104+
105+
void _testsForInvalidHelpUsageRequests(final List<String> args) {
106+
group('when $args is received', () {
107+
final betterRunnerOutput = StringBuffer();
108+
final upstreamRunnerOutput = StringBuffer();
109+
UsageException? betterRunnerException;
110+
UsageException? upstreamRunnerException;
111+
setUpAll(() async {
112+
try {
113+
await runZoned(
114+
() async => await _buildBetterRunner().run(args),
115+
zoneSpecification: ZoneSpecification(
116+
print: (final _, final __, final ___, final String line) {
117+
betterRunnerOutput.writeln(line);
118+
},
119+
),
120+
);
121+
} on UsageException catch (e) {
122+
betterRunnerException = e;
123+
}
124+
try {
125+
await runZoned(
126+
() async => await _buildUpstreamRunner().run(args),
127+
zoneSpecification: ZoneSpecification(
128+
print: (final _, final __, final ___, final String line) {
129+
upstreamRunnerOutput.writeln(line);
130+
},
131+
),
132+
);
133+
} on UsageException catch (e) {
134+
upstreamRunnerException = e;
135+
}
136+
});
137+
test('then it throws UsageException exactly like CommandRunner', () {
138+
expect(betterRunnerException, isA<UsageException>());
139+
expect(upstreamRunnerException, isA<UsageException>());
140+
expect(
141+
betterRunnerException!.message,
142+
equals(upstreamRunnerException!.message),
143+
);
144+
expect(
145+
betterRunnerException!.usage,
146+
equals(upstreamRunnerException!.usage),
147+
);
148+
});
149+
test('then MessageOutput is not bypassed', () {
150+
expect(
151+
betterRunnerOutput.toString(),
152+
stringContainsInOrder(
153+
[
154+
_moExceptionText,
155+
betterRunnerException!.message,
156+
betterRunnerException!.usage,
157+
],
158+
),
159+
);
160+
});
161+
test('then it can subsume upstream HelpCommand output', () {
162+
expect(
163+
betterRunnerOutput.toString(),
164+
contains(upstreamRunnerOutput.toString()),
165+
);
166+
});
167+
});
168+
}

0 commit comments

Comments
 (0)