Re: [PATCH v3] gitk: Allow commit editing
On Thu, 25 Aug 2011, Jeff King wrote:
Show 28 quoted lines
> On Fri, Aug 19, 2011 at 02:25:53PM +0200, Michal Sojka wrote:
>
> > + if {[exec git branch -r --contains=$rowmenuid] ne {}} {
> > + if {![confirm_popup [mc "The commit you are going to edit appears in at least one\
> > + remote branch. It is a bad idea to change a branch that is\
> > + possibly used by other people. See git-rebase(1) for details.\n\n\
> > + Do you want to continue?"]]} return }
>
> We frown on using porcelain like "git branch" here, because it was not
> meant to be scriptable (i.e., its behavior may change in future
> releases).
>
> As I mentioned elsewhere, I think you really want to see whether the
> commit is referenced by any other ref, not just branches; if it is, then
> the rebase is potentially problematic. You can do that with something
> like:
>
> us=`git symbolic-ref HEAD`
> git for-each-ref --format='%(refname)' |
> while read ref; do
> test "$ref" = $us" && continue
> echo "^$ref"
> done |
> git rev-list HEAD --stdin
>
> That will give a list of commits found only on HEAD and nowhere else
> (i.e., those which are safe to rebase). If your commit is among them,
> then it's safe.I implemented something like you ontlined above (see below). Instead of rev-listing HEAD, I use the commit id to be edited. This way I don't have to find the commit in the returned list, but I only check whether the list is empty or not.
Additionally, besides skiping HEAD ref, I also skip refs/stash. Rebasing before stash should not (I hope) cause any problems and therefore it is not necessary to warn the user.
Show 7 quoted lines
> Speaking of which, notice that I used HEAD here. What happens with your
> patch if I do:
>
> $ git checkout foo
> $ gitk bar
>
> and select a commit to edit that is not in "foo"?
I added a check for this. If this is detected, error message is displayed and no edit is possible.
The other changes in this version are:
- added --no-autosquash to rebase invocation
- fixed indexes of menu entries following the new edit entry.
Regards, -Michal
--8<---------------cut here---------------start------------->8--- I often use gitk to review patches before pushing them out and I would like to have an easy way to edit commits. The current approach with copying the commitid, switching to terminal, invoking git rebase -i, editing the commit and switching back to gitk is a way too complicated just for changing a single letter in the commit message or removing a debug printf().
This patch adds "Edit this commit" item to gitk's context menu which invokes interactive rebase in a non-interactive way :-). git gui is used to actually edit the commit.
Besides editing the commit message, splitting of commits, as described in git-rebase(1), is also supported.
The user is warned if the commit to be edited is contained in another ref besides the current branch and the stash (e.g. in a remote branch). Additionally, error box is displayed if the user attempts to edit a commit not contained in the current branch.
Signed-off-by: Michal Sojka <sojka@os.inf.tu-dresden.de>
---
gitk-git/gitk | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
1 files changed, 67 insertions(+), 4 deletions(-)
Show changes to gitk-git/gitk +67 −4
diff --git a/gitk-git/gitk b/gitk-git/gitk
index 4cde0c4..0968efa 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -2487,6 +2487,7 @@ proc makewindow {} {
makemenu $rowctxmenu {
{mc "Diff this -> selected" command {diffvssel 0}}
{mc "Diff selected -> this" command {diffvssel 1}}
+ {mc "Edit this commit" command edit_commit}
{mc "Make patch" command mkpatch}
{mc "Create tag" command mktag}
{mc "Write commit to file" command writecommit}
@@ -8428,18 +8429,18 @@ proc rowmenu {x y id} {
if {$id ne $nullid && $id ne $nullid2} {
set menu $rowctxmenu
if {$mainhead ne {}} {
- $menu entryconfigure 7 -label [mc "Reset %s branch to here" $mainhead] -state normal
+ $menu entryconfigure 8 -label [mc "Reset %s branch to here" $mainhead] -state normal
} else {
- $menu entryconfigure 7 -label [mc "Detached head: can't reset" $mainhead] -state disabled
+ $menu entryconfigure 8 -label [mc "Detached head: can't reset" $mainhead] -state disabled
}
if {[info exists markedid] && $markedid ne $id} {
- $menu entryconfigure 9 -state normal
$menu entryconfigure 10 -state normal
$menu entryconfigure 11 -state normal
+ $menu entryconfigure 12 -state normal
} else {
- $menu entryconfigure 9 -state disabled
$menu entryconfigure 10 -state disabled
$menu entryconfigure 11 -state disabled
+ $menu entryconfigure 12 -state disabled
}
} else {
set menu $fakerowmenu
@@ -9102,6 +9103,68 @@ proc cherrypick {} {
notbusy cherrypick
}
+proc rebase_ok {id} {
+ if {[exec git merge-base $id HEAD] ne $id} {
+ error_popup [mc "You cannot edit commits outside of the current branch."]
+ return 0
+ }
+ set headref [exec git symbolic-ref HEAD]
+ set allrefs [split [exec git for-each-ref --format=%(refname)] "\n"]
+ set otherrefs {}
+ set otherrefsneg ""
+ foreach ref $allrefs {
+ if {$ref eq "refs/stash" || $ref eq $headref} continue
+ set otherrefsneg "$otherrefsneg^$ref\n"
+ }
+
+ # Check whether some other refs contain the commit to be edited
+ if {[exec git rev-list --stdin $id << $otherrefsneg] eq {}} {
+ if { [confirm_popup [mc "The commit you are going to edit is contained in another, possibly non-local, ref (e.g. branch or tag).\
+ It is a bad idea to change a ref that is possibly used by other people. See git-rebase(1) for details.\n\n\
+ Do you want to continue?"]]} {
+ return 1
+ } else {
+ return 0
+ }
+ }
+ return 1
+}
+
+proc edit_commit {} {
+ global rowmenuid selectedline
+
+ if {![rebase_ok $rowmenuid]} return
+
+ nowbusy edit [mc "Editing commit"]
+ if {[catch {exec sh -c "(GIT_EDITOR='sed -ie 1s/^pick/edit/' git rebase -p -i --no-autosquash $rowmenuid^ && git gui citool --amend) 2>&1"} err]} {
+ notbusy edit
+ error_popup $err
+ exec git rebase --abort
+ return
+ }
+ set newcommit [exec git rev-parse HEAD]
+ while {[catch {exec sh -c "git diff-index --quiet --cached HEAD && git diff-files --quiet"} err]} {
+ if {[confirm_popup [mc "There are uncommited changes in the working tree or in the index.\
+ Do you want to create a new commit (OK) or discard them (Cancel)?"]]} {
+ catch {exec git gui citool} err;
+ # In case of error (i.e. the user did not commit anything), we just ask him again
+ } else {
+ exec git reset --hard
+ }
+ }
+ if {[catch {exec sh -c "git rebase --continue 2>&1"} err]} {
+ notbusy edit
+ error_popup $err
+ exec git rebase --abort
+ return
+ }
+ updatecommits
+ # XXX How to select the edited commit? This doesn't work.
+ selbyid $newcommit
+ notbusy edit
+}
+
+
proc resethead {} {
global mainhead rowmenuid confirm_ok resettype NS
--
1.7.5.4
--8<---------------cut here---------------end--------------->8---
Michal Sojka
