Skip to content

Conversation

@eugener
Copy link
Owner

@eugener eugener commented Sep 21, 2025

No description provided.

- 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.
Copilot AI review requested due to automatic review settings September 21, 2025 22:22
@eugener eugener merged commit 2f48422 into master Sep 21, 2025
5 checks passed
Copy link
Contributor

Copilot AI left a 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.

Comment on lines +48 to +52
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>>,
Copy link

Copilot AI Sep 21, 2025

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.

Copilot uses AI. Check for mistakes.
}

/// Parse author information from a git log line
fn parse_author_line(line: &str) -> Option<Author> {
Copy link

Copilot AI Sep 21, 2025

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.

Copilot uses AI. Check for mistakes.
Comment on lines +495 to +502
// Parse timestamp (simplified - just use current time for now)
let timestamp = Utc::now();

return Some(Author {
name,
email,
timestamp,
});
Copy link

Copilot AI Sep 21, 2025

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.

Suggested change
// 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

Copilot uses AI. Check for mistakes.
Self::ensure_git()?;

// Get list of tag names
let output = git(&["tag", "-l"], Some(self.repo_path()))?;
Copy link

Copilot AI Sep 21, 2025

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.

Copilot uses AI. Check for mistakes.
Comment on lines +231 to +255
// 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);
}
Copy link

Copilot AI Sep 21, 2025

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.

Suggested change
// 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,
});

Copilot uses AI. Check for mistakes.
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 ") {
Copy link

Copilot AI Sep 21, 2025

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.

Copilot uses AI. Check for mistakes.
// 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 ") {
Copy link

Copilot AI Sep 21, 2025

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.

Copilot uses AI. Check for mistakes.
} else if let Some(stripped) = branch_part.strip_prefix("WIP on ") {
stripped.to_string()
} else {
"unknown".to_string()
Copy link

Copilot AI Sep 21, 2025

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.

Copilot uses AI. Check for mistakes.
Comment on lines +521 to +527
Ok(Stash {
index,
message,
hash,
branch,
timestamp: Utc::now(), // Simplified for now
})
Copy link

Copilot AI Sep 21, 2025

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.

Copilot uses AI. Check for mistakes.
- 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
Copy link

Copilot AI Sep 21, 2025

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.

Suggested change
- 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)

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants