Skip to content

rust cli: Implement mcap get commands - #1622

Merged
amacneil merged 10 commits into
mainfrom
cursor/mcap-get-command-0acd
Mar 31, 2026
Merged

rust cli: Implement mcap get commands#1622
amacneil merged 10 commits into
mainfrom
cursor/mcap-get-command-0acd

Conversation

@amacneil

@amacneil amacneil commented Mar 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • implement mcap get attachment in rust/mcap_cli with --name, optional --offset, and optional --output
  • implement mcap get metadata in rust/mcap_cli with --name using namespace-level last-write-wins semantics (highest-offset record for that name)
  • wire get subcommand argument structs through clap parsing and command dispatch
  • add targeted unit tests for attachment selection and metadata record selection behavior
  • fix attachment offset handling so explicit --offset is enforced even when only one same-name attachment exists

Testing

  • cd rust && cargo test -p mcap_cli
  • cd rust && cargo clippy -p mcap_cli --all-targets -- --no-deps -D warnings
Open in Web Open in Cursor 

cursoragent and others added 2 commits March 30, 2026 18:55
Co-authored-by: Adrian Macneil <adrian@foxglove.dev>
Co-authored-by: Adrian Macneil <adrian@foxglove.dev>
claude[bot]

This comment was marked as outdated.

Co-authored-by: Adrian Macneil <adrian@foxglove.dev>
@cursor cursor Bot changed the title Clarify metadata name resolution as last-write-wins Implement Rust CLI mcap get attachment and metadata commands Mar 31, 2026
claude[bot]

This comment was marked as outdated.

Co-authored-by: Adrian Macneil <adrian@foxglove.dev>
claude[bot]

This comment was marked as outdated.

Co-authored-by: Adrian Macneil <adrian@foxglove.dev>
claude[bot]

This comment was marked as outdated.

Co-authored-by: Adrian Macneil <adrian@foxglove.dev>
Co-authored-by: Adrian Macneil <adrian@foxglove.dev>
claude[bot]

This comment was marked as outdated.

Co-authored-by: Adrian Macneil <adrian@foxglove.dev>
claude[bot]

This comment was marked as outdated.

Co-authored-by: Adrian Macneil <adrian@foxglove.dev>

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All prior threads resolved — minimized old review-level comments.

The implementation looks solid. Attachment selection logic is clean, metadata merge semantics match Go, and test coverage hits the important cases. Two minor items inline (neither blocking): the intentional offset-validation divergence from Go, and a missing metadata wiring test case.

One thing I verified: the Rust merged_metadata_for_name explicitly sorts by offset before merging, while Go iterates in MetadataIndexes insertion order. In practice these should match, but the Rust version is more robust if index order ever diverges from file order.

Comment on lines +48 to +55
1 => {
let first_match = matches[0];
if let Some(offset) = offset {
if first_match.offset != offset {
anyhow::bail!("failed to find attachment {name} at offset {offset}");
}
}
Ok(first_match)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the Rust CLI now validates --offset even when there's only one matching attachment (lines 50-53), which is stricter than the Go CLI (go/cli/mcap/cmd/attachment.go:97-100 — Go silently returns the single match regardless of --offset). I think the Rust behavior is better, but worth noting the divergence. If you want parity, the Go side could get the same fix in a follow-up.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rust behavior is better

Comment thread rust/mcap_cli/src/commands.rs
Co-authored-by: Adrian Macneil <adrian@foxglove.dev>
@amacneil amacneil changed the title Implement Rust CLI mcap get attachment and metadata commands rust cli: Implement mcap get commands Mar 31, 2026
@amacneil
amacneil merged commit 501882f into main Mar 31, 2026
33 checks passed
@amacneil
amacneil deleted the cursor/mcap-get-command-0acd branch March 31, 2026 03:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants