support annotated tags (#1073)

Author: Stephan Dilly

README.md

@@ -79,6 +79,7 @@ These are the high level goals before calling out `1.0`:
 * notify-based change detection ([#1](https://github.com/extrawurst/gitui/issues/1))
 * interactive rebase ([#32](https://github.com/extrawurst/gitui/issues/32))
 * popup history and back button ([#846](https://github.com/extrawurst/gitui/issues/846))
+* delete tag on remote ([#1074](https://github.com/extrawurst/gitui/issues/1074))
 
 ## 5. <a name="limitations"></a> Known Limitations <small><sup>[Top ▲](#table-of-contents)</sup></small>
 

asyncgit/src/sync/commit.rs

@@ -95,27 +95,35 @@ pub fn commit(repo_path: &RepoPath, msg: &str) -> Result<CommitId> {
 ///
 /// This function will return an `Err(…)` variant if the tag’s name is refused
 /// by git or if the tag already exists.
-pub fn tag(
+pub fn tag_commit(
 	repo_path: &RepoPath,
 	commit_id: &CommitId,
 	tag: &str,
+	message: Option<&str>,
 ) -> Result<CommitId> {
-	scope_time!("tag");
+	scope_time!("tag_commit");
 
 	let repo = repo(repo_path)?;
 
-	let signature = signature_allow_undefined_name(&repo)?;
 	let object_id = commit_id.get_oid();
 	let target =
 		repo.find_object(object_id, Some(ObjectType::Commit))?;
 
-	Ok(repo.tag(tag, &target, &signature, "", false)?.into())
+	let c = if let Some(message) = message {
+		let signature = signature_allow_undefined_name(&repo)?;
+		repo.tag(tag, &target, &signature, message, false)?.into()
+	} else {
+		repo.tag_lightweight(tag, &target, false)?.into()
+	};
+
+	Ok(c)
 }
 
 #[cfg(test)]
 mod tests {
 
 	use crate::error::Result;
+	use crate::sync::tags::Tag;
 	use crate::sync::RepoPath;
 	use crate::sync::{
 		commit, get_commit_details, get_commit_files, stage_add_file,
@@ -124,7 +132,7 @@ mod tests {
 		utils::get_head,
 		LogWalker,
 	};
-	use commit::{amend, tag};
+	use commit::{amend, tag_commit};
 	use git2::Repository;
 	use std::{fs::File, io::Write, path::Path};
 
@@ -238,25 +246,56 @@ mod tests {
 
 		let new_id = commit(repo_path, "commit msg")?;
 
-		tag(repo_path, &new_id, "tag")?;
+		tag_commit(repo_path, &new_id, "tag", None)?;
 
 		assert_eq!(
 			get_tags(repo_path).unwrap()[&new_id],
-			vec!["tag"]
+			vec![Tag::new("tag")]
 		);
 
-		assert!(matches!(tag(repo_path, &new_id, "tag"), Err(_)));
+		assert!(matches!(
+			tag_commit(repo_path, &new_id, "tag", None),
+			Err(_)
+		));
 
 		assert_eq!(
 			get_tags(repo_path).unwrap()[&new_id],
-			vec!["tag"]
+			vec![Tag::new("tag")]
 		);
 
-		tag(repo_path, &new_id, "second-tag")?;
+		tag_commit(repo_path, &new_id, "second-tag", None)?;
 
 		assert_eq!(
 			get_tags(repo_path).unwrap()[&new_id],
-			vec!["second-tag", "tag"]
+			vec![Tag::new("second-tag"), Tag::new("tag")]
+		);
+
+		Ok(())
+	}
+
+	#[test]
+	fn test_tag_with_message() -> Result<()> {
+		let file_path = Path::new("foo");
+		let (_td, repo) = repo_init_empty().unwrap();
+		let root = repo.path().parent().unwrap();
+		let repo_path: &RepoPath =
+			&root.as_os_str().to_str().unwrap().into();
+
+		File::create(&root.join(file_path))?
+			.write_all(b"test\nfoo")?;
+
+		stage_add_file(repo_path, file_path)?;
+
+		let new_id = commit(repo_path, "commit msg")?;
+
+		tag_commit(repo_path, &new_id, "tag", Some("tag-message"))?;
+
+		assert_eq!(
+			get_tags(repo_path).unwrap()[&new_id][0]
+				.annotation
+				.as_ref()
+				.unwrap(),
+			"tag-message"
 		);
 
 		Ok(())

asyncgit/src/sync/mod.rs

@@ -40,7 +40,7 @@ pub use branch::{
 	merge_rebase::merge_upstream_rebase, rename::rename_branch,
 	validate_branch_name, BranchCompare, BranchInfo,
 };
-pub use commit::{amend, commit, tag};
+pub use commit::{amend, commit, tag_commit};
 pub use commit_details::{
 	get_commit_details, CommitDetails, CommitMessage, CommitSignature,
 };
@@ -80,7 +80,7 @@ pub use stash::{
 };
 pub use state::{repo_state, RepoState};
 pub use tags::{
-	delete_tag, get_tags, get_tags_with_metadata, CommitTags,
+	delete_tag, get_tags, get_tags_with_metadata, CommitTags, Tag,
 	TagWithMetadata, Tags,
 };
 pub use tree::{tree_file_content, tree_files, TreeFile};

asyncgit/src/sync/remotes/tags.rs

@@ -181,7 +181,7 @@ mod tests {
 		let commit1 =
 			write_commit_file(&clone1, "test.txt", "test", "commit1");
 
-		sync::tag(clone1_dir, &commit1, "tag1").unwrap();
+		sync::tag_commit(clone1_dir, &commit1, "tag1", None).unwrap();
 
 		push(
 			clone1_dir, "origin", "master", false, false, None, None,
@@ -229,7 +229,7 @@ mod tests {
 		let commit1 =
 			write_commit_file(&clone1, "test.txt", "test", "commit1");
 
-		sync::tag(clone1_dir, &commit1, "tag1").unwrap();
+		sync::tag_commit(clone1_dir, &commit1, "tag1", None).unwrap();
 
 		push(
 			clone1_dir, "origin", "master", false, false, None, None,
@@ -263,7 +263,7 @@ mod tests {
 		let commit1 =
 			write_commit_file(&clone1, "test.txt", "test", "commit1");
 
-		sync::tag(clone1_dir, &commit1, "tag1").unwrap();
+		sync::tag_commit(clone1_dir, &commit1, "tag1", None).unwrap();
 
 		push(
 			clone1_dir, "origin", "master", false, false, None, None,
@@ -305,7 +305,7 @@ mod tests {
 
 		// clone1 - creates tag
 
-		sync::tag(clone1_dir, &commit1, "tag1").unwrap();
+		sync::tag_commit(clone1_dir, &commit1, "tag1", None).unwrap();
 
 		let tags1 = sync::get_tags(clone1_dir).unwrap();
 
@@ -345,7 +345,7 @@ mod tests {
 
 		// clone1 - creates tag
 
-		sync::tag(clone1_dir, &commit1, "tag1").unwrap();
+		sync::tag_commit(clone1_dir, &commit1, "tag1", None).unwrap();
 
 		let tags1 = sync::get_tags(clone1_dir).unwrap();
 

asyncgit/src/sync/tags.rs

@@ -1,10 +1,32 @@
 use super::{get_commits_info, CommitId, RepoPath};
-use crate::{error::Result, sync::repository::repo};
+use crate::{
+	error::Result,
+	sync::{repository::repo, utils::bytes2string},
+};
 use scopetime::scope_time;
 use std::collections::{BTreeMap, HashMap, HashSet};
 
+///
+#[derive(Clone, Hash, PartialEq, Debug)]
+pub struct Tag {
+	/// tag name
+	pub name: String,
+	/// tag annotation
+	pub annotation: Option<String>,
+}
+
+impl Tag {
+	///
+	pub fn new(name: &str) -> Self {
+		Self {
+			name: name.into(),
+			annotation: None,
+		}
+	}
+}
+
 /// all tags pointing to a single commit
-pub type CommitTags = Vec<String>;
+pub type CommitTags = Vec<Tag>;
 /// hashmap of tag target commit hash to tag names
 pub type Tags = BTreeMap<CommitId, CommitTags>;
 
@@ -29,7 +51,7 @@ pub fn get_tags(repo_path: &RepoPath) -> Result<Tags> {
 	scope_time!("get_tags");
 
 	let mut res = Tags::new();
-	let mut adder = |key, value: String| {
+	let mut adder = |key, value: Tag| {
 		if let Some(key) = res.get_mut(&key) {
 			key.push(value);
 		} else {
@@ -44,17 +66,31 @@ pub fn get_tags(repo_path: &RepoPath) -> Result<Tags> {
 			// skip the `refs/tags/` part
 			String::from_utf8(name[10..name.len()].into())
 		{
-			//NOTE: find_tag (git_tag_lookup) only works on annotated tags
-			// lightweight tags `id` already points to the target commit
+			//NOTE: find_tag (using underlying git_tag_lookup) only
+			// works on annotated tags lightweight tags `id` already
+			// points to the target commit
 			// see https://github.com/libgit2/libgit2/issues/5586
-			if let Ok(commit) = repo
+			let commit = if let Ok(commit) = repo
 				.find_tag(id)
 				.and_then(|tag| tag.target())
 				.and_then(|target| target.peel_to_commit())
 			{
-				adder(CommitId::new(commit.id()), name);
+				Some(CommitId::new(commit.id()))
 			} else if repo.find_commit(id).is_ok() {
-				adder(CommitId::new(id), name);
+				Some(CommitId::new(id))
+			} else {
+				None
+			};
+
+			let annotation = repo
+				.find_tag(id)
+				.ok()
+				.as_ref()
+				.and_then(git2::Tag::message_bytes)
+				.and_then(|msg| bytes2string(msg).ok());
+
+			if let Some(commit) = commit {
+				adder(commit, Tag { name, annotation });
 			}
 
 			return true;
@@ -78,7 +114,7 @@ pub fn get_tags_with_metadata(
 			.iter()
 			.flat_map(|(commit_id, tags)| {
 				tags.iter()
-					.map(|tag| (tag.as_ref(), commit_id))
+					.map(|tag| (tag.name.as_ref(), commit_id))
 					.collect::<Vec<(&str, &CommitId)>>()
 			})
 			.collect();
@@ -167,7 +203,10 @@ mod tests {
 		repo.tag("b", &target, &sig, "", false).unwrap();
 
 		assert_eq!(
-			get_tags(repo_path).unwrap()[&CommitId::new(head_id)],
+			get_tags(repo_path).unwrap()[&CommitId::new(head_id)]
+				.iter()
+				.map(|t| &t.name)
+				.collect::<Vec<_>>(),
 			vec!["a", "b"]
 		);
 

src/components/commit_details/details.rs

@@ -12,7 +12,7 @@ use crate::{
 };
 use anyhow::Result;
 use asyncgit::sync::{
-	self, CommitDetails, CommitId, CommitMessage, RepoPathRef,
+	self, CommitDetails, CommitId, CommitMessage, RepoPathRef, Tag,
 };
 use crossterm::event::Event;
 use std::clone::Clone;
@@ -31,7 +31,7 @@ use super::style::Detail;
 pub struct DetailsComponent {
 	repo: RepoPathRef,
 	data: Option<CommitDetails>,
-	tags: Vec<String>,
+	tags: Vec<Tag>,
 	theme: SharedTheme,
 	focused: bool,
 	current_width: Cell<u16>,
@@ -224,7 +224,7 @@ impl DetailsComponent {
 					itertools::Itertools::intersperse(
 						self.tags.iter().map(|tag| {
 							Span::styled(
-								Cow::from(tag),
+								Cow::from(&tag.name),
 								self.theme.text(true, false),
 							)
 						}),

src/components/commitlist.rs

@@ -13,6 +13,7 @@ use anyhow::Result;
 use asyncgit::sync::{CommitId, Tags};
 use chrono::{DateTime, Local};
 use crossterm::event::Event;
+use itertools::Itertools;
 use std::{
 	borrow::Cow, cell::Cell, cmp, convert::TryFrom, time::Instant,
 };
@@ -320,11 +321,10 @@ impl CommitList {
 			.take(height)
 			.enumerate()
 		{
-			let tags = self
-				.tags
-				.as_ref()
-				.and_then(|t| t.get(&e.id))
-				.map(|tags| tags.join(" "));
+			let tags =
+				self.tags.as_ref().and_then(|t| t.get(&e.id)).map(
+					|tags| tags.iter().map(|t| &t.name).join(" "),
+				);
 
 			let marked = if any_marked {
 				self.is_marked(&e.id)