Support reword in log tab & refactor external editor calls #312

WizardOhio24 · 2020-10-07T22:53:14Z

No more do we have to go though an interactive rebase in which the exact commit reference must be known, where we then must choose 'reword' on the correct commit.

This PR provides the reword functionality on the log(commit list) tab, simply press 'r' on the commit, type in the new message and press enter to change it.

This does not do an interactive rebase in git2rs, rather a slightly more complex rebase operation which achieves the same thing (there is no such thing as an interactive rebase in git2rs/libgit2)
This does not work on the first commit because it has no parent.

Linked to #32

extrawurst

Thanks for looking into this!❤️

asyncgit/src/sync/rebase.rs

src/components/reword.rs

WizardOhio24 · 2020-10-10T22:12:56Z

This does work, but it doesn't keep it's commit selection prior to the change, it just goes to the top.

WizardOhio24 · 2020-10-10T22:31:07Z

Fixed

asyncgit/src/sync/rebase.rs

src/components/reword.rs

asyncgit/src/sync/rebase.rs

src/app.rs

WizardOhio24 · 2021-04-29T23:05:19Z

Updated to master.

extrawurst · 2021-05-17T22:26:13Z

superseded by #672

WizardOhio24 · 2021-05-18T01:20:35Z

This has nothing to do with #672? This is about rewording commits in the log tab, nothing to do with searching.
Perhaps you meant to close #506? (Feel free, it has been superceded).

extrawurst · 2021-05-18T10:08:33Z

was too late, of course I meant to close #506

@WizardOhio24 for this to be reviewed it needs to be up to date

extrawurst

I worked through all of this again and it cannot be merged:

this happens right when I simply open the commit popup, go to external editor and close it again. this is currently broken.

most of the external editor stuff can be most like reverted anyway if you use the Mode in the commit popup. actually the entire reword-popup can now be handled inside CommitComponent, this would be much easier I think

extrawurst · 2021-05-27T20:15:42Z

src/app.rs

