fix: YAML block scalar for single-line strings, and repeated CSV/table headers with --page-all#37
Conversation
🦋 Changeset detectedLatest commit: ddaaa29 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 two critical bugs related to output formatting and introduces a significant usability enhancement for command-line arguments. It corrects YAML serialization to prevent misinterpretation of single-line strings with special characters and refines paginated CSV/table output to ensure headers appear only once. Additionally, it adds convenient kebab-case aliases for existing camelCase API resource and method names, making the CLI more user-friendly. Highlights
Changelog
Activity
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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request delivers two important bug fixes and a significant usability improvement. The fix for YAML block scalars ensures that single-line strings with special characters are correctly quoted, improving output correctness. The change to paginated CSV and table formats, which now only print headers on the first page, is a great fix for --page-all usability. Additionally, the introduction of kebab-case aliases for camelCase commands is a thoughtful feature that enhances the CLI's ergonomics. The implementation is solid, with comprehensive tests for all new logic. I have one minor suggestion to improve code conciseness.
|
/gemini review |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
Gemini review feedback (PR googleworkspace#37): since resolve_name returns a key guaranteed to exist in the methods map, the inner if let Some() guard was redundant. Replace with direct indexing for clarity. Co-authored-by: gemini-code-assist[bot] <gemini-code-assist[bot]@users.noreply.github.com>
Review feedback addressedAddressed Gemini Code Assist inline feedback (commit f01857e):
|
jpoehnelt
left a comment
There was a problem hiding this comment.
Good fixes for both bugs. The YAML block-scalar restriction to newline-only strings is the correct approach, and the paginated-header suppression is clean.
A few notes:
-
YAML quoting completeness — double-quoting handles
#and:well; worth verifying that strings containing"are correctly escaped (via\"substitution) in the new path before this lands. -
format_value_paginatedpublic API — since this is now the public entry point for pagination, confirm that the now-removedformat_value_compacthad no callers outside the crate (integration tests etc.) before merging. -
Changeset overlap with PR #34 — this PR includes changesets for both the YAML/paging fix and the kebab-case feature. The kebab-case changeset (
feat-kebab-case-aliases.md) duplicates PR #34's changeset. Confirm these won't double-apply on merge — whichever lands second should drop the duplicate changeset file. -
Test coverage — 8 new unit tests are excellent. The roundtrip test in
test_camel_kebab_roundtripis particularly valuable.
Overall solid and well-tested.
- YAML: only emit block scalar (|) for strings with genuine newlines; single-line strings containing '#' or ':' are now double-quoted instead, e.g. 'drive#file' renders as '"drive#file"' not a block scalar. - --page-all: CSV/table formats no longer re-emit column headers on every page; headers appear only on the first page. A new public format_value_paginated() helper replaces the now-removed format_value_compact(). - Add unit tests for both fixes (8 new test cases in formatter.rs).
f01857e to
ddaaa29
Compare
jpoehnelt
left a comment
There was a problem hiding this comment.
Thanks for the updates and resolving the changeset! LGTM.
…oogleworkspace#37) - YAML: only emit block scalar (|) for strings with genuine newlines; single-line strings containing '#' or ':' are now double-quoted instead, e.g. 'drive#file' renders as '"drive#file"' not a block scalar. - --page-all: CSV/table formats no longer re-emit column headers on every page; headers appear only on the first page. A new public format_value_paginated() helper replaces the now-removed format_value_compact(). - Add unit tests for both fixes (8 new test cases in formatter.rs).
Fixes #35 and #36.
Changes
Bug 1 — YAML block scalar for strings with
#or:(fixes #35)Strings like
drive#fileandhttps://...were being emitted as YAML block scalars:Root cause:
json_to_yaml()informatter.rsused block scalar for any string containing#or:, even single-line values. Block scalars are only appropriate for strings with genuine newline characters.Fix: Only use block scalar (
|) when the string contains\n. All other strings are double-quoted, with\\and\"escaped.Bug 2 —
--page-allrepeats CSV/table headers per page (fixes #36)Root cause: In pagination mode,
format_value_compact()delegated toformat_value()for non-JSON formats, which always includes headers.Fix: New public
format_value_paginated(value, format, is_first_page)function. CSV/table formats suppress column headers on continuation pages (is_first_page = false). The now-unusedformat_value_compact()has been removed.Tests
8 new unit tests added in
formatter.rs:test_format_yaml_hash_in_string_is_quoted_not_blocktest_format_yaml_colon_in_string_is_quotedtest_format_yaml_multiline_still_uses_blocktest_format_value_paginated_csv_first_page_has_headertest_format_value_paginated_csv_continuation_no_headertest_format_value_paginated_table_first_page_has_headertest_format_value_paginated_table_continuation_no_headertest_format_value_paginated_json_is_compactAll 321 existing tests continue to pass. Clippy clean. Formatted with
cargo fmt.