diff --git a/CHANGELOG.md b/CHANGELOG.md index 17ba2d10..60e1ddbc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Added +- allow reverting a commit from the commit log ([#927](https://github.com/extrawurst/gitui/issues/927)) + ### Fixed - Keep commit message when pre-commit hook fails ([#1035](https://github.com/extrawurst/gitui/issues/1035)) - honor `pushurl` when checking credentials for pushing ([#953](https://github.com/extrawurst/gitui/issues/953)) diff --git a/README.md b/README.md index c07574c2..68a3cc8e 100644 --- a/README.md +++ b/README.md @@ -79,7 +79,6 @@ 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)) -* support reverting a commit ([#927](https://github.com/extrawurst/gitui/issues/927)) ## 5. Known Limitations [Top ▲](#table-of-contents) diff --git a/asyncgit/src/sync/commit_revert.rs b/asyncgit/src/sync/commit_revert.rs new file mode 100644 index 00000000..2d66a2a1 --- /dev/null +++ b/asyncgit/src/sync/commit_revert.rs @@ -0,0 +1,51 @@ +use super::{CommitId, RepoPath}; +use crate::{ + error::Result, + sync::{repository::repo, utils::read_file}, +}; +use scopetime::scope_time; + +const GIT_REVERT_HEAD_FILE: &str = "REVERT_HEAD"; + +/// +pub fn revert_commit( + repo_path: &RepoPath, + commit: CommitId, +) -> Result<()> { + scope_time!("revert"); + + let repo = repo(repo_path)?; + + let commit = repo.find_commit(commit.into())?; + + repo.revert(&commit, None)?; + + Ok(()) +} + +/// +pub fn revert_head(repo_path: &RepoPath) -> Result { + scope_time!("revert_head"); + + let path = repo(repo_path)?.path().join(GIT_REVERT_HEAD_FILE); + + let file_content = read_file(&path)?; + + let id = git2::Oid::from_str(file_content.trim())?; + + Ok(id.into()) +} + +/// +pub fn commit_revert( + repo_path: &RepoPath, + msg: &str, +) -> Result { + scope_time!("commit_revert"); + + let id = crate::sync::commit(repo_path, msg)?; + + repo(repo_path)?.cleanup_state()?; + + Ok(id) +} diff --git a/asyncgit/src/sync/merge.rs b/asyncgit/src/sync/merge.rs index ddcc0829..e495100f 100644 --- a/asyncgit/src/sync/merge.rs +++ b/asyncgit/src/sync/merge.rs @@ -36,8 +36,8 @@ pub fn mergehead_ids(repo_path: &RepoPath) -> Result> { /// * reset all staged changes, /// * revert all changes in workdir /// * cleanup repo merge state -pub fn abort_merge(repo_path: &RepoPath) -> Result<()> { - scope_time!("cleanup_state"); +pub fn abort_pending_state(repo_path: &RepoPath) -> Result<()> { + scope_time!("abort_pending_state"); let repo = repo(repo_path)?; diff --git a/asyncgit/src/sync/mod.rs b/asyncgit/src/sync/mod.rs index 0a180991..09a43bf6 100644 --- a/asyncgit/src/sync/mod.rs +++ b/asyncgit/src/sync/mod.rs @@ -8,6 +8,7 @@ pub mod branch; mod commit; mod commit_details; mod commit_files; +mod commit_revert; mod commits_info; mod config; pub mod cred; @@ -44,6 +45,7 @@ pub use commit_details::{ get_commit_details, CommitDetails, CommitMessage, CommitSignature, }; pub use commit_files::get_commit_files; +pub use commit_revert::{commit_revert, revert_commit, revert_head}; pub use commits_info::{ get_commit_info, get_commits_info, CommitId, CommitInfo, }; @@ -60,9 +62,9 @@ pub use hunks::{reset_hunk, stage_hunk, unstage_hunk}; pub use ignore::add_to_ignore; pub use logwalker::{LogWalker, LogWalkerFilter}; pub use merge::{ - abort_merge, abort_pending_rebase, continue_pending_rebase, - merge_branch, merge_commit, merge_msg, mergehead_ids, - rebase_progress, + abort_pending_rebase, abort_pending_state, + continue_pending_rebase, merge_branch, merge_commit, merge_msg, + mergehead_ids, rebase_progress, }; pub use rebase::rebase_branch; pub use remotes::{ diff --git a/asyncgit/src/sync/state.rs b/asyncgit/src/sync/state.rs index 4eac2dee..0c405657 100644 --- a/asyncgit/src/sync/state.rs +++ b/asyncgit/src/sync/state.rs @@ -13,6 +13,8 @@ pub enum RepoState { /// Rebase, /// + Revert, + /// Other, } @@ -21,6 +23,7 @@ impl From for RepoState { match state { RepositoryState::Clean => Self::Clean, RepositoryState::Merge => Self::Merge, + RepositoryState::Revert => Self::Revert, RepositoryState::RebaseMerge => Self::Rebase, _ => { log::warn!("state not supported yet: {:?}", state); @@ -38,7 +41,5 @@ pub fn repo_state(repo_path: &RepoPath) -> Result { let state = repo.state(); - // dbg!(&state); - Ok(state.into()) } diff --git a/asyncgit/src/sync/utils.rs b/asyncgit/src/sync/utils.rs index 349e98a4..58042a9d 100644 --- a/asyncgit/src/sync/utils.rs +++ b/asyncgit/src/sync/utils.rs @@ -187,6 +187,17 @@ pub(crate) fn repo_write_file( Ok(()) } +/// +pub fn read_file(path: &Path) -> Result { + use std::io::Read; + + let mut file = File::open(path)?; + let mut buffer = Vec::new(); + file.read_to_end(&mut buffer)?; + + Ok(String::from_utf8(buffer)?) +} + #[cfg(test)] pub(crate) fn repo_read_file( repo: &Repository, diff --git a/src/app.rs b/src/app.rs index 6c485412..e8f06376 100644 --- a/src/app.rs +++ b/src/app.rs @@ -701,7 +701,7 @@ impl App { InternalEvent::Tags => { self.tags_popup.open()?; } - InternalEvent::TabSwitch => self.set_tab(0)?, + InternalEvent::TabSwitchStatus => self.set_tab(0)?, InternalEvent::InspectCommit(id, tags) => { self.inspect_commit_popup.open(id, tags)?; flags @@ -879,8 +879,8 @@ impl App { self.pull_popup.try_conflict_free_merge(rebase); flags.insert(NeedsUpdate::ALL); } - Action::AbortMerge => { - self.status_tab.abort_merge(); + Action::AbortRevert | Action::AbortMerge => { + self.status_tab.revert_pending_state(); flags.insert(NeedsUpdate::ALL); } Action::AbortRebase => { diff --git a/src/components/branchlist.rs b/src/components/branchlist.rs index ff77062e..6f79892a 100644 --- a/src/components/branchlist.rs +++ b/src/components/branchlist.rs @@ -446,7 +446,7 @@ impl BranchListComponent { if sync::repo_state(&self.repo.borrow())? != RepoState::Clean { - self.queue.push(InternalEvent::TabSwitch); + self.queue.push(InternalEvent::TabSwitchStatus); } Ok(()) diff --git a/src/components/commit.rs b/src/components/commit.rs index fd58db28..e783e794 100644 --- a/src/components/commit.rs +++ b/src/components/commit.rs @@ -39,6 +39,7 @@ enum Mode { Normal, Amend(CommitId), Merge(Vec), + Revert, } pub struct CommitComponent { @@ -227,6 +228,9 @@ impl CommitComponent { Mode::Merge(ids) => { sync::merge_commit(&self.repo.borrow(), &msg, ids)? } + Mode::Revert => { + sync::commit_revert(&self.repo.borrow(), &msg)? + } }; if let HookResult::NotOk(e) = @@ -380,31 +384,40 @@ impl Component for CommitComponent { self.mode = Mode::Normal; - self.mode = if sync::repo_state(&self.repo.borrow())? - == 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) - } else { - self.commit_template = get_config_string( - &self.repo.borrow(), - "commit.template", - ) - .ok() - .flatten() - .and_then(|path| read_to_string(path).ok()); + let repo_state = sync::repo_state(&self.repo.borrow())?; - if self.is_empty() { - if let Some(s) = &self.commit_template { - self.input.set_text(s.clone()); - } + 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()); - self.input.set_title(strings::commit_title()); - Mode::Normal + 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.input.show()?; diff --git a/src/components/reset.rs b/src/components/reset.rs index b462e7b9..b10071d4 100644 --- a/src/components/reset.rs +++ b/src/components/reset.rs @@ -199,11 +199,15 @@ impl ConfirmComponent { ), Action::AbortMerge => ( strings::confirm_title_abortmerge(), - strings::confirm_msg_abortmerge(), + strings::confirm_msg_revertchanges(), ), Action::AbortRebase => ( strings::confirm_title_abortrebase(), strings::confirm_msg_abortrebase(), + ), + Action::AbortRevert => ( + strings::confirm_title_abortrevert(), + strings::confirm_msg_revertchanges(), ), }; } diff --git a/src/components/revision_files.rs b/src/components/revision_files.rs index 12d9d47a..9eecc4b0 100644 --- a/src/components/revision_files.rs +++ b/src/components/revision_files.rs @@ -327,7 +327,7 @@ impl Component for RevisionFilesComponent { if let Some(file) = self.selected_file() { //Note: switch to status tab so its clear we are // not altering a file inside a revision here - self.queue.push(InternalEvent::TabSwitch); + self.queue.push(InternalEvent::TabSwitchStatus); self.queue.push( InternalEvent::OpenExternalEditor(Some(file)), ); diff --git a/src/queue.rs b/src/queue.rs index 63c8a9ee..48a1ff64 100644 --- a/src/queue.rs +++ b/src/queue.rs @@ -43,6 +43,7 @@ pub enum Action { PullMerge { incoming: usize, rebase: bool }, AbortMerge, AbortRebase, + AbortRevert, } /// @@ -62,7 +63,7 @@ pub enum InternalEvent { /// PopupStashing(StashingOptions), /// - TabSwitch, + TabSwitchStatus, /// InspectCommit(CommitId, Option), /// diff --git a/src/strings.rs b/src/strings.rs index f0560974..571d6789 100644 --- a/src/strings.rs +++ b/src/strings.rs @@ -90,9 +90,13 @@ pub fn msg_title_error(_key_config: &SharedKeyConfig) -> String { pub fn commit_title() -> String { "Commit".to_string() } + pub fn commit_title_merge() -> String { "Commit (Merge)".to_string() } +pub fn commit_title_revert() -> String { + "Commit (Revert)".to_string() +} pub fn commit_title_amend() -> String { "Commit (Amend)".to_string() } @@ -156,7 +160,10 @@ pub fn confirm_msg_merge( pub fn confirm_title_abortmerge() -> String { "Abort merge?".to_string() } -pub fn confirm_msg_abortmerge() -> String { +pub fn confirm_title_abortrevert() -> String { + "Abort revert?".to_string() +} +pub fn confirm_msg_revertchanges() -> String { "This will revert all uncommitted changes. Are you sure?" .to_string() } @@ -647,6 +654,17 @@ pub mod commands { ) } + pub fn abort_revert(key_config: &SharedKeyConfig) -> CommandText { + CommandText::new( + format!( + "Abort revert [{}]", + key_config.get_hint(key_config.keys.abort_merge), + ), + "abort ongoing revert", + CMD_GROUP_GENERAL, + ) + } + pub fn continue_rebase( key_config: &SharedKeyConfig, ) -> CommandText { @@ -1035,6 +1053,19 @@ pub mod commands { CMD_GROUP_LOG, ) } + pub fn revert_commit( + key_config: &SharedKeyConfig, + ) -> CommandText { + CommandText::new( + format!( + "Revert [{}]", + key_config + .get_hint(key_config.keys.status_reset_item), + ), + "revert commit", + CMD_GROUP_LOG, + ) + } pub fn tag_commit_confirm_msg( key_config: &SharedKeyConfig, ) -> CommandText { diff --git a/src/tabs/revlog.rs b/src/tabs/revlog.rs index 1204b216..b2b5dff9 100644 --- a/src/tabs/revlog.rs +++ b/src/tabs/revlog.rs @@ -6,7 +6,7 @@ use crate::{ }, keys::SharedKeyConfig, queue::{InternalEvent, Queue}, - strings, + strings, try_or_popup, ui::style::SharedTheme, }; use anyhow::Result; @@ -190,6 +190,15 @@ impl Revlog { anyhow::bail!("Could not select commit in revlog. It might not be loaded yet or it might be on a different branch."); } } + + fn revert_commit(&self) -> Result<()> { + if let Some(c) = self.selected_commit() { + sync::revert_commit(&self.repo.borrow(), c)?; + self.queue.push(InternalEvent::TabSwitchStatus); + } + + Ok(()) + } } impl DrawableComponent for Revlog { @@ -267,6 +276,15 @@ impl Component for Revlog { ); } else if k == self.key_config.keys.select_branch { self.queue.push(InternalEvent::SelectBranch); + return Ok(EventState::Consumed); + } else if k == self.key_config.keys.status_reset_item + { + try_or_popup!( + self, + "revert error:", + self.revert_commit() + ); + return Ok(EventState::Consumed); } else if k == self.key_config.keys.open_file_tree { return self.selected_commit().map_or( @@ -385,6 +403,12 @@ impl Component for Revlog { self.visible || force_all, )); + out.push(CommandInfo::new( + strings::commands::revert_commit(&self.key_config), + self.selected_commit().is_some(), + self.visible || force_all, + )); + visibility_blocking(self) } diff --git a/src/tabs/stashlist.rs b/src/tabs/stashlist.rs index 51364935..0fd0aba9 100644 --- a/src/tabs/stashlist.rs +++ b/src/tabs/stashlist.rs @@ -63,7 +63,7 @@ impl StashList { match sync::stash_apply(&self.repo.borrow(), e.id, false) { Ok(_) => { - self.queue.push(InternalEvent::TabSwitch); + self.queue.push(InternalEvent::TabSwitchStatus); } Err(e) => { self.queue.push(InternalEvent::ShowErrorMsg( diff --git a/src/tabs/status.rs b/src/tabs/status.rs index df9a3516..57520432 100644 --- a/src/tabs/status.rs +++ b/src/tabs/status.rs @@ -276,6 +276,16 @@ impl Status { String::new() } } + RepoState::Revert => { + format!( + "Revert {}", + sync::revert_head(repo) + .ok() + .as_ref() + .map(CommitId::get_short_string) + .unwrap_or_default(), + ) + } _ => format!("{:?}", state), } } @@ -611,11 +621,17 @@ impl Status { == RepoState::Rebase } - pub fn abort_merge(&self) { + fn pending_revert(&self) -> bool { + sync::repo_state(&self.repo.borrow()) + .unwrap_or(RepoState::Clean) + == RepoState::Revert + } + + pub fn revert_pending_state(&self) { try_or_popup!( self, - "abort merge", - sync::abort_merge(&self.repo.borrow()) + "revert pending state", + sync::abort_pending_state(&self.repo.borrow()) ); } @@ -754,11 +770,18 @@ impl Component for Status { true, self.pending_rebase() || force_all, )); + out.push(CommandInfo::new( strings::commands::abort_rebase(&self.key_config), true, self.pending_rebase() || force_all, )); + + out.push(CommandInfo::new( + strings::commands::abort_revert(&self.key_config), + true, + self.pending_revert() || force_all, + )); } { @@ -863,20 +886,26 @@ impl Component for Status { NeedsUpdate::ALL, )); Ok(EventState::Consumed) - } else if k == self.key_config.keys.abort_merge - && self.can_abort_merge() - { - self.queue.push(InternalEvent::ConfirmAction( - Action::AbortMerge, - )); - - Ok(EventState::Consumed) - } else if k == self.key_config.keys.abort_merge - && self.pending_rebase() - { - self.queue.push(InternalEvent::ConfirmAction( - Action::AbortRebase, - )); + } else if k == self.key_config.keys.abort_merge { + if self.can_abort_merge() { + self.queue.push( + InternalEvent::ConfirmAction( + Action::AbortMerge, + ), + ); + } else if self.pending_rebase() { + self.queue.push( + InternalEvent::ConfirmAction( + Action::AbortRebase, + ), + ); + } else if self.pending_revert() { + self.queue.push( + InternalEvent::ConfirmAction( + Action::AbortRevert, + ), + ); + } Ok(EventState::Consumed) } else if k == self.key_config.keys.rebase_branch