feat(cli): allow known globals after command; query output + contract…#926
Conversation
… docs GlobalOptions parses the full argv: after the command token, recognized globals (--network, --output, --wallet, --grpc-endpoint, etc.) are applied and removed from command args; --help/-h after the command are left for the command parser; unknown trailing flags pass through unchanged. Malformed or invalid values for known globals still fail at the global layer (validated globally). Client.requestsJsonOutput no longer stops scanning at the first non-flag so post-command --output json is respected. OutputFormatter adds queryResult(): text mode renders small flat primitive maps as Metadata, larger/nested/array payloads as indented Result: JSON; success/formatMessage-only paths wrap plain strings for TEXT JSON payloads. QueryCommands and ContractCommands use queryResult for RPC-style successes; several protobuf paths now emit not_found when the RPC returns null. Updates standard-cli-contract-spec and qa/contracts.tsv (post-command network succeeds; unknown post-command option stays command-local usage). Adds GlobalOptions, OutputFormatter, and ClientMain tests for the above.
| private void renderQueryResult(Object jsonData) { | ||
| if (shouldRenderAsMetadata(jsonData)) { | ||
| renderMetadata(jsonData); | ||
| return; |
There was a problem hiding this comment.
queryResult() in TEXT mode drops both empty objects and message-only results.
normalizeJsonData() wraps non-JSON strings into { message: ... }, and empty protobuf/list payloads may become {}.
Then shouldRenderAsMetadata() returns true, but renderMetadata() (line 242) skips the message field, leaving nothing to print—so the output degrades to just “X successful !!”.
This affects the commands switched to queryResult() in this commit (e.g., QueryCommands.java:746).
The previous printMessage at least printed the raw payload.
Suggestion:
- Make
shouldRenderAsMetadata()require at least one non-messagefield. - Otherwise, fall back to
Result:output, or directly printmessagefor message-only cases.
Suggested tests:
| Input | After normalize | shouldRenderAsMetadata | renderMetadata output |
|---|---|---|---|
"plain text" (non-JSON) |
{"message":"plain text"} (Map) |
true (only message, skipped) |
empty → nothing printed |
"{}" (empty JSON, e.g. empty proto) |
empty JsonObject | true (size ≤ 12, loop not entered) | empty → nothing printed |
{"message":"only"} |
single-key JsonObject | true (only key skipped) | empty → nothing printed |
There was a problem hiding this comment.
Fixed. shouldRenderAsMetadata() now requires at least one printable non-message field before using the Metadata rendering path, so message-only payloads and empty objects fall back to Result: instead of being dropped.
Added regression coverage for non-JSON text (data.message) and empty JSON object query results in TEXT mode.
| case "help": | ||
| if (commandSeen) { | ||
| commandArgs.add(token); | ||
| break; | ||
| } | ||
| ensureNoInlineValue(parsed, "--help"); | ||
| opts.help = true; | ||
| break; | ||
| case "version": | ||
| ensureNoInlineValue(parsed, "--version"); | ||
| opts.version = true; | ||
| break; |
There was a problem hiding this comment.
Behavior change:
--version and --interactive are now accepted as post-command global options, and are handled early in Client.java:4721 before command execution.
This means wallet-cli get-balance --version returns the version instead of a command usage error.
The docs seem to allow “known global options can appear after the command”, but --help is explicitly reserved for command-level handling.
Suggestion:
Confirm whether --version / --interactive should also be restricted to pre-command only, for consistency with --help.
There was a problem hiding this comment.
Changed this contract: --version and --interactive are now treated as top-level mode selectors and are recognized only before the command token. When they appear after a command, they are passed through as command-local args and produce command usage errors such as Unknown option: --version.
Updated standard-cli-contract-spec.md, added parser/runner tests, and added QA contract cases for post-command --version / --interactive.
| private String indentResult(String text) { | ||
| return " " + text.replace(System.lineSeparator(), System.lineSeparator() + " "); |
There was a problem hiding this comment.
Cross-platform bug:
OutputFormatter.indentResult() uses System.lineSeparator(), but Gson always emits \n (see OutputFormatter.java:238-240):
private String indentResult(String text) {
return " " + text.replace(System.lineSeparator(), System.lineSeparator() + " ");
}gson.toJson(...) internally hardcodes \n as the line break, regardless of platform.
On Windows, System.lineSeparator() == "\r\n", so replace() never matches → multiline JSON is not indented except for the first line.
Suggestion:
return " " + text.replace("\n", "\n ");(println(...) will append the platform-specific newline anyway; using \n here for internal formatting is sufficient.)
There was a problem hiding this comment.
Fixed. indentResult() now indents Gson pretty-printed output by replacing literal \n, which matches Gson's internal line separator regardless of the platform line separator.
Added TEXT mode coverage that asserts pretty-printed JSON lines are indented after line feeds.
Prevent text query results from dropping message-only or empty payloads, and make Gson-formatted result indentation platform-independent. Treat --version and --interactive as pre-command top-level modes only, with post-command usage now handled by command parsing; update the standard CLI contract, QA cases, and regression tests accordingly.
5 participants
GlobalOptions parses the full argv: after the command token, recognized globals (--network, --output, --wallet, --grpc-endpoint, etc.) are applied and removed from command args; --help/-h after the command are left for the command parser; unknown trailing flags pass through unchanged. Malformed or invalid values for known globals still fail at the global layer (validated globally).
Client.requestsJsonOutput no longer stops scanning at the first non-flag so post-command --output json is respected.
OutputFormatter adds queryResult(): text mode renders small flat primitive maps as Metadata, larger/nested/array payloads as indented Result: JSON; success/formatMessage-only paths wrap plain strings for TEXT JSON payloads.
QueryCommands and ContractCommands use queryResult for RPC-style successes; several protobuf paths now emit not_found when the RPC returns null.
Updates standard-cli-contract-spec and qa/contracts.tsv (post-command network succeeds; unknown post-command option stays command-local usage).
Adds GlobalOptions, OutputFormatter, and ClientMain tests for the above.