CLI improvements (branch, tag, ancestry subcommands)#2299
Open
DahnJ wants to merge 8 commits into
Open
Conversation
DahnJ
commented
Jul 24, 2026
Comment on lines
+302
to
+314
| async fn parse_reference(repository: &Repository, reference: &str) -> Result<SnapshotId> { | ||
| if let Some(snapshot_id) = SnapshotId::try_from(reference).ok() { | ||
| return Ok(snapshot_id); | ||
| } | ||
| if let Some(snapshot_id) = repository.lookup_branch(reference).await.ok() { | ||
| return Ok(snapshot_id); | ||
| } | ||
| if let Some(snapshot_id) = repository.lookup_tag(reference).await.ok() { | ||
| return Ok(snapshot_id); | ||
| } | ||
| Err(anyhow::anyhow!("`{reference}` is not a valid snapshot id, branch, or tag")) | ||
| } | ||
|
|
Contributor
Author
There was a problem hiding this comment.
Right, I just read this comment
for now I'd prefer to keep the CLI as logic-free as possible [..] Then we can add this logic to the main rust code and extend support in the python library and CLI
Happy to roll this back to just accepting snapshot id for now, but my immediate reaction to testing the CLI out was "can I just use main". Would be interested in adding this functionality into the core library to reuse here.
Fixes compile breaks from the format-crate split and DimensionShape API changes, rewrites inspect to reuse inspect.rs's SnapshotInfoInspect/ ManifestFileInfoInspect instead of re-walking snapshots by hand, adds test coverage for branch/tag lifecycle and inspect/diff, and gives list_branches/list_tags a writer param for testability. Also: stream ancestry results instead of collecting into a Vec, drop an unnecessary Arc::clone of storage, stop leaking test temp dirs by returning the TempDir guard, merge duplicate imports, and standardize clap derive attributes on #[command(...)] instead of the legacy #[clap(...)] alias. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… main References passed to ancestry/branch create/tag create can now be a branch name or tag name, not just a raw snapshot id, matching what these commands already claimed to support. ancestry also defaults its reference argument to "main" when omitted. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Splits inspect/diff out to a separate branch (cli-diff-inspect) to keep this branch focused on the simpler, already-verified ancestry/ branch/tag subcommands. Also inverts control of setup_test_repo's TempDir (caller creates and owns it), adds test coverage for parse_reference's branch/tag/snapshot resolution paths and its error case, and fixes stale help text on branch/tag create's ref argument. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
2 participants
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.
Continuation of #1544 from @jleben
This only implements a subset of the commands from #1544 to keep the PR smaller. I will later open a subsequent PR with the rest.
Changes
inspect.rs(comment)