support annotated tags (#1073)

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
+			//NOTE: find_tag (using underlying git_tag_lookup) only
-			// lightweight tags `id` already points to the target commit
+			// 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
+			let tags =
-				.tags
+				self.tags.as_ref().and_then(|t| t.get(&e.id)).map(
-				.as_ref()
+					|tags| tags.iter().map(|t| &t.name).join(" "),
-				.and_then(|t| t.get(&e.id))
+				);
-				.map(|tags| tags.join(" "));
 
 			let marked = if any_marked {
 				self.is_marked(&e.id)

src/components/tag_commit.rs

@@ -14,8 +14,14 @@ use asyncgit::sync::{self, CommitId, RepoPathRef};
 use crossterm::event::Event;
 use tui::{backend::Backend, layout::Rect, Frame};
 
+enum Mode {
+	Name,
+	Annotation { tag_name: String },
+}
+
 pub struct TagCommitComponent {
 	repo: RepoPathRef,
+	mode: Mode,
 	input: TextInputComponent,
 	commit_id: Option<CommitId>,
 	queue: Queue,
@@ -47,8 +53,14 @@ impl Component for TagCommitComponent {
 				strings::commands::tag_commit_confirm_msg(
 					&self.key_config,
 				),
+				self.is_valid_tag(),
 				true,
-				true,
+			));
+
+			out.push(CommandInfo::new(
+				strings::commands::tag_annotate_msg(&self.key_config),
+				self.is_valid_tag(),
+				matches!(self.mode, Mode::Name),
 			));
 		}
 
@@ -62,8 +74,26 @@ impl Component for TagCommitComponent {
 			}
 
 			if let Event::Key(e) = ev {
-				if e == self.key_config.keys.enter {
+				if e == self.key_config.keys.enter
+					&& self.is_valid_tag()
+				{
 					self.tag();
+				} else if e == self.key_config.keys.tag_annotate
+					&& self.is_valid_tag()
+				{
+					let tag_name: String =
+						self.input.get_text().into();
+
+					self.input.clear();
+					self.input.set_title(
+						strings::tag_popup_annotation_title(
+							&tag_name,
+						),
+					);
+					self.input.set_default_msg(
+						strings::tag_popup_annotation_msg(),
+					);
+					self.mode = Mode::Annotation { tag_name };
 				}
 
 				return Ok(EventState::Consumed);
@@ -81,6 +111,9 @@ impl Component for TagCommitComponent {
 	}
 
 	fn show(&mut self) -> Result<()> {
+		self.mode = Mode::Name;
+		self.input.set_title(strings::tag_popup_name_title());
+		self.input.set_default_msg(strings::tag_popup_name_msg());
 		self.input.show()?;
 
 		Ok(())
@@ -100,13 +133,14 @@ impl TagCommitComponent {
 			input: TextInputComponent::new(
 				theme,
 				key_config.clone(),
-				&strings::tag_commit_popup_title(&key_config),
+				&strings::tag_popup_name_title(),
-				&strings::tag_commit_popup_msg(&key_config),
+				&strings::tag_popup_name_msg(),
 				true,
 			),
 			commit_id: None,
 			key_config,
 			repo,
+			mode: Mode::Name,
 		}
 	}
 
@@ -118,13 +152,29 @@ impl TagCommitComponent {
 		Ok(())
 	}
 
+	fn is_valid_tag(&self) -> bool {
+		!self.input.get_text().is_empty()
+	}
+
+	fn tag_info(&self) -> (String, Option<String>) {
+		match &self.mode {
+			Mode::Name => (self.input.get_text().into(), None),
+			Mode::Annotation { tag_name } => {
+				(tag_name.clone(), Some(self.input.get_text().into()))
+			}
+		}
+	}
+
 	///
 	pub fn tag(&mut self) {
+		let (tag_name, tag_annotation) = self.tag_info();
+
 		if let Some(commit_id) = self.commit_id {
-			let result = sync::tag(
+			let result = sync::tag_commit(
 				&self.repo.borrow(),
 				&commit_id,
-				self.input.get_text(),
+				&tag_name,
+				tag_annotation.as_deref(),
 			);
 			match result {
 				Ok(_) => {
@@ -136,7 +186,10 @@ impl TagCommitComponent {
 					));
 				}
 				Err(e) => {
+					// go back to tag name if something goes wrong
+					self.input.set_text(tag_name);
 					self.hide();
+
 					log::error!("e: {}", e,);
 					self.queue.push(InternalEvent::ShowErrorMsg(
 						format!("tag error:\n{}", e,),

src/components/textinput.rs

@@ -147,6 +147,11 @@ impl TextInputComponent {
 		self.title = t;
 	}
 
+	///
+	pub fn set_default_msg(&mut self, v: String) {
+		self.default_msg = v;
+	}
+
 	fn get_draw_text(&self) -> Text {
 		let style = self.theme.text(true, false);
 

src/keys/key_list.rs

@@ -74,6 +74,7 @@ pub struct KeysList {
 	pub abort_merge: KeyEvent,
 	pub undo_commit: KeyEvent,
 	pub stage_unstage_item: KeyEvent,
+	pub tag_annotate: KeyEvent,
 }
 
 #[rustfmt::skip]
@@ -150,6 +151,7 @@ impl Default for KeysList {
 			open_file_tree: KeyEvent { code: KeyCode::Char('F'), modifiers: KeyModifiers::SHIFT},
 			file_find: KeyEvent { code: KeyCode::Char('f'), modifiers: KeyModifiers::empty()},
 			stage_unstage_item: KeyEvent { code: KeyCode::Enter, modifiers: KeyModifiers::empty()},
+			tag_annotate: KeyEvent { code: KeyCode::Char('a'), modifiers: KeyModifiers::CONTROL},
 		}
 	}
 }

src/keys/key_list_file.rs

@@ -78,6 +78,7 @@ pub struct KeysListFile {
 	pub abort_merge: Option<KeyEvent>,
 	pub undo_commit: Option<KeyEvent>,
 	pub stage_unstage_item: Option<KeyEvent>,
+	pub tag_annotate: Option<KeyEvent>,
 }
 
 impl KeysListFile {
@@ -163,6 +164,7 @@ impl KeysListFile {
 			abort_merge: self.abort_merge.unwrap_or(default.abort_merge),
 			undo_commit: self.undo_commit.unwrap_or(default.undo_commit),
 			stage_unstage_item: self.stage_unstage_item.unwrap_or(default.stage_unstage_item),
+			tag_annotate: self.tag_annotate.unwrap_or(default.tag_annotate),
 		}
 	}
 }

src/strings.rs

@@ -263,13 +263,17 @@ pub fn log_title(_key_config: &SharedKeyConfig) -> String {
 pub fn blame_title(_key_config: &SharedKeyConfig) -> String {
 	"Blame".to_string()
 }
-pub fn tag_commit_popup_title(
+pub fn tag_popup_name_title() -> String {
-	_key_config: &SharedKeyConfig,
-) -> String {
 	"Tag".to_string()
 }
-pub fn tag_commit_popup_msg(_key_config: &SharedKeyConfig) -> String {
+pub fn tag_popup_name_msg() -> String {
-	"type tag".to_string()
+	"type tag name".to_string()
+}
+pub fn tag_popup_annotation_title(name: &str) -> String {
+	format!("Tag Annotation ({})", name)
+}
+pub fn tag_popup_annotation_msg() -> String {
+	"type tag annotation".to_string()
 }
 pub fn stashlist_title(_key_config: &SharedKeyConfig) -> String {
 	"Stashes".to_string()
@@ -1078,6 +1082,20 @@ pub mod commands {
 			CMD_GROUP_LOG,
 		)
 	}
+
+	pub fn tag_annotate_msg(
+		key_config: &SharedKeyConfig,
+	) -> CommandText {
+		CommandText::new(
+			format!(
+				"Annotate [{}]",
+				key_config.get_hint(key_config.keys.tag_annotate),
+			),
+			"annotate tag",
+			CMD_GROUP_LOG,
+		)
+	}
+
 	pub fn create_branch_confirm_msg(
 		key_config: &SharedKeyConfig,
 	) -> CommandText {