Re: [PATCH/RFC] rebase: new convenient option to edit/reword/delete a single commit
From: Michael Haggerty <hidden>
Date: 2016-06-15 23:00:15
On 03/09/2014 03:49 AM, Nguyễn Thái Ngọc Duy wrote:
Prepare the todo list for you to edit/reword/delete the given commit. Signed-off-by: Nguyễn Thái Ngọc Duy <redacted> --- Allowing multiple actions is a bit too much for my shell skills. I don't really need it so I won't push it, but if somebody gives me a sketch, I'll try to polish it. --squash and --fixup would require two commits, making it a bit awkward in option handling. Or is the fixup/squash always HEAD?
These commands always squash/fixup the indicated commit with the previous one. I think the same approach that you use below should work for these commands, too.
quoted hunk ↗ jump to hunk
Documentation/git-rebase.txt | 11 +++++++++++ git-rebase--interactive.sh | 17 ++++++++++++++--- git-rebase.sh | 22 +++++++++++++++++++++- 3 files changed, 46 insertions(+), 4 deletions(-)diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 2a93c64..becb749 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt@@ -13,6 +13,7 @@ SYNOPSIS 'git rebase' [-i | --interactive] [options] [--exec <cmd>] [--onto <newbase>] --root [<branch>] 'git rebase' --continue | --skip | --abort | --edit-todo +'git rebase' [--edit | -E | --reword | -R | --delete | -D ] <commit-ish> DESCRIPTION -----------@@ -356,6 +357,16 @@ unless the `--fork-point` option is specified. user edit that list before rebasing. This mode can also be used to split commits (see SPLITTING COMMITS below). +-E=<commit>:: +--edit=<commit>:: +-R=<commit>:: +--reword=<commit>:: +-D=<commit>:: +--delete=<commit>:: + Prepare the todo list to edit or reword or delete the + specified commit. Configuration variable `rebase.autostash` is + ignored.
If I understand correctly, when one of these options is used, the editor
is not presented to the user at all. If so, then it is probably
confusing to emphasize "the todo list", because the user will never see
it. How about
Edit, reword, or delete the specified commit, replaying subsequent
commits on top of it (like running `git rebase --interactive
commit^` and then changing the command on the line containing
commit). If conflicts arise when replaying the later commits,
resolve them and run "git rebase --continue", as usual. The
configuration variable `rebase.autosquash` is ignored when these
options are used.
quoted hunk ↗ jump to hunk
+ -p:: --preserve-merges:: Instead of ignoring merges, try to recreate them.diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index a1adae8..3ded4c7 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh@@ -1040,9 +1040,20 @@ fi has_action "$todo" || die_abort "Nothing to do" -cp "$todo" "$todo".backup -git_sequence_editor "$todo" || - die_abort "Could not execute editor" +if test -n "$one_action" +then + commit="`git rev-parse --short $one_commit`" + sed "1s/pick $commit /$one_action $commit /" "$todo" > "$todo.new" ||
It wouldn't hurt to anchor this pattern at the beginning of the line. I understand that it wouldn't help, either (assuming everything else is working right), but it makes the intention clearer.
+ die_abort "failed to update todo list" + grep "^$one_action $commit " "$todo.new" >/dev/null || + die_abort "expected to find '$one_action $commit' line but did not"
The die_aborts above is really an internal consistency check, right? If so, maybe it should start with "internal error:" so that the user doesn't think that he has done something wrong.
quoted hunk ↗ jump to hunk
+ mv "$todo.new" "$todo" || + die_abort "failed to update todo list" +else + cp "$todo" "$todo".backup + git_sequence_editor "$todo" || + die_abort "Could not execute editor" +fi has_action "$todo" || die_abort "Nothing to do"diff --git a/git-rebase.sh b/git-rebase.sh index 5f6732b..2acffb4 100755 --- a/git-rebase.sh +++ b/git-rebase.sh@@ -32,6 +32,9 @@ verify allow pre-rebase hook to run rerere-autoupdate allow rerere to update index with resolved conflicts root! rebase all reachable commits up to the root(s) autosquash move commits that begin with squash!/fixup! under -i +E,edit=! generate todo list to edit this commit +R,reword=! generate todo list to reword this commit's message +D,delete=! generate todo list to delete this commit committer-date-is-author-date! passed to 'git am' ignore-date! passed to 'git am' whitespace=! passed to 'git apply'@@ -228,6 +231,7 @@ then fi test -n "$type" && in_progress=t +one_action= total_argc=$# while test $# != 0 do@@ -290,6 +294,7 @@ do ;; --autostash) autostash=true + explicit_autosquash=t
Should that be "explicit_autostash"?
quoted hunk ↗ jump to hunk
;; --verbose) verbose=t@@ -335,6 +340,13 @@ do --gpg-sign=*) gpg_sign_opt="-S${1#--gpg-sign=}" ;; + --edit=*|--reword=*|--delete=*) + test -n "$one_action" && die "$(gettext "--edit, --reword or --delete cannot be used multiple times")" + interactive_rebase=explicit + one_action="${1%=*}" + one_action="${one_action#--}" + one_commit="${1#--*=}" + ;;
Is "delete" a valid todo-list command? I would have thought that you would change the command to "#pick" in the case of "--delete".
quoted hunk ↗ jump to hunk
--) shift break@@ -342,6 +354,7 @@ do esac shift done +test -n "$one_action" && test $# -gt 0 && usage test $# -gt 2 && usage if test -n "$cmd" &&@@ -438,7 +451,14 @@ else state_dir="$apply_dir" fi -if test -z "$rebase_root" +if test -n "$one_action" +then + upstream_name="$one_commit^" + upstream=$(peel_committish "${upstream_name}") || + die "$(eval_gettext "invalid upstream \$upstream_name")" + upstream_arg="$upstream_name" + test -n "$explicit_autosquash" || autosquash= +elif test -z "$rebase_root"
It would be nice if these options (though not --squash and --fixup) would support editing the root commit. The logic would be similar to the code in the "else" branch of this "if" chain.
then case "$#" in 0)
Cheers, Michael -- Michael Haggerty mhagger@alum.mit.edu http://softwareswirl.blogspot.com/