Skip to content

Reword commit (#829) #1553

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Feb 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
8 changes: 8 additions & 0 deletions asyncgit/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

///
Expand Down
2 changes: 2 additions & 0 deletions asyncgit/src/sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ mod rebase;
pub mod remotes;
mod repository;
mod reset;
mod reword;
mod staging;
mod stash;
mod state;
Expand Down Expand Up @@ -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,
Expand Down
170 changes: 170 additions & 0 deletions asyncgit/src/sync/reword.rs
Original file line number Diff line number Diff line change
@@ -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
);
}
}
3 changes: 3 additions & 0 deletions src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()?;
Expand Down
130 changes: 82 additions & 48 deletions src/components/commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -42,6 +42,7 @@ enum Mode {
Amend(CommitId),
Merge(Vec<CommitId>),
Revert,
Reword(CommitId),
}

pub struct CommitComponent {
Expand Down Expand Up @@ -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(())
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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(())
}
}
Loading