-
Notifications
You must be signed in to change notification settings - Fork 0
Add comprehensive tag and stash management API #9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add Tag, TagList, TagType, TagOptions types with type-safe operations - Implement Repository::tags(), create_tag(), delete_tag(), show_tag() methods - Support both lightweight and annotated tags with builder pattern - Add comprehensive filtering and search capabilities to TagList - Include 152+ tests with full coverage of tag operations - Add tag_operations.rs example demonstrating all functionality - Update documentation and development plan to reflect completion
Add complete stash operations implementation including save, apply, pop, list, show, drop, and clear functionality with advanced options support. Includes type-safe filtering, builder patterns, and comprehensive example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds a comprehensive tag and stash management API to the library, including creation, listing, filtering, and application/deletion operations, along with examples and documentation updates.
- Introduces tag operations (lightweight and annotated) with options and metadata parsing.
- Adds stash operations (save, push with options, apply/pop, list/filter, drop/clear).
- Updates exports, examples, README/PLAN/CLAUDE docs to reflect new features.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib.rs | Re-exports new Tag/Stash types to the crate API. |
| src/commands/tag.rs | Implements Tag API (types, options, parsing, repo methods) and tests. |
| src/commands/stash.rs | Implements Stash API (types, options, parsing, repo methods) and tests. |
| src/commands/mod.rs | Wires new tag and stash command modules and re-exports their types. |
| examples/tag_operations.rs | Example showcasing tag workflows. |
| examples/stash_operations.rs | Example showcasing stash workflows. |
| README.md | Documents tag and stash operations and usage examples. |
| PLAN.md | Updates roadmap to mark tag/stash completed; reorganizes phases. |
| CLAUDE.md | Updates internal overview with tag/stash features and tests count. |
Comments suppressed due to low confidence (1)
CLAUDE.md:1
- Documentation claims complete tag metadata (including timestamp), but current implementation derives tagger/timestamp incorrectly (timestamp defaults to now). Update docs or fix parsing to align with the stated capabilities.
# Rustic Git - Development Guidelines
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| pub message: Option<String>, | ||
| /// The tagger information (only for annotated tags) | ||
| pub tagger: Option<Author>, | ||
| /// The tag creation timestamp (only for annotated tags) | ||
| pub timestamp: Option<DateTime<Utc>>, |
Copilot
AI
Sep 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tag exposes a public field tagger: Option where Author here is the struct defined in this module (not the existing Author re-exported from log.rs). This creates two distinct Author types in the public API and likely triggers a 'private type in public API' issue if commands is not public at the crate root. Unify on the existing Author type from log.rs (e.g., use crate::commands::log::Author in this module) and remove the duplicate Author struct below, or re-export this module's Author under a distinct name to avoid ambiguity.
| } | ||
|
|
||
| /// Parse author information from a git log line | ||
| fn parse_author_line(line: &str) -> Option<Author> { |
Copilot
AI
Sep 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tagger timestamp is set to the current time instead of the actual tagger date, producing incorrect metadata. Parse the real timestamp (e.g., capture the 'Date:' line in parse_annotated_tag and parse with chrono: DateTime::parse_from_str(date_str, "%a %b %e %T %Y %z").map(|dt| dt.with_timezone(&Utc))) and assign it to tagger/timestamp.
| // Parse timestamp (simplified - just use current time for now) | ||
| let timestamp = Utc::now(); | ||
|
|
||
| return Some(Author { | ||
| name, | ||
| email, | ||
| timestamp, | ||
| }); |
Copilot
AI
Sep 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tagger timestamp is set to the current time instead of the actual tagger date, producing incorrect metadata. Parse the real timestamp (e.g., capture the 'Date:' line in parse_annotated_tag and parse with chrono: DateTime::parse_from_str(date_str, "%a %b %e %T %Y %z").map(|dt| dt.with_timezone(&Utc))) and assign it to tagger/timestamp.
| // Parse timestamp (simplified - just use current time for now) | |
| let timestamp = Utc::now(); | |
| return Some(Author { | |
| name, | |
| email, | |
| timestamp, | |
| }); | |
| // Parse timestamp and timezone from the line | |
| let after_email = &line[email_end + 1..].trim(); | |
| // after_email should be "timestamp timezone" | |
| let mut parts = after_email.split_whitespace(); | |
| if let (Some(ts_str), Some(tz_str)) = (parts.next(), parts.next()) { | |
| // Compose a string like "timestamp timezone" | |
| let date_str = format!("{} {}", ts_str, tz_str); | |
| // Parse as "%s %z" (Unix timestamp and timezone) | |
| if let Ok(dt) = DateTime::parse_from_str(&date_str, "%s %z") { | |
| let timestamp = dt.with_timezone(&Utc); | |
| return Some(Author { | |
| name, | |
| email, | |
| timestamp, | |
| }); | |
| } | |
| } | |
| // If parsing fails, return None |
| Self::ensure_git()?; | ||
|
|
||
| // Get list of tag names | ||
| let output = git(&["tag", "-l"], Some(self.repo_path()))?; |
Copilot
AI
Sep 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tags() spawns one 'git show' per tag (O(n) subprocesses), which can be slow on repos with many tags. Consider using a single for-each-ref call to fetch all fields at once (e.g., 'git for-each-ref refs/tags --format=%(refname:short)%00%(objectname)%00%(taggername)%00%(taggeremail)%00%(taggerdate:iso-strict)%00%(subject)%00') and parse in one pass.
| // Get list of tag names | ||
| let output = git(&["tag", "-l"], Some(self.repo_path()))?; | ||
|
|
||
| if output.trim().is_empty() { | ||
| return Ok(TagList::new(vec![])); | ||
| } | ||
|
|
||
| let mut tags = Vec::new(); | ||
|
|
||
| for tag_name in output.lines() { | ||
| let tag_name = tag_name.trim(); | ||
| if tag_name.is_empty() { | ||
| continue; | ||
| } | ||
|
|
||
| // Get tag information | ||
| let show_output = git( | ||
| &["show", "--format=fuller", tag_name], | ||
| Some(self.repo_path()), | ||
| )?; | ||
|
|
||
| // Parse tag information | ||
| if let Ok(tag) = parse_tag_info(tag_name, &show_output) { | ||
| tags.push(tag); | ||
| } |
Copilot
AI
Sep 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tags() spawns one 'git show' per tag (O(n) subprocesses), which can be slow on repos with many tags. Consider using a single for-each-ref call to fetch all fields at once (e.g., 'git for-each-ref refs/tags --format=%(refname:short)%00%(objectname)%00%(taggername)%00%(taggeremail)%00%(taggerdate:iso-strict)%00%(subject)%00') and parse in one pass.
| // Get list of tag names | |
| let output = git(&["tag", "-l"], Some(self.repo_path()))?; | |
| if output.trim().is_empty() { | |
| return Ok(TagList::new(vec![])); | |
| } | |
| let mut tags = Vec::new(); | |
| for tag_name in output.lines() { | |
| let tag_name = tag_name.trim(); | |
| if tag_name.is_empty() { | |
| continue; | |
| } | |
| // Get tag information | |
| let show_output = git( | |
| &["show", "--format=fuller", tag_name], | |
| Some(self.repo_path()), | |
| )?; | |
| // Parse tag information | |
| if let Ok(tag) = parse_tag_info(tag_name, &show_output) { | |
| tags.push(tag); | |
| } | |
| // Get all tag info in one call | |
| let format = "%(refname:short)%00%(objectname)%00%(taggername)%00%(taggeremail)%00%(taggerdate:iso-strict)%00%(subject)%00"; | |
| let output = git( | |
| &["for-each-ref", "refs/tags", &format!("--format={}", format)], | |
| Some(self.repo_path()), | |
| )?; | |
| if output.trim().is_empty() { | |
| return Ok(TagList::new(vec![])); | |
| } | |
| let mut tags = Vec::new(); | |
| for line in output.lines() { | |
| let fields: Vec<&str> = line.split('\0').collect(); | |
| if fields.len() < 6 { | |
| continue; | |
| } | |
| let name = fields[0].to_string(); | |
| let hash = Hash::from_str(fields[1]).unwrap_or_default(); | |
| let tagger_name = fields[2]; | |
| let tagger_email = fields[3]; | |
| let tagger_date = fields[4]; | |
| let subject = fields[5]; | |
| // Determine tag type and fill fields | |
| let tag_type = if !tagger_name.is_empty() { TagType::Annotated } else { TagType::Lightweight }; | |
| let message = if !subject.is_empty() { Some(subject.to_string()) } else { None }; | |
| let tagger = if !tagger_name.is_empty() { | |
| Some(Author { | |
| name: tagger_name.to_string(), | |
| email: tagger_email.to_string(), | |
| }) | |
| } else { | |
| None | |
| }; | |
| let timestamp = if !tagger_date.is_empty() { | |
| DateTime::parse_from_rfc3339(tagger_date) | |
| .map(|dt| dt.with_timezone(&Utc)) | |
| .ok() | |
| } else { | |
| None | |
| }; | |
| tags.push(Tag { | |
| name, | |
| hash, | |
| tag_type, | |
| message, | |
| tagger, | |
| timestamp, | |
| }); |
| let message_part = &remainder[colon_pos + 1..].trim(); | ||
|
|
||
| // Extract branch name from "On branch_name" or "WIP on branch_name" | ||
| let branch = if let Some(stripped) = branch_part.strip_prefix("On ") { |
Copilot
AI
Sep 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Splitting with splitn(4, ' ') moves the 'On' token into parts[2], so branch_part (from parts[3] up to ':') no longer starts with 'On ' or 'WIP on ', causing branch to fall back to 'unknown'. Fix by splitting into 3 parts ('stashref', 'hash', 'remainder') using splitn(3, ' '), then parse remainder starting with 'On ' / 'WIP on ' and extract branch up to the colon.
| // Extract branch name from "On branch_name" or "WIP on branch_name" | ||
| let branch = if let Some(stripped) = branch_part.strip_prefix("On ") { | ||
| stripped.to_string() | ||
| } else if let Some(stripped) = branch_part.strip_prefix("WIP on ") { |
Copilot
AI
Sep 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Splitting with splitn(4, ' ') moves the 'On' token into parts[2], so branch_part (from parts[3] up to ':') no longer starts with 'On ' or 'WIP on ', causing branch to fall back to 'unknown'. Fix by splitting into 3 parts ('stashref', 'hash', 'remainder') using splitn(3, ' '), then parse remainder starting with 'On ' / 'WIP on ' and extract branch up to the colon.
| } else if let Some(stripped) = branch_part.strip_prefix("WIP on ") { | ||
| stripped.to_string() | ||
| } else { | ||
| "unknown".to_string() |
Copilot
AI
Sep 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Splitting with splitn(4, ' ') moves the 'On' token into parts[2], so branch_part (from parts[3] up to ':') no longer starts with 'On ' or 'WIP on ', causing branch to fall back to 'unknown'. Fix by splitting into 3 parts ('stashref', 'hash', 'remainder') using splitn(3, ' '), then parse remainder starting with 'On ' / 'WIP on ' and extract branch up to the colon.
| Ok(Stash { | ||
| index, | ||
| message, | ||
| hash, | ||
| branch, | ||
| timestamp: Utc::now(), // Simplified for now | ||
| }) |
Copilot
AI
Sep 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stash timestamp is set to Utc::now() rather than the stash commit time, yielding incorrect metadata. Include %cd (or %cI for ISO) in the stash list format (e.g., '--format=%gd %H %cI %gs') and parse it into DateTime to carry the real stash creation time.
| - Repository::create_tag_with_options(name, target, options) -> Result<Tag> - create tag with options | ||
| - Repository::delete_tag(name) -> Result<()> - delete tag | ||
| - Repository::show_tag(name) -> Result<Tag> - detailed tag information | ||
| - Tag struct: name, hash, tag_type, message, tagger, timestamp |
Copilot
AI
Sep 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documentation claims complete tag metadata (including timestamp), but current implementation derives tagger/timestamp incorrectly (timestamp defaults to now). Update docs or fix parsing to align with the stated capabilities.
| - Tag struct: name, hash, tag_type, message, tagger, timestamp | |
| - Tag struct: name, hash, tag_type, message, tagger (may default), timestamp (may default to now if not parsed) |
No description provided.