-/// the main app type
+/// Used to determine where the user should
+/// be put when the external editor is closed
+pub enum EditorSource {


should be put into the externaleditor.rs

extrawurst · 2021-05-27T20:16:45Z

src/app.rs

@@ -64,10 +72,10 @@ pub struct App {
    theme: SharedTheme,
    key_config: SharedKeyConfig,
    input: Input,
+    external_editor: Option<(Option<String>, EditorSource)>,


this should be a struct instead of tuple to help express the intent of this. something like OpenEditorState

extrawurst · 2021-05-27T20:21:13Z

src/app.rs

@@ -528,14 +553,23 @@ impl App {
                    .insert(NeedsUpdate::ALL | NeedsUpdate::COMMANDS);
            }
            InternalEvent::Update(u) => flags.insert(u),
-            InternalEvent::OpenCommit => self.commit.show()?,
+            InternalEvent::OpenCommit => {
+                self.external_editor =


I am not sure why we have to do this now?
doesn't this trigger show_editor now every time we just open the regular commit popup?

extrawurst · 2021-05-27T20:23:40Z

src/components/externaleditor.rs

@@ -187,3 +193,34 @@ impl Component for ExternalEditorComponent {
        Ok(())
    }
 }
+
+pub fn show_editor(with_text: Option<&String>) -> Result<String> {
+    let temp_file = NamedTempFile::new()?;


by now we intentionally use a specific filename to edit commit msgs in so that vim knows how to highlight it, we should keep doing this (see COMMIT_EDITMSG)

extrawurst · 2021-05-27T20:24:18Z

src/queue.rs

@@ -69,7 +72,7 @@ pub enum InternalEvent {
    ///
    SelectBranch,
    ///
-    OpenExternalEditor(Option<String>),
+    OpenExternalEditor(Option<String>, EditorSource),


use a struct like mentioned above

extrawurst · 2021-05-27T20:29:45Z

asyncgit/src/sync/rebase.rs

+/// 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


what does that mean? what does it imply?

extrawurst · 2021-05-27T20:30:29Z

asyncgit/src/sync/rebase.rs

+    let sig = signature_allow_undefined_name(&repo)?;
+
+    let parent_commit_oid = if let Ok(parent_commit) =
+        repo.find_commit(commit_oid)?.parent(0)


what if the commit is a merge commit with multiple parents? please add a unittest for that case

extrawurst · 2021-05-27T20:34:42Z

asyncgit/src/sync/rebase.rs

+    };
+
+    // If we are on a branch
+    if let Ok(Some(branch)) = get_cur_branch(&repo) {


lets map this into the Err we want and ? it here to early out if not on a branch, this makes the rest indent one less and easier to read

extrawurst · 2021-05-27T20:36:15Z

asyncgit/src/sync/rebase.rs

+        File::create(&root.join(file_path))?.write_all(b"a")?;
+        stage_add_file(repo_path, file_path).unwrap();
+        commit(repo_path, "commit1").unwrap();


the new write_commit_file makes creating/writing/committing test files more convenient/easier to read now

extrawurst · 2021-05-27T20:37:23Z

asyncgit/src/sync/rebase.rs

+    use std::{fs::File, io::Write, path::Path};
+
+    #[test]
+    fn test_reword() -> Result<()> {


we currently only test the happy path, what if the rename does not go through and the rebase has to be rolled back. we need a test for that aswell

extrawurst · 2021-05-27T20:42:18Z

going back to draft in the meantime. change it back to review once you think its ready 👍

extrawurst · 2023-02-18T20:27:05Z

closing in favour of #1553

extrawurst requested changes Oct 8, 2020

View reviewed changes

WizardOhio24 force-pushed the reword branch from 5e13e45 to c642654 Compare October 10, 2020 22:07

WizardOhio24 changed the title ~~WIP: Support reword in log tab~~ Support reword in log tab Oct 10, 2020

extrawurst requested changes Oct 25, 2020

View reviewed changes

asyncgit/src/sync/rebase.rs Outdated Show resolved Hide resolved

asyncgit/src/sync/rebase.rs Outdated Show resolved Hide resolved

src/components/reword.rs Show resolved Hide resolved

WizardOhio24 force-pushed the reword branch 2 times, most recently from c6c10b3 to d7676d4 Compare October 26, 2020 22:53

WizardOhio24 requested a review from extrawurst October 26, 2020 23:02

extrawurst requested changes Oct 27, 2020

View reviewed changes

WizardOhio24 mentioned this pull request Nov 4, 2020

allow interactive amend commit #222

Closed

WizardOhio24 and others added 19 commits November 5, 2020 18:50

Initial attempt

5aef9c8

Create reword_safe

823b065

Remove Rebase Struct

4378ecc

reword text fill with previous commit message

7e6cf14

Remove unnessessary comment

c44b64b

Fix comment typo

b4ee5bc

Remove unused head

34854d8

Fix clippy errors

6232181

reword_safe gets current branch

1a981fd

Fix vim test

0c8dc2a

Modify reword function

5d7e0f6

Make reword private

269297a

remove except in reword component open function

a79ca04

Remove reword from sync mod

1b51822

Keep previous selection when rewording commit

b2fcc30

fix clippy

7f5effb

Use Utf8Error for reword if invalid utf8

2b27d20

Add reword_safe test

7bba5d5

Allow opening external editor to edit reword

fbbb1ac

WizardOhio24 changed the title ~~Support reword in log tab & refactor external editor~~ Support reword in log tab & refactor external editor calls Feb 6, 2021

extrawurst force-pushed the master branch 2 times, most recently from 81a8849 to f84f6f4 Compare March 3, 2021 21:27

WizardOhio24 added 9 commits April 29, 2021 22:08

Merge branch 'master' into reword

b70a768

Fix keys

5905942

Remove mod select_branch

9b78391

Remove branch

d82a6b3

Some Fixes

5e18d25

Fix build

258cc76

Small Fixes

24bed95

Remove unnessessary comments

f3ecf33

Fix clippy errors

a8ca289

extrawurst closed this May 17, 2021

extrawurst reopened this May 18, 2021

WizardOhio24 added 4 commits May 23, 2021 11:33

Merge branch 'master' into reword

5933353

Fix merge problems

1107a72

Merge branch 'master' into reword

e140e98

Fix merge problems

ff4afb2

WizardOhio24 mentioned this pull request May 25, 2021

Squash commits in commit log #403

Draft

extrawurst requested changes May 27, 2021

View reviewed changes

extrawurst marked this pull request as draft May 27, 2021 20:42

extrawurst force-pushed the master branch from 3bca9d4 to 8905fd2 Compare September 1, 2021 21:24

extrawurst mentioned this pull request Feb 18, 2023

Create Fixup Commits #829

Closed

extrawurst closed this Feb 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support reword in log tab & refactor external editor calls #312

Support reword in log tab & refactor external editor calls #312

WizardOhio24 commented Oct 7, 2020

extrawurst left a comment

WizardOhio24 commented Oct 10, 2020

WizardOhio24 commented Oct 10, 2020

WizardOhio24 commented Apr 29, 2021

extrawurst commented May 17, 2021

WizardOhio24 commented May 18, 2021

extrawurst commented May 18, 2021 •

edited

Loading

extrawurst left a comment

extrawurst May 27, 2021

extrawurst May 27, 2021

extrawurst May 27, 2021

extrawurst May 27, 2021

extrawurst May 27, 2021

extrawurst May 27, 2021

extrawurst May 27, 2021

extrawurst May 27, 2021

extrawurst May 27, 2021

extrawurst May 27, 2021

extrawurst commented May 27, 2021

extrawurst commented Feb 18, 2023

Support reword in log tab & refactor external editor calls #312

Support reword in log tab & refactor external editor calls #312

Conversation

WizardOhio24 commented Oct 7, 2020

extrawurst left a comment

Choose a reason for hiding this comment

WizardOhio24 commented Oct 10, 2020

WizardOhio24 commented Oct 10, 2020

WizardOhio24 commented Apr 29, 2021

extrawurst commented May 17, 2021

WizardOhio24 commented May 18, 2021

extrawurst commented May 18, 2021 • edited Loading

extrawurst left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

extrawurst commented May 27, 2021

extrawurst commented Feb 18, 2023

extrawurst commented May 18, 2021 •

edited

Loading