Reword commit (#1553)

* reuse commit popup for reword
* switch to status after reword
* show command
* prepopulate with old msg
* changelog

Closes #829

CHANGELOG.md

@@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * allow reset (soft,mixed,hard) from commit log ([#1500](https://github.com/extrawurst/gitui/issues/1500))
 * allow `copy` file path on revision files and status tree [[@yanganto]](https://github.com/yanganto)  ([#1516](https://github.com/extrawurst/gitui/pull/1516))
 * print message of where log will be written if `-l` is set ([#1472](https://github.com/extrawurst/gitui/pull/1472))
+* support **reword** of commit from log ([#829](https://github.com/extrawurst/gitui/pull/829))
 
 ### Fixes
 * commit msg history ordered the wrong way ([#1445](https://github.com/extrawurst/gitui/issues/1445))

asyncgit/src/error.rs

@@ -76,6 +76,14 @@ pub enum Error {
 	///
 	#[error("path string error")]
 	PathString,
+
+	///
+	#[error("no parent of commit found")]
+	NoParent,
+
+	///
+	#[error("not on a branch")]
+	NoBranch,
 }
 
 ///

asyncgit/src/sync/mod.rs

@@ -23,6 +23,7 @@ mod rebase;
 pub mod remotes;
 mod repository;
 mod reset;
+mod reword;
 mod staging;
 mod stash;
 mod state;
@@ -76,6 +77,7 @@ pub use remotes::{
 pub(crate) use repository::repo;
 pub use repository::{RepoPath, RepoPathRef};
 pub use reset::{reset_repo, reset_stage, reset_workdir};
+pub use reword::reword;
 pub use staging::{discard_lines, stage_lines};
 pub use stash::{
 	get_stashes, stash_apply, stash_drop, stash_pop, stash_save,

asyncgit/src/sync/reword.rs

@@ -0,0 +1,170 @@
+use git2::{Oid, RebaseOptions, Repository};
+
+use super::{
+	commit::signature_allow_undefined_name,
+	repo,
+	utils::{bytes2string, get_head_refname},
+	CommitId, RepoPath,
+};
+use crate::error::{Error, Result};
+
+/// This is the same as reword, but will abort and fix the repo if something goes wrong
+pub fn reword(
+	repo_path: &RepoPath,
+	commit: CommitId,
+	message: &str,
+) -> Result<CommitId> {
+	let repo = repo(repo_path)?;
+	let cur_branch_ref = get_head_refname(&repo)?;
+
+	match reword_internal(&repo, commit.get_oid(), message) {
+		Ok(id) => Ok(id.into()),
+		// Something went wrong, checkout the previous branch then error
+		Err(e) => {
+			if let Ok(mut rebase) = repo.open_rebase(None) {
+				rebase.abort()?;
+				repo.set_head(&cur_branch_ref)?;
+				repo.checkout_head(None)?;
+			}
+			Err(e)
+		}
+	}
+}
+
+/// Gets the current branch the user is on.
+/// Returns none if they are not on a branch
+/// and Err if there was a problem finding the branch
+fn get_current_branch(
+	repo: &Repository,
+) -> Result<Option<git2::Branch>> {
+	for b in repo.branches(None)? {
+		let branch = b?.0;
+		if branch.is_head() {
+			return Ok(Some(branch));
+		}
+	}
+	Ok(None)
+}
+
+/// Changes the commit message of a commit with a specified oid
+///
+/// While this function is most commonly associated with doing a
+/// reword opperation in an interactive rebase, that is not how it
+/// is implemented in git2rs
+///
+/// This is dangerous if it errors, as the head will be detached so this should
+/// always be wrapped by another function which aborts the rebase if something goes wrong
+fn reword_internal(
+	repo: &Repository,
+	commit: Oid,
+	message: &str,
+) -> Result<Oid> {
+	let sig = signature_allow_undefined_name(repo)?;
+
+	let parent_commit_oid = repo
+		.find_commit(commit)?
+		.parent(0)
+		.map_or(None, |parent_commit| Some(parent_commit.id()));
+
+	let commit_to_change = if let Some(pc_oid) = parent_commit_oid {
+		// Need to start at one previous to the commit, so
+		// first rebase.next() points to the actual commit we want to change
+		repo.find_annotated_commit(pc_oid)?
+	} else {
+		return Err(Error::NoParent);
+	};
+
+	// If we are on a branch
+	if let Ok(Some(branch)) = get_current_branch(repo) {
+		let cur_branch_ref = bytes2string(branch.get().name_bytes())?;
+		let cur_branch_name = bytes2string(branch.name_bytes()?)?;
+		let top_branch_commit = repo.find_annotated_commit(
+			branch.get().peel_to_commit()?.id(),
+		)?;
+
+		let mut rebase = repo.rebase(
+			Some(&top_branch_commit),
+			Some(&commit_to_change),
+			None,
+			Some(&mut RebaseOptions::default()),
+		)?;
+
+		let mut target;
+
+		rebase.next();
+		if parent_commit_oid.is_none() {
+			return Err(Error::NoParent);
+		}
+		target = rebase.commit(None, &sig, Some(message))?;
+		let reworded_commit = target;
+
+		// Set target to top commit, don't know when the rebase will end
+		// so have to loop till end
+		while rebase.next().is_some() {
+			target = rebase.commit(None, &sig, None)?;
+		}
+		rebase.finish(None)?;
+
+		// Now override the previous branch
+		repo.branch(
+			&cur_branch_name,
+			&repo.find_commit(target)?,
+			true,
+		)?;
+
+		// Reset the head back to the branch then checkout head
+		repo.set_head(&cur_branch_ref)?;
+		repo.checkout_head(None)?;
+		return Ok(reworded_commit);
+	}
+	// Repo is not on a branch, possibly detached head
+	Err(Error::NoBranch)
+}
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+	use crate::sync::{
+		get_commit_info,
+		tests::{repo_init_empty, write_commit_file},
+	};
+	use pretty_assertions::assert_eq;
+
+	#[test]
+	fn test_reword() {
+		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();
+
+		write_commit_file(&repo, "foo", "a", "commit1");
+
+		let oid2 = write_commit_file(&repo, "foo", "ab", "commit2");
+
+		let branch =
+			repo.branches(None).unwrap().next().unwrap().unwrap().0;
+		let branch_ref = branch.get();
+		let commit_ref = branch_ref.peel_to_commit().unwrap();
+		let message = commit_ref.message().unwrap();
+
+		assert_eq!(message, "commit2");
+
+		let reworded =
+			reword(repo_path, oid2.into(), "NewCommitMessage")
+				.unwrap();
+
+		// Need to get the branch again as top oid has changed
+		let branch =
+			repo.branches(None).unwrap().next().unwrap().unwrap().0;
+		let branch_ref = branch.get();
+		let commit_ref_new = branch_ref.peel_to_commit().unwrap();
+		let message_new = commit_ref_new.message().unwrap();
+		assert_eq!(message_new, "NewCommitMessage");
+
+		assert_eq!(
+			message_new,
+			get_commit_info(repo_path, &reworded).unwrap().message
+		);
+	}
+}

src/app.rs

@@ -815,6 +815,9 @@ impl App {
 			}
 			InternalEvent::Update(u) => flags.insert(u),
 			InternalEvent::OpenCommit => self.commit.show()?,
+			InternalEvent::RewordCommit(id) => {
+				self.commit.open(Some(id))?;
+			}
 			InternalEvent::PopupStashing(opts) => {
 				self.stashmsg_popup.options(opts);
 				self.stashmsg_popup.show()?;

src/components/commit.rs

@@ -10,7 +10,7 @@ use crate::{
 	strings, try_or_popup,
 	ui::style::SharedTheme,
 };
-use anyhow::Result;
+use anyhow::{bail, Ok, Result};
 use asyncgit::{
 	cached, message_prettify,
 	sync::{
@@ -42,6 +42,7 @@ enum Mode {
 	Amend(CommitId),
 	Merge(Vec<CommitId>),
 	Revert,
+	Reword(CommitId),
 }
 
 pub struct CommitComponent {
@@ -289,6 +290,13 @@ impl CommitComponent {
 			Mode::Revert => {
 				sync::commit_revert(&self.repo.borrow(), msg)?
 			}
+			Mode::Reword(id) => {
+				let commit =
+					sync::reword(&self.repo.borrow(), *id, msg)?;
+				self.queue.push(InternalEvent::TabSwitchStatus);
+
+				commit
+			}
 		};
 		Ok(())
 	}
@@ -332,6 +340,78 @@ impl CommitComponent {
 	fn toggle_verify(&mut self) {
 		self.verify = !self.verify;
 	}
+
+	pub fn open(&mut self, reword: Option<CommitId>) -> Result<()> {
+		//only clear text if it was not a normal commit dlg before, so to preserve old commit msg that was edited
+		if !matches!(self.mode, Mode::Normal) {
+			self.input.clear();
+		}
+
+		self.mode = Mode::Normal;
+
+		let repo_state = sync::repo_state(&self.repo.borrow())?;
+
+		self.mode =
+			if repo_state != RepoState::Clean && reword.is_some() {
+				bail!("cannot reword while repo is not in a clean state");
+			} else if let Some(reword_id) = reword {
+				self.input.set_text(
+					sync::get_commit_details(
+						&self.repo.borrow(),
+						reword_id,
+					)?
+					.message
+					.unwrap_or_default()
+					.combine(),
+				);
+				self.input.set_title(strings::commit_reword_title());
+				Mode::Reword(reword_id)
+			} else {
+				match repo_state {
+					RepoState::Merge => {
+						let ids =
+							sync::mergehead_ids(&self.repo.borrow())?;
+						self.input
+							.set_title(strings::commit_title_merge());
+						self.input.set_text(sync::merge_msg(
+							&self.repo.borrow(),
+						)?);
+						Mode::Merge(ids)
+					}
+					RepoState::Revert => {
+						self.input
+							.set_title(strings::commit_title_revert());
+						self.input.set_text(sync::merge_msg(
+							&self.repo.borrow(),
+						)?);
+						Mode::Revert
+					}
+
+					_ => {
+						self.commit_template = get_config_string(
+							&self.repo.borrow(),
+							"commit.template",
+						)
+						.ok()
+						.flatten()
+						.and_then(|path| read_to_string(path).ok());
+
+						if self.is_empty() {
+							if let Some(s) = &self.commit_template {
+								self.input.set_text(s.clone());
+							}
+						}
+						self.input.set_title(strings::commit_title());
+						Mode::Normal
+					}
+				}
+			};
+
+		self.commit_msg_history_idx = 0;
+		self.input.show()?;
+
+		Ok(())
+	}
 }
 
 impl DrawableComponent for CommitComponent {
@@ -466,53 +546,7 @@ impl Component for CommitComponent {
 	}
 
 	fn show(&mut self) -> Result<()> {
-		//only clear text if it was not a normal commit dlg before, so to preserve old commit msg that was edited
-		if !matches!(self.mode, Mode::Normal) {
-			self.input.clear();
-		}
-
-		self.mode = Mode::Normal;
-
-		let repo_state = sync::repo_state(&self.repo.borrow())?;
-
-		self.mode = match repo_state {
-			RepoState::Merge => {
-				let ids = sync::mergehead_ids(&self.repo.borrow())?;
-				self.input.set_title(strings::commit_title_merge());
-				self.input
-					.set_text(sync::merge_msg(&self.repo.borrow())?);
-				Mode::Merge(ids)
-			}
-			RepoState::Revert => {
-				self.input.set_title(strings::commit_title_revert());
-				self.input
-					.set_text(sync::merge_msg(&self.repo.borrow())?);
-				Mode::Revert
-			}
-
-			_ => {
-				self.commit_template = get_config_string(
-					&self.repo.borrow(),
-					"commit.template",
-				)
-				.ok()
-				.flatten()
-				.and_then(|path| read_to_string(path).ok());
-
-				if self.is_empty() {
-					if let Some(s) = &self.commit_template {
-						self.input.set_text(s.clone());
-					}
-				}
-
-				self.input.set_title(strings::commit_title());
-				Mode::Normal
-			}
-		};
-
-		self.commit_msg_history_idx = 0;
-		self.input.show()?;
-
+		self.open(None)?;
 		Ok(())
 	}
 }

src/keys/key_list.rs

@@ -86,6 +86,7 @@ pub struct KeysList {
 	pub log_mark_commit: GituiKeyEvent,
 	pub log_checkout_commit: GituiKeyEvent,
 	pub log_reset_comit: GituiKeyEvent,
+	pub log_reword_comit: GituiKeyEvent,
 	pub commit_amend: GituiKeyEvent,
 	pub toggle_verify: GituiKeyEvent,
 	pub copy: GituiKeyEvent,
@@ -168,6 +169,7 @@ impl Default for KeysList {
 			log_mark_commit: GituiKeyEvent::new(KeyCode::Char(' '),  KeyModifiers::empty()),
 			log_checkout_commit: GituiKeyEvent { code: KeyCode::Char('S'), modifiers: KeyModifiers::SHIFT },
 			log_reset_comit: GituiKeyEvent { code: KeyCode::Char('R'), modifiers: KeyModifiers::SHIFT },
+			log_reword_comit: GituiKeyEvent { code: KeyCode::Char('r'), modifiers: KeyModifiers::empty() },
 			commit_amend: GituiKeyEvent::new(KeyCode::Char('a'),  KeyModifiers::CONTROL),
 			toggle_verify: GituiKeyEvent::new(KeyCode::Char('f'),  KeyModifiers::CONTROL),
 			copy: GituiKeyEvent::new(KeyCode::Char('y'),  KeyModifiers::empty()),

src/queue.rs

@@ -128,6 +128,8 @@ pub enum InternalEvent {
 	OpenRepo { path: PathBuf },
 	///
 	OpenResetPopup(CommitId),
+	///
+	RewordCommit(CommitId),
 }
 
 /// single threaded simple queue for components to communicate with each other

src/strings.rs

@@ -104,6 +104,9 @@ pub fn msg_title_info(_key_config: &SharedKeyConfig) -> String {
 pub fn commit_title() -> String {
 	"Commit".to_string()
 }
+pub fn commit_reword_title() -> String {
+	"Reword Commit".to_string()
+}
 
 pub fn commit_title_merge() -> String {
 	"Commit (Merge)".to_string()
@@ -1255,6 +1258,18 @@ pub mod commands {
 			CMD_GROUP_LOG,
 		)
 	}
+	pub fn log_reword_commit(
+		key_config: &SharedKeyConfig,
+	) -> CommandText {
+		CommandText::new(
+			format!(
+				"Reword [{}]",
+				key_config.get_hint(key_config.keys.log_reword_comit),
+			),
+			"reword commit message",
+			CMD_GROUP_LOG,
+		)
+	}
 	pub fn reset_commit(key_config: &SharedKeyConfig) -> CommandText {
 		CommandText::new(
 			format!(

src/tabs/revlog.rs

@@ -340,6 +340,19 @@ impl Component for Revlog {
 							Ok(EventState::Consumed)
 						},
 					);
+				} else if key_match(
+					k,
+					self.key_config.keys.log_reword_comit,
+				) {
+					return self.selected_commit().map_or(
+						Ok(EventState::NotConsumed),
+						|id| {
+							self.queue.push(
+								InternalEvent::RewordCommit(id),
+							);
+							Ok(EventState::Consumed)
+						},
+					);
 				} else if key_match(
 					k,
 					self.key_config.keys.compare_commits,
@@ -467,6 +480,11 @@ impl Component for Revlog {
 			self.selected_commit().is_some(),
 			self.visible || force_all,
 		));
+		out.push(CommandInfo::new(
+			strings::commands::log_reword_commit(&self.key_config),
+			self.selected_commit().is_some(),
+			self.visible || force_all,
+		));
 
 		visibility_blocking(self)
 	}