Skip to content

Commit 9d83ce3

Browse files
authored
Reword commit (#1553)
* reuse commit popup for reword * switch to status after reword * show command * prepopulate with old msg * changelog Closes #829
1 parent ab01fc7 commit 9d83ce3

File tree

10 files changed

+303
-48
lines changed

10 files changed

+303
-48
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2222
* allow reset (soft,mixed,hard) from commit log ([#1500](https://github.com/extrawurst/gitui/issues/1500))
2323
* allow `copy` file path on revision files and status tree [[@yanganto]](https://github.com/yanganto) ([#1516](https://github.com/extrawurst/gitui/pull/1516))
2424
* print message of where log will be written if `-l` is set ([#1472](https://github.com/extrawurst/gitui/pull/1472))
25+
* support **reword** of commit from log ([#829](https://github.com/extrawurst/gitui/pull/829))
2526

2627
### Fixes
2728
* commit msg history ordered the wrong way ([#1445](https://github.com/extrawurst/gitui/issues/1445))

asyncgit/src/error.rs

+8
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,14 @@ pub enum Error {
7676
///
7777
#[error("path string error")]
7878
PathString,
79+
80+
///
81+
#[error("no parent of commit found")]
82+
NoParent,
83+
84+
///
85+
#[error("not on a branch")]
86+
NoBranch,
7987
}
8088

8189
///

asyncgit/src/sync/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ mod rebase;
2323
pub mod remotes;
2424
mod repository;
2525
mod reset;
26+
mod reword;
2627
mod staging;
2728
mod stash;
2829
mod state;
@@ -76,6 +77,7 @@ pub use remotes::{
7677
pub(crate) use repository::repo;
7778
pub use repository::{RepoPath, RepoPathRef};
7879
pub use reset::{reset_repo, reset_stage, reset_workdir};
80+
pub use reword::reword;
7981
pub use staging::{discard_lines, stage_lines};
8082
pub use stash::{
8183
get_stashes, stash_apply, stash_drop, stash_pop, stash_save,

asyncgit/src/sync/reword.rs

+170
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
use git2::{Oid, RebaseOptions, Repository};
2+
3+
use super::{
4+
commit::signature_allow_undefined_name,
5+
repo,
6+
utils::{bytes2string, get_head_refname},
7+
CommitId, RepoPath,
8+
};
9+
use crate::error::{Error, Result};
10+
11+
/// This is the same as reword, but will abort and fix the repo if something goes wrong
12+
pub fn reword(
13+
repo_path: &RepoPath,
14+
commit: CommitId,
15+
message: &str,
16+
) -> Result<CommitId> {
17+
let repo = repo(repo_path)?;
18+
let cur_branch_ref = get_head_refname(&repo)?;
19+
20+
match reword_internal(&repo, commit.get_oid(), message) {
21+
Ok(id) => Ok(id.into()),
22+
// Something went wrong, checkout the previous branch then error
23+
Err(e) => {
24+
if let Ok(mut rebase) = repo.open_rebase(None) {
25+
rebase.abort()?;
26+
repo.set_head(&cur_branch_ref)?;
27+
repo.checkout_head(None)?;
28+
}
29+
Err(e)
30+
}
31+
}
32+
}
33+
34+
/// Gets the current branch the user is on.
35+
/// Returns none if they are not on a branch
36+
/// and Err if there was a problem finding the branch
37+
fn get_current_branch(
38+
repo: &Repository,
39+
) -> Result<Option<git2::Branch>> {
40+
for b in repo.branches(None)? {
41+
let branch = b?.0;
42+
if branch.is_head() {
43+
return Ok(Some(branch));
44+
}
45+
}
46+
Ok(None)
47+
}
48+
49+
/// Changes the commit message of a commit with a specified oid
50+
///
51+
/// While this function is most commonly associated with doing a
52+
/// reword opperation in an interactive rebase, that is not how it
53+
/// is implemented in git2rs
54+
///
55+
/// This is dangerous if it errors, as the head will be detached so this should
56+
/// always be wrapped by another function which aborts the rebase if something goes wrong
57+
fn reword_internal(
58+
repo: &Repository,
59+
commit: Oid,
60+
message: &str,
61+
) -> Result<Oid> {
62+
let sig = signature_allow_undefined_name(repo)?;
63+
64+
let parent_commit_oid = repo
65+
.find_commit(commit)?
66+
.parent(0)
67+
.map_or(None, |parent_commit| Some(parent_commit.id()));
68+
69+
let commit_to_change = if let Some(pc_oid) = parent_commit_oid {
70+
// Need to start at one previous to the commit, so
71+
// first rebase.next() points to the actual commit we want to change
72+
repo.find_annotated_commit(pc_oid)?
73+
} else {
74+
return Err(Error::NoParent);
75+
};
76+
77+
// If we are on a branch
78+
if let Ok(Some(branch)) = get_current_branch(repo) {
79+
let cur_branch_ref = bytes2string(branch.get().name_bytes())?;
80+
let cur_branch_name = bytes2string(branch.name_bytes()?)?;
81+
let top_branch_commit = repo.find_annotated_commit(
82+
branch.get().peel_to_commit()?.id(),
83+
)?;
84+
85+
let mut rebase = repo.rebase(
86+
Some(&top_branch_commit),
87+
Some(&commit_to_change),
88+
None,
89+
Some(&mut RebaseOptions::default()),
90+
)?;
91+
92+
let mut target;
93+
94+
rebase.next();
95+
if parent_commit_oid.is_none() {
96+
return Err(Error::NoParent);
97+
}
98+
target = rebase.commit(None, &sig, Some(message))?;
99+
let reworded_commit = target;
100+
101+
// Set target to top commit, don't know when the rebase will end
102+
// so have to loop till end
103+
while rebase.next().is_some() {
104+
target = rebase.commit(None, &sig, None)?;
105+
}
106+
rebase.finish(None)?;
107+
108+
// Now override the previous branch
109+
repo.branch(
110+
&cur_branch_name,
111+
&repo.find_commit(target)?,
112+
true,
113+
)?;
114+
115+
// Reset the head back to the branch then checkout head
116+
repo.set_head(&cur_branch_ref)?;
117+
repo.checkout_head(None)?;
118+
return Ok(reworded_commit);
119+
}
120+
// Repo is not on a branch, possibly detached head
121+
Err(Error::NoBranch)
122+
}
123+
124+
#[cfg(test)]
125+
mod tests {
126+
use super::*;
127+
use crate::sync::{
128+
get_commit_info,
129+
tests::{repo_init_empty, write_commit_file},
130+
};
131+
use pretty_assertions::assert_eq;
132+
133+
#[test]
134+
fn test_reword() {
135+
let (_td, repo) = repo_init_empty().unwrap();
136+
let root = repo.path().parent().unwrap();
137+
138+
let repo_path: &RepoPath =
139+
&root.as_os_str().to_str().unwrap().into();
140+
141+
write_commit_file(&repo, "foo", "a", "commit1");
142+
143+
let oid2 = write_commit_file(&repo, "foo", "ab", "commit2");
144+
145+
let branch =
146+
repo.branches(None).unwrap().next().unwrap().unwrap().0;
147+
let branch_ref = branch.get();
148+
let commit_ref = branch_ref.peel_to_commit().unwrap();
149+
let message = commit_ref.message().unwrap();
150+
151+
assert_eq!(message, "commit2");
152+
153+
let reworded =
154+
reword(repo_path, oid2.into(), "NewCommitMessage")
155+
.unwrap();
156+
157+
// Need to get the branch again as top oid has changed
158+
let branch =
159+
repo.branches(None).unwrap().next().unwrap().unwrap().0;
160+
let branch_ref = branch.get();
161+
let commit_ref_new = branch_ref.peel_to_commit().unwrap();
162+
let message_new = commit_ref_new.message().unwrap();
163+
assert_eq!(message_new, "NewCommitMessage");
164+
165+
assert_eq!(
166+
message_new,
167+
get_commit_info(repo_path, &reworded).unwrap().message
168+
);
169+
}
170+
}

src/app.rs

+3
Original file line numberDiff line numberDiff line change
@@ -815,6 +815,9 @@ impl App {
815815
}
816816
InternalEvent::Update(u) => flags.insert(u),
817817
InternalEvent::OpenCommit => self.commit.show()?,
818+
InternalEvent::RewordCommit(id) => {
819+
self.commit.open(Some(id))?;
820+
}
818821
InternalEvent::PopupStashing(opts) => {
819822
self.stashmsg_popup.options(opts);
820823
self.stashmsg_popup.show()?;

src/components/commit.rs

+82-48
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use crate::{
1010
strings, try_or_popup,
1111
ui::style::SharedTheme,
1212
};
13-
use anyhow::Result;
13+
use anyhow::{bail, Ok, Result};
1414
use asyncgit::{
1515
cached, message_prettify,
1616
sync::{
@@ -42,6 +42,7 @@ enum Mode {
4242
Amend(CommitId),
4343
Merge(Vec<CommitId>),
4444
Revert,
45+
Reword(CommitId),
4546
}
4647

4748
pub struct CommitComponent {
@@ -289,6 +290,13 @@ impl CommitComponent {
289290
Mode::Revert => {
290291
sync::commit_revert(&self.repo.borrow(), msg)?
291292
}
293+
Mode::Reword(id) => {
294+
let commit =
295+
sync::reword(&self.repo.borrow(), *id, msg)?;
296+
self.queue.push(InternalEvent::TabSwitchStatus);
297+
298+
commit
299+
}
292300
};
293301
Ok(())
294302
}
@@ -332,6 +340,78 @@ impl CommitComponent {
332340
fn toggle_verify(&mut self) {
333341
self.verify = !self.verify;
334342
}
343+
344+
pub fn open(&mut self, reword: Option<CommitId>) -> Result<()> {
345+
//only clear text if it was not a normal commit dlg before, so to preserve old commit msg that was edited
346+
if !matches!(self.mode, Mode::Normal) {
347+
self.input.clear();
348+
}
349+
350+
self.mode = Mode::Normal;
351+
352+
let repo_state = sync::repo_state(&self.repo.borrow())?;
353+
354+
self.mode =
355+
if repo_state != RepoState::Clean && reword.is_some() {
356+
bail!("cannot reword while repo is not in a clean state");
357+
} else if let Some(reword_id) = reword {
358+
self.input.set_text(
359+
sync::get_commit_details(
360+
&self.repo.borrow(),
361+
reword_id,
362+
)?
363+
.message
364+
.unwrap_or_default()
365+
.combine(),
366+
);
367+
self.input.set_title(strings::commit_reword_title());
368+
Mode::Reword(reword_id)
369+
} else {
370+
match repo_state {
371+
RepoState::Merge => {
372+
let ids =
373+
sync::mergehead_ids(&self.repo.borrow())?;
374+
self.input
375+
.set_title(strings::commit_title_merge());
376+
self.input.set_text(sync::merge_msg(
377+
&self.repo.borrow(),
378+
)?);
379+
Mode::Merge(ids)
380+
}
381+
RepoState::Revert => {
382+
self.input
383+
.set_title(strings::commit_title_revert());
384+
self.input.set_text(sync::merge_msg(
385+
&self.repo.borrow(),
386+
)?);
387+
Mode::Revert
388+
}
389+
390+
_ => {
391+
self.commit_template = get_config_string(
392+
&self.repo.borrow(),
393+
"commit.template",
394+
)
395+
.ok()
396+
.flatten()
397+
.and_then(|path| read_to_string(path).ok());
398+
399+
if self.is_empty() {
400+
if let Some(s) = &self.commit_template {
401+
self.input.set_text(s.clone());
402+
}
403+
}
404+
self.input.set_title(strings::commit_title());
405+
Mode::Normal
406+
}
407+
}
408+
};
409+
410+
self.commit_msg_history_idx = 0;
411+
self.input.show()?;
412+
413+
Ok(())
414+
}
335415
}
336416

337417
impl DrawableComponent for CommitComponent {
@@ -466,53 +546,7 @@ impl Component for CommitComponent {
466546
}
467547

468548
fn show(&mut self) -> Result<()> {
469-
//only clear text if it was not a normal commit dlg before, so to preserve old commit msg that was edited
470-
if !matches!(self.mode, Mode::Normal) {
471-
self.input.clear();
472-
}
473-
474-
self.mode = Mode::Normal;
475-
476-
let repo_state = sync::repo_state(&self.repo.borrow())?;
477-
478-
self.mode = match repo_state {
479-
RepoState::Merge => {
480-
let ids = sync::mergehead_ids(&self.repo.borrow())?;
481-
self.input.set_title(strings::commit_title_merge());
482-
self.input
483-
.set_text(sync::merge_msg(&self.repo.borrow())?);
484-
Mode::Merge(ids)
485-
}
486-
RepoState::Revert => {
487-
self.input.set_title(strings::commit_title_revert());
488-
self.input
489-
.set_text(sync::merge_msg(&self.repo.borrow())?);
490-
Mode::Revert
491-
}
492-
493-
_ => {
494-
self.commit_template = get_config_string(
495-
&self.repo.borrow(),
496-
"commit.template",
497-
)
498-
.ok()
499-
.flatten()
500-
.and_then(|path| read_to_string(path).ok());
501-
502-
if self.is_empty() {
503-
if let Some(s) = &self.commit_template {
504-
self.input.set_text(s.clone());
505-
}
506-
}
507-
508-
self.input.set_title(strings::commit_title());
509-
Mode::Normal
510-
}
511-
};
512-
513-
self.commit_msg_history_idx = 0;
514-
self.input.show()?;
515-
549+
self.open(None)?;
516550
Ok(())
517551
}
518552
}

0 commit comments

Comments
 (0)