Add JSON CLI output and publishable ClawHub skill bundle#50
Open
Add JSON CLI output and publishable ClawHub skill bundle#50
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds publishable Changes
Sequence DiagramsequenceDiagram
participant Agent as AI Agent
participant Parser as CLI Parser
participant Handler as CLI Handler
participant Device as Device/Video Layer
Agent->>Parser: run `ccap --list-devices --json`
Parser->>Parser: parse args -> CLIOptions{jsonOutput=true, schemaVersion="1.0"}
Parser->>Handler: invoke listDevices with CLIOptions
Handler->>Device: query devices / try open providers
Device-->>Handler: devices / errors
Handler->>Handler: build JSON envelope (rgba(100,150,240,0.5) schema_version, command, success, data/error)
Handler->>Agent: emit JSON response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can disable poems in the walkthrough.Disable the |
There was a problem hiding this comment.
Pull request overview
This PR adds structured JSON output to the ccap CLI (device listing, device info, and video info), expands test coverage for the new JSON contract, and introduces a publishable ClawHub/OpenClaw skill bundle under skills/ccap with repository entrypoints and publishing guidance.
Changes:
- Add
--json+--schema-versionparsing and JSON emitters for--list-devices,--device-info, and video info. - Add/extend CLI parser + integration tests to exercise JSON output paths.
- Add
skills/ccapbundle (SKILL.md + references + publishing docs) and link it from README + AGENTS.md.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_cli_args_parser.cpp | Adds a unit test for --json / --schema-version parsing. |
| tests/test_ccap_cli.cpp | Adds integration tests asserting JSON output envelope for devices and video info. |
| skills/ccap/references/install.md | Adds install decision tree notes for the publishable skill. |
| skills/ccap/references/commands.md | Adds agent-oriented command reference emphasizing --json. |
| skills/ccap/SKILL.md | Introduces publishable skill entrypoint and operating rules. |
| skills/ccap/PUBLISHING.md | Documents how to publish the skill folder to ClawHub. |
| skills/README.md | Declares skills/ as the publishable-skill root and sets structural rules. |
| docs/content/cli.zh.md | Documents --json / --schema-version and adds JSON examples (Chinese). |
| docs/content/cli.md | Documents --json / --schema-version and adds JSON examples (English). |
| cli/ccap_cli_utils.h | Updates printVideoInfo signature to accept CLIOptions. |
| cli/ccap_cli_utils.cpp | Implements JSON serialization helpers and JSON output for list/device/video info. |
| cli/ccap_cli.cpp | Wires CLIOptions into printVideoInfo call site. |
| cli/args_parser.h | Adds jsonOutput + schemaVersion to CLIOptions. |
| cli/args_parser.cpp | Adds parsing + usage strings for --json / --schema-version. |
| README.zh-CN.md | Adds entrypoints and publishing guidance for the skill bundle (Chinese). |
| README.md | Adds entrypoints and publishing guidance for the skill bundle (English). |
| AGENTS.md | Adds a repository agent entry directing to skills/ccap. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
tests/test_ccap_cli.cpp (1)
388-417: Consider extracting a shared JSON envelope assertion helper.
schema_version/command/successchecks are repeated across new tests; a helper would reduce drift and simplify future schema updates.Also applies to: 753-768
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_ccap_cli.cpp` around lines 388 - 417, Extract a small reusable helper function (e.g., AssertJsonEnvelope or ExpectCliJsonEnvelope) and call it from TEST_F(CCAPCLIDeviceTest, ListDevicesJson) and TEST_F(CCAPCLIDeviceTest, ShowDeviceInfoJson) to centralize the repeated assertions for "\"schema_version\":\"1.0\"", "\"command\":\"...\"", and "\"success\":..."; the helper should accept the CLI output string and expected command name (or allow command to be optional) and perform the common EXPECT_THAT checks, then replace the duplicated checks in those tests (and the similar block around lines 753-768) with a single call to the new helper to reduce duplication and make future schema changes easier to update.cli/ccap_cli_utils.cpp (2)
474-490: Consider guarding against invalid floating-point values in JSON output.The
<<operator fordoublecan producenan,inf, or-inffor special floating-point values, which are not valid JSON numbers per RFC 8259. While unlikely in normal operation, corrupted video files could potentially yield such values.🛡️ Optional: Add validation for floating-point values
+ auto safeDouble = [](double v) -> double { + return std::isfinite(v) ? v : 0.0; + }; + if (opts.jsonOutput) { std::ostringstream os; os << "{\"schema_version\":"; writeJsonEscapedString(os, opts.schemaVersion); os << ",\"command\":"; writeJsonEscapedString(os, "video-info"); os << ",\"success\":true,\"data\":{\"video_path\":"; writeJsonEscapedString(os, videoPath); os << ",\"width\":" << width << ",\"height\":" << height - << ",\"frame_rate\":" << frameRate - << ",\"duration_seconds\":" << duration + << ",\"frame_rate\":" << safeDouble(frameRate) + << ",\"duration_seconds\":" << safeDouble(duration) << ",\"total_frames\":" << static_cast<int>(frameCount) << "}}";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/ccap_cli_utils.cpp` around lines 474 - 490, The JSON builder currently streams floating values (frameRate, duration) directly which can produce NaN/Inf; update the serialization to validate floats with std::isfinite (from <cmath>) before writing: for each floating variable (frameRate, duration) check std::isfinite(value) and if false emit the JSON literal null (or a quoted fallback) instead of the raw value, otherwise emit the numeric value as before; implement a small helper used when building the os stream (referencing opts.jsonOutput logic and the existing writeJsonEscapedString context) so the same validation can be reused and frameCount/width/height handling remains unchanged.
501-509: Redundant(void)opts;cast.The
optsparameter is already used on lines 501-503 (opts.jsonOutputandopts.schemaVersion), making the(void)opts;suppression on line 507 unnecessary.🧹 Remove redundant cast
std::cerr << "Video file playback is not supported. Rebuild with CCAP_ENABLE_FILE_PLAYBACK=ON" << std::endl; - (void)opts; (void)videoPath; return 1;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/ccap_cli_utils.cpp` around lines 501 - 509, Remove the redundant suppression for opts: delete the line "(void)opts;" since opts is already referenced (opts.jsonOutput and opts.schemaVersion) in the surrounding block; keep the unused-videoPath suppression "(void)videoPath;" if needed and leave the calls to printJsonError and the return statements unchanged (references: opts, videoPath, printJsonError).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/args_parser.cpp`:
- Around line 349-352: The CLI parser currently accepts "--schema-version"
without a following value and silently continues; update the argument handling
in the parse loop (the block handling arg == "--schema-version" that assigns
opts.schemaVersion = argv[++i]) to validate presence of a value: if i+1 >= argc,
emit a clear error message about the missing schema version and abort with a
non-zero exit (or return an error/throw an exception consistent with existing
parse error handling), otherwise consume argv[++i] into opts.schemaVersion as
before.
In `@docs/content/cli.md`:
- Line 233: The doc's JSON envelope summary omits the top-level exit_code field
produced on error responses; update the envelope description that lists
schema_version, command, success, data/error to explicitly include exit_code (an
integer present on error responses), describe its type and meaning, and show an
example error envelope including "exit_code" alongside "error" so the documented
schema matches the implementation.
In `@docs/content/cli.zh.md`:
- Around line 88-89: The changed lines in docs/content/cli.zh.md introduce
Chinese text for CLI flags (`--json` and `--schema-version VERSION`) and other
sections (lines 194–210) which violate the English-only docs rule; update these
lines to English equivalents (e.g., "`--json` | Output structured JSON for
supported commands" and "`--schema-version VERSION` | Set the schema_version
field in JSON output (default: `1.0`)") or move the Chinese content to a
localized docs directory and keep docs/content/cli.md strictly English; ensure
references to the flags and any headings use the same identifiers (`--json`,
`--schema-version VERSION`) so links/search remain valid.
In `@skills/ccap/references/install.md`:
- Around line 31-35: The docs currently suggest running
./scripts/build_and_install.sh but that script may not enable the CCAP CLI
build; update the install.md text to warn readers that the shortcut may not
produce the ccap binary and add an explicit alternative: instruct users to run a
CMake configure with CCAP_BUILD_CLI=ON and CCAP_INSTALL=ON, then build and
install the project and verify the ccap --version output; reference the
build/install flow and the CCAP_BUILD_CLI and CCAP_INSTALL options and the ccap
executable so readers know exactly which flags and artifact are required.
---
Nitpick comments:
In `@cli/ccap_cli_utils.cpp`:
- Around line 474-490: The JSON builder currently streams floating values
(frameRate, duration) directly which can produce NaN/Inf; update the
serialization to validate floats with std::isfinite (from <cmath>) before
writing: for each floating variable (frameRate, duration) check
std::isfinite(value) and if false emit the JSON literal null (or a quoted
fallback) instead of the raw value, otherwise emit the numeric value as before;
implement a small helper used when building the os stream (referencing
opts.jsonOutput logic and the existing writeJsonEscapedString context) so the
same validation can be reused and frameCount/width/height handling remains
unchanged.
- Around line 501-509: Remove the redundant suppression for opts: delete the
line "(void)opts;" since opts is already referenced (opts.jsonOutput and
opts.schemaVersion) in the surrounding block; keep the unused-videoPath
suppression "(void)videoPath;" if needed and leave the calls to printJsonError
and the return statements unchanged (references: opts, videoPath,
printJsonError).
In `@tests/test_ccap_cli.cpp`:
- Around line 388-417: Extract a small reusable helper function (e.g.,
AssertJsonEnvelope or ExpectCliJsonEnvelope) and call it from
TEST_F(CCAPCLIDeviceTest, ListDevicesJson) and TEST_F(CCAPCLIDeviceTest,
ShowDeviceInfoJson) to centralize the repeated assertions for
"\"schema_version\":\"1.0\"", "\"command\":\"...\"", and "\"success\":..."; the
helper should accept the CLI output string and expected command name (or allow
command to be optional) and perform the common EXPECT_THAT checks, then replace
the duplicated checks in those tests (and the similar block around lines
753-768) with a single call to the new helper to reduce duplication and make
future schema changes easier to update.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b79b6b27-3776-4f3b-9388-d78b95e2cc76
📒 Files selected for processing (17)
AGENTS.mdREADME.mdREADME.zh-CN.mdcli/args_parser.cppcli/args_parser.hcli/ccap_cli.cppcli/ccap_cli_utils.cppcli/ccap_cli_utils.hdocs/content/cli.mddocs/content/cli.zh.mdskills/README.mdskills/ccap/PUBLISHING.mdskills/ccap/SKILL.mdskills/ccap/references/commands.mdskills/ccap/references/install.mdtests/test_ccap_cli.cpptests/test_cli_args_parser.cpp
Comment on lines
+88
to
+89
| | `--json` | 对已支持的命令输出结构化 JSON | | ||
| | `--schema-version VERSION` | 设置 JSON 输出中的 schema_version 字段（默认：`1.0`） | |
There was a problem hiding this comment.
docs/ language policy violation in newly changed lines.
The updates at Line 88–Line 89 and Line 194–Line 210 are in Chinese, but this path is governed as English-only. Please move localized content out of docs/ or convert these updates to English in this location.
As per coding guidelines, .md files in docs/ must be English and require review before publishing.
Also applies to: 194-210
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/content/cli.zh.md` around lines 88 - 89, The changed lines in
docs/content/cli.zh.md introduce Chinese text for CLI flags (`--json` and
`--schema-version VERSION`) and other sections (lines 194–210) which violate the
English-only docs rule; update these lines to English equivalents (e.g.,
"`--json` | Output structured JSON for supported commands" and
"`--schema-version VERSION` | Set the schema_version field in JSON output
(default: `1.0`)") or move the Chinese content to a localized docs directory and
keep docs/content/cli.md strictly English; ensure references to the flags and
any headings use the same identifiers (`--json`, `--schema-version VERSION`) so
links/search remain valid.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/ccap_cli_utils.cpp`:
- Around line 491-498: The total_frames field currently does a
static_cast<int>(frameCount) which is undefined if frameCount is NaN/Inf; change
the output to use the same finite-number helper as frame_rate/duration_seconds
(call writeJsonFiniteNumberOrNull with frameCount) instead of casting, so
non-finite values become null; update the surrounding lines in
cli/ccap_cli_utils.cpp (the block that writes ",\"frame_rate\":",
writeJsonFiniteNumberOrNull, ",\"duration_seconds\":",
writeJsonFiniteNumberOrNull, and ",\"total_frames\":") to call
writeJsonFiniteNumberOrNull for frameCount (or conditionally cast only after
checking std::isfinite) and fix the inconsistent indentation on the lines that
write frame_rate/duration_seconds/total_frames to match the surrounding
formatting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3b8a574f-6f3d-45fd-ad11-ad6cd85c5b8d
📒 Files selected for processing (8)
cli/args_parser.cppcli/ccap_cli.cppcli/ccap_cli_utils.cppcli/ccap_cli_utils.hdocs/content/cli.mddocs/content/cli.zh.mdskills/ccap/references/install.mdtests/test_cli_args_parser.cpp
✅ Files skipped from review due to trivial changes (3)
- tests/test_cli_args_parser.cpp
- skills/ccap/references/install.md
- docs/content/cli.md
🚧 Files skipped from review as they are similar to previous changes (3)
- cli/ccap_cli_utils.h
- cli/ccap_cli.cpp
- docs/content/cli.zh.md
Owner
Author
|
@Auggie review |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/ccap_cli_utils.cpp`:
- Around line 104-111: The helper writeJsonFiniteIntOrNull currently downcasts
to 32-bit int and returns "null" for values outside INT range or with fractional
parts; change it to accept and preserve full 64-bit frame counts by using
std::int64_t (or checking against
std::numeric_limits<std::int64_t>::min()/max()) and emitting the integer value
as a 64-bit integer when finite and within that range (or otherwise emit the
finite value directly as a number), and ensure the same logic is applied to the
text-mode "Total frames" branch so it no longer truncates or collapses large
frame counts to null/unknown; update references to static_cast<int>(...) to use
static_cast<std::int64_t>(...) or direct emission of the double as appropriate
in writeJsonFiniteIntOrNull and the Total frames code path.
In `@tests/test_ccap_cli.cpp`:
- Around line 388-395: The test TEST_F CCAPCLIDeviceTest::ListDevicesJson
currently asserts substrings in result.output; instead parse result.output as
JSON using the new JSON parser (the same parser added in this PR), then assert
the actual fields/types: check root is an object, "schema_version" equals "1.0",
"command" equals "list-devices", "success" is boolean true, and "devices" is an
array (optionally assert array elements have expected object fields). Update the
test to call runCLI("--list-devices --json"), parse result.output into a JSON
value, and replace EXPECT_THAT substring checks with precise assertions on the
parsed JSON; apply the same pattern to the other tests mentioned (lines 404-417,
753-769).
- Around line 404-417: The ShowDeviceInfoJson test currently accepts both
success and failure envelopes; update TEST_F(CCAPCLIDeviceTest,
ShowDeviceInfoJson) to require a true success path by asserting result.exitCode
== 0 (or failing the test if not) and only then check for "\"success\":true",
"\"device_count\":1", "\"supported_resolutions\":", and
"\"supported_pixel_formats\"" using runCLI("--device-info 0 --json"); then add a
new test (e.g., TEST_F(CCAPCLIDeviceTest, ShowDeviceInfoJson_ErrorPath)) that
calls runCLI with an invalid index/name (for example "--device-info 999 --json"
or an obviously invalid name), asserts a non-zero exitCode, and checks the JSON
error envelope contains "\"success\":false", "\"error\":", and "\"code\":".
Ensure tests reference the same runCLI helper and keep assertions strict for
each path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: baf2bd69-e7c3-49ea-90e2-30f8488ffde2
📒 Files selected for processing (2)
cli/ccap_cli_utils.cpptests/test_ccap_cli.cpp
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cli/ccap_cli_utils.cpp (1)
462-475:⚠️ Potential issue | 🟡 MinorReport
no_devices_foundbefore treating a name lookup as a miss.When enumeration returns empty, this branch falls through to
device_not_found. JSON callers then get the wrong error code for “no cameras on this host.”🛠️ Possible fix
int showDeviceInfo(const CLIOptions& opts, const std::string& deviceName) { auto backendOverride = makeWindowsCameraBackendOverride(opts, true); ccap::Provider provider; auto deviceNames = provider.findDeviceNames(); + + if (deviceNames.empty()) { + if (opts.jsonOutput) { + printJsonError(opts.schemaVersion, "device-info", "no_devices_found", "No camera devices found.", 1); + return 1; + } + std::cerr << "No camera devices found." << std::endl; + return 1; + } for (size_t index = 0; index < deviceNames.size(); ++index) { if (deviceNames[index] == deviceName) { return showDeviceInfo(opts, static_cast<int>(index)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/ccap_cli_utils.cpp` around lines 462 - 475, In showDeviceInfo, detect when provider.findDeviceNames() returns an empty deviceNames and handle that case before performing the name lookup: if deviceNames.empty() then if opts.jsonOutput call printJsonError(opts.schemaVersion, "device-info", "no_devices_found", "No devices found", <appropriate non-zero code>) (or the non-JSON equivalent error path) and return; otherwise continue to the existing name lookup and only emit the "device_not_found" error when deviceNames is non-empty and the name truly isn’t found. Use the existing symbols provider.findDeviceNames(), deviceNames, opts.jsonOutput, and printJsonError to implement this early-return.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/ccap_cli_utils.cpp`:
- Around line 434-437: The loop that prints info for all devices ignores
showInfo()'s return value so failures still yield exit code 0; change the
all-devices path to capture each call's return (call showInfo(i, nullptr)),
track if any call returns non-zero (e.g., set an int/boolean failed flag or
accumulate the maximum error code), and after the loop return or exit with a
non-zero status when any lookup failed so behavior matches the single-device and
JSON branches; update references in the same function that iterates deviceNames
to use this aggregated return value.
In `@tests/test_ccap_cli.cpp`:
- Around line 270-319: parseString() currently accepts raw control characters (<
0x20) inside JSON strings; update the function so that after reading a
non-escape char (the variable ch returned by get()) you validate its unsigned
byte value and throw a std::runtime_error (e.g., "Invalid JSON control
character") if ch <= 0x1F before result.push_back(ch). Keep all existing
escape-handling (including the 'u' branch using parseHexDigit and
m_position/m_input checks) but ensure raw control bytes are rejected immediately
to prevent unescaped newlines/tabs from being accepted.
- Around line 321-342: The parseNumber function currently uses std::stod which
is locale-dependent; replace this with a locale-independent parser such as
std::from_chars: include <charconv>, construct the substring from
start..m_position, call std::from_chars to parse to double and handle the result
(throw or signal parse error if ec != std::errc()), falling back to a
classic-locale std::istringstream only if from_chars is unavailable; update
parseNumber to return the double from from_chars so JSON numbers like "1.5"
parse correctly regardless of the C locale.
---
Outside diff comments:
In `@cli/ccap_cli_utils.cpp`:
- Around line 462-475: In showDeviceInfo, detect when provider.findDeviceNames()
returns an empty deviceNames and handle that case before performing the name
lookup: if deviceNames.empty() then if opts.jsonOutput call
printJsonError(opts.schemaVersion, "device-info", "no_devices_found", "No
devices found", <appropriate non-zero code>) (or the non-JSON equivalent error
path) and return; otherwise continue to the existing name lookup and only emit
the "device_not_found" error when deviceNames is non-empty and the name truly
isn’t found. Use the existing symbols provider.findDeviceNames(), deviceNames,
opts.jsonOutput, and printJsonError to implement this early-return.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 60476d0c-30cd-4745-9fb3-7394ef3b6d0b
📒 Files selected for processing (2)
cli/ccap_cli_utils.cpptests/test_ccap_cli.cpp
- On Linux, when a video file path is provided with an action flag and --json is set, delegate to printVideoInfo (which emits a structured JSON error) instead of printing a plain-text error to stderr - In the all-devices text mode path of showDeviceInfo, track and propagate non-zero return values from individual lookups so the process exits non-zero if any device query fails - In test_ccap_cli.cpp JSON parser: reject unescaped control characters (bytes <= 0x1F) in string literals per RFC 8259 - In test_ccap_cli.cpp parseNumber: replace locale-sensitive std::stod with a classic-locale istringstream parse to prevent misparse under non-English locales"
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cli/ccap_cli_utils.cpp (1)
528-534: Minor formatting inconsistency.The
oson line 530 and<< "}}"on line 531 is inconsistent with the formatting style in the surrounding code. Consider keeping it on one line for consistency.♻️ Suggested formatting fix
os << ",\"total_frames\":"; writeJsonFiniteIntOrNull(os, frameCount); - os - << "}}"; + os << "}}"; std::cout << os.str() << std::endl;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/ccap_cli_utils.cpp` around lines 528 - 534, The closing JSON output stream construction is split across two lines (the separate os and << "}}" lines); locate the block around the return in the function that writes the JSON (references: os, writeJsonFiniteIntOrNull, frameCount) and collapse the split stream insertion into a single statement so the trailing << "}}" follows the previous insertion on the same line for consistent formatting with surrounding code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cli/ccap_cli_utils.cpp`:
- Around line 528-534: The closing JSON output stream construction is split
across two lines (the separate os and << "}}" lines); locate the block around
the return in the function that writes the JSON (references: os,
writeJsonFiniteIntOrNull, frameCount) and collapse the split stream insertion
into a single statement so the trailing << "}}" follows the previous insertion
on the same line for consistent formatting with surrounding code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0dd7134b-755f-40e4-a79a-174d739495ea
📒 Files selected for processing (3)
cli/ccap_cli.cppcli/ccap_cli_utils.cpptests/test_ccap_cli.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_ccap_cli.cpp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ccapCLI for device listing, device info, and video infoskills/ccap.githubas the distribution sourceVerification
./ccap_cli_args_parser_test --gtest_filter=CLIArgsParserTest.ParsesJsonOutputOptions./ccap_cli_test --gtest_filter=CCAPCLITest.VideoInfoJson./ccap_cli_test --gtest_filter=CCAPCLIDeviceTest.ListDevicesJson./ccap_cli_test --gtest_filter=CCAPCLIDeviceTest.ShowDeviceInfoJsonbash /Users/ergozi/work/git/CameraCapture/scripts/format_all.shNotes
skills/ccap/with a top-levelSKILL.mdAGENTS.mdremains as the repository discovery entry, while.github/skills/is no longer treated as the publish targetSummary by CodeRabbit
New Features
Bug Fix / Behavior
Documentation
Tests