git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH/RFC] rebase: new convenient option to edit/reword/delete a single commit

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Mar 10, 2014, 08:30 UTC
Message-ID
<531D7820.1090403@alum.mit.edu>
In-Reply-To
<1394333354-16257-1-git-send-email-pclouds@gmail.com>
On 03/09/2014 03:49 AM, Nguyễn Thái Ngọc Duy wrote:
Show 10 quoted lines
> Prepare the todo list for you to edit/reword/delete the given commit.
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  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.

Show 30 quoted lines
>  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.
Show 19 quoted lines
> +
>  -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.

Show 37 quoted lines
> +	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"?
Show 14 quoted lines
>  		;;
>  	--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".

Show 24 quoted lines
>  	--)
>  		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/
Previous: Matthieu MoyNext: Matthieu Moy
Message 39 of 40 in “rebase: new convenient option to edit a single commit”
  1. rebase: new convenient option to edit a single commitNguyễn Thái Ngọc Duy, Feb 27, 2014
  2. Matthieu MoyFeb 27, 2014
  3. Jeff KingFeb 28, 2014
  4. Duy NguyenFeb 28, 2014
  5. Jeff KingFeb 28, 2014
  6. Philip OakleyFeb 28, 2014
  7. 0/3 rebase's convenient optionsNguyễn Thái Ngọc Duy, Mar 2, 2014
  8. 1/3 rev-parse: support OPT_NUMBER_CALLBACK in --parseoptNguyễn Thái Ngọc Duy, Mar 2, 2014
  9. Junio C HamanoMar 4, 2014
  10. 2/3 rebase: accept -<number> as another way of saying HEAD~<number>Nguyễn Thái Ngọc Duy, Mar 2, 2014
  11. Eric SunshineMar 2, 2014
  12. Duy NguyenMar 2, 2014
  13. Eric SunshineMar 2, 2014
  14. Eric SunshineMar 2, 2014
  15. Matthieu MoyMar 2, 2014
  16. Michael HaggertyMar 3, 2014
  17. Matthieu MoyMar 3, 2014
  18. Duy NguyenMar 3, 2014
  19. David KastrupMar 3, 2014
  20. Matthieu MoyMar 3, 2014
  21. Jeff KingMar 3, 2014
  22. Junio C HamanoMar 3, 2014
  23. Matthieu MoyMar 3, 2014
  24. Junio C HamanoMar 3, 2014
  25. 3/3 rebase: new convenient option to edit a single commitNguyễn Thái Ngọc Duy, Mar 2, 2014
  26. Eric SunshineMar 2, 2014
  27. Eric SunshineMar 2, 2014
  28. Michael HaggertyMar 3, 2014
  29. Duy NguyenMar 3, 2014
  30. David KastrupMar 3, 2014
  31. Eric SunshineMar 3, 2014
  32. Duy NguyenMar 4, 2014
  33. Michael HaggertyMar 4, 2014
  34. Duy NguyenMar 4, 2014
  35. Michael HaggertyMar 4, 2014
  36. Junio C HamanoMar 4, 2014
  37. rebase: new convenient option to edit/reword/delete a single commitNguyễn Thái Ngọc Duy, Mar 9, 2014
  38. Matthieu MoyMar 9, 2014
  39. Michael HaggertyMar 10, 2014
  40. Matthieu MoyMar 10, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.