fix(helpers): respect --format flag in sheets +append, +read, and docs +write#703
fix(helpers): respect --format flag in sheets +append, +read, and docs +write#703nuthalapativarun wants to merge 7 commits intogoogleworkspace:mainfrom
Conversation
…s +write These helpers hard-coded OutputFormat::default() (JSON) when invoking executor::execute_method, so --format table/yaml/csv had no effect. Each handler now reads the global --format flag from ArgMatches and passes it through, matching the behaviour of calendar +agenda and gmail +triage.
Rename the subcommand ArgMatches variable from `matches` to `sub_matches` so the outer `matches` is still in scope. Read the global --format flag from the parent matches rather than the subcommand matches, ensuring clap v4 correctly resolves the globally-defined argument.
…mand ArgMatches In clap v4, global args propagate to subcommand ArgMatches, so shadowing the outer `matches` with the subcommand result is correct. Reading from the parent matches ignored flags placed after the subcommand (e.g. `gws sheets +append --format table`) and introduced a regression for dry-run lookups.
…rent matches Avoid shadowing the parent `matches` with the subcommand result. Subcommand- specific args (passed to build_write_request, parse_append_args, parse_read_args) use `sub_matches`; global flags `--format` and `--dry-run` are read from the parent `matches` which is where they are defined and reliably present.
🦋 Changeset detectedLatest commit: 1e10e10 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a bug where specific Google Workspace CLI commands were ignoring the user-provided output format flag. By updating the command handlers to properly parse and propagate the format configuration, the CLI now provides consistent output formatting across all supported operations. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request fixes a bug where the --format flag was ignored in the sheets +append, sheets +read, and docs +write helper commands by extracting the format from the command-line arguments. The review feedback recommends using the subcommand-specific sub_matches instead of the top-level matches to retrieve the --format flag. This change ensures more robust argument resolution and maintains consistency with how other arguments are handled within those subcommand blocks.
Using top-level matches to read --format worked only when the flag was passed before the subcommand name. Use sub_matches in all three sites so the flag is resolved correctly when passed as a subcommand argument: gws sheets spreadsheets +append --format table ... gws sheets spreadsheets +read --format csv ... gws docs documents +write --format yaml ...
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request fixes an issue where the --format flag was ignored in the sheets and docs helpers by passing the output format to the executor. However, the current implementation incorrectly attempts to retrieve the global --format flag from the subcommand-specific sub_matches instead of the top-level matches object. This would cause the flag to continue being ignored because it is defined at the root command level. Feedback has been provided to correctly query the parent matches for the format configuration.
--format is defined on the root command, not the +write/+append/+read subcommands, so sub_matches always returns None causing output to fall back to JSON regardless of the flag. Reverts to reading from the parent matches object, consistent with how --dry-run is handled.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request fixes a bug where the --format flag was ignored in the sheets and docs helpers by passing the selected format to the executor. Feedback indicates that the flag should be retrieved from the subcommand's matches (sub_matches) rather than the parent matches to ensure it is correctly captured regardless of its position in the command line.
--format is defined with .global(true) on the root command (commands.rs:48),
so clap propagates it to all subcommands. sub_matches.get_one("format")
correctly resolves the flag regardless of whether it appears before or after
the subcommand on the command line. matches.get_one("format") would miss it
when placed after the subcommand (e.g. gws sheets +append --format table).
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request fixes an issue where the --format flag was being ignored in the sheets +append, sheets +read, and docs +write commands. The changes ensure the output format is correctly passed to the executor. Review feedback indicates that the --format flag should be retrieved from the parent matches object rather than the subcommand-specific sub_matches to correctly capture it as a global argument.
Description
sheets +append,sheets +read, anddocs +writeall hard-codedOutputFormat::default()(JSON) when callingexecutor::execute_method, meaning--format table,--format yaml, and--format csvhad no effect on their output.Each handler now reads the global
--formatflag fromArgMatchesand passes it through to the executor, matching the existing behaviour ofcalendar +agendaandgmail +triage.Example before fix:
# --format table was silently ignored; output was always JSON gws sheets +read --spreadsheet ID --range Sheet1 --format tableAfter fix: output respects the
--formatflag as with all other commands.Dry Run Output:
Checklist:
AGENTS.mdguidelines (no generatedgoogle-*crates).cargo fmt --allto format the code perfectly.cargo clippy -- -D warningsand resolved all warnings.pnpx changeset) to document my changes.