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

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

[Nguyễn Thái Ngọc Duy]

I find myself often do "git rebase -i xxx" and replace one "pick" line
with "edit" to amend just one commit when I see something I don't like
in that commit. This happens often while cleaning up a series. This
automates the "replace" step so it sends me straight to that commit.

"commit --fixup" then "rebase --autosquash" would work too but I still
need to edit todo file to make it stop after squashing so I can test
that commit. So still extra steps.

Or is there a better way to do this?

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 git-rebase--interactive.sh | 17 ++++++++++++++---
 git-rebase.sh              | 10 ++++++++++
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index d741b04..0988b5c 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -1027,9 +1027,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 "$edit_one"
+then
+	edit_one="`git rev-parse --short $edit_one`"
+	sed "1s/pick $edit_one /edit $edit_one /" "$todo" > "$todo.new" ||
+		die_abort "failed to update todo list"
+	grep "^edit $edit_one " "$todo.new" >/dev/null ||
+		die_abort "expected to find 'edit $edit_one' line but did not"
+	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 1cf8dba..98796cc 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -31,6 +31,7 @@ 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
+1,edit-one!        generate todo list to edit this commit
 committer-date-is-author-date! passed to 'git am'
 ignore-date!       passed to 'git am'
 whitespace=!       passed to 'git apply'
@@ -249,6 +250,10 @@ do
 	-i)
 		interactive_rebase=explicit
 		;;
+	-1)
+		interactive_rebase=explicit
+		edit_one=t
+		;;
 	-k)
 		keep_empty=yes
 		;;
@@ -450,6 +455,11 @@ then
 		;;
 	*)	upstream_name="$1"
 		shift
+		if test -n "$edit_one"
+		then
+			edit_one="$upstream_name"
+			upstream_name="$upstream_name^"
+		fi
 		;;
 	esac
 	upstream=$(peel_committish "${upstream_name}") ||
-- 
1.9.0.66.g14f785a

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

[Matthieu Moy]

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:

> I find myself often do "git rebase -i xxx" and replace one "pick" line
> with "edit" to amend just one commit when I see something I don't like
> in that commit. This happens often while cleaning up a series. This
> automates the "replace" step so it sends me straight to that commit.

Sounds a good idea to me.

>  git-rebase--interactive.sh | 17 ++++++++++++++---
>  git-rebase.sh              | 10 ++++++++++

(obviously, don't forget doc and test if this becomes a non-RFC)

>  has_action "$todo" ||
>  	die_abort "Nothing to do"
> diff --git a/git-rebase.sh b/git-rebase.sh
> index 1cf8dba..98796cc 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -31,6 +31,7 @@ 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
> +1,edit-one!        generate todo list to edit this commit
>  committer-date-is-author-date! passed to 'git am'
>  ignore-date!       passed to 'git am'
>  whitespace=!       passed to 'git apply'
> @@ -249,6 +250,10 @@ do
>  	-i)
>  		interactive_rebase=explicit
>  		;;
> +	-1)
> +		interactive_rebase=explicit
> +		edit_one=t
> +		;;
>  	-k)
>  		keep_empty=yes
>  		;;
> @@ -450,6 +455,11 @@ then
>  		;;
>  	*)	upstream_name="$1"
>  		shift
> +		if test -n "$edit_one"
> +		then
> +			edit_one="$upstream_name"
> +			upstream_name="$upstream_name^"
> +		fi
>  		;;

I think you forgot the case where the user specified -1 but no extra
argument (i.e. use the default to upstream branch). Shouldn't the added
code be after the esac?

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

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

[Jeff King]

On Thu, Feb 27, 2014 at 08:01:18PM +0700, Nguyễn Thái Ngọc Duy wrote:

> I find myself often do "git rebase -i xxx" and replace one "pick" line
> with "edit" to amend just one commit when I see something I don't like
> in that commit. This happens often while cleaning up a series. This
> automates the "replace" step so it sends me straight to that commit.

Yeah, I do this a lot, too.  The interface you propose makes sense to
me, though I'm not sure how much I would use it, as I often do not know
the specifier of the commit I want to change (was it "HEAD~3 or
HEAD~4?"). I guess using ":/" could make that easier.

One comment on the option name:

> +1,edit-one!        generate todo list to edit this commit

I'd expect "-$n" to mean "rebase the last $n commits" (as opposed to
everything not in the upstream). That does not work currently, of
course, but:

  1. It has the potential to confuse people who read it, since it's
     unlike what "-1" means in most of the rest of git.

  2. It closes the door if we want to support "-$n" in the future.

-Peff

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

[Duy Nguyen]

On Fri, Feb 28, 2014 at 1:58 PM, Jeff King <peff@peff.net> wrote:
> On Thu, Feb 27, 2014 at 08:01:18PM +0700, Nguyễn Thái Ngọc Duy wrote:
>
>> I find myself often do "git rebase -i xxx" and replace one "pick" line
>> with "edit" to amend just one commit when I see something I don't like
>> in that commit. This happens often while cleaning up a series. This
>> automates the "replace" step so it sends me straight to that commit.
>
> Yeah, I do this a lot, too.  The interface you propose makes sense to
> me, though I'm not sure how much I would use it, as I often do not know
> the specifier of the commit I want to change (was it "HEAD~3 or
> HEAD~4?"). I guess using ":/" could make that easier.

In my case, I just copy/paste the commit ID from "git log -lp". I
think :/ already works with rebase..

>
> One comment on the option name:
>
>> +1,edit-one!        generate todo list to edit this commit
>
> I'd expect "-$n" to mean "rebase the last $n commits" (as opposed to
> everything not in the upstream). That does not work currently, of
> course, but:
>
>   1. It has the potential to confuse people who read it, since it's
>      unlike what "-1" means in most of the rest of git.
>
>   2. It closes the door if we want to support "-$n" in the future.

I really like to do "git rebase -5" == "git rebase HEAD~5" but never
gotten around do make it so. "-1/--edit-one" was chosen without much
thought. Will change it to something else.
-- 
Duy

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

[Jeff King]

On Fri, Feb 28, 2014 at 02:34:16PM +0700, Duy Nguyen wrote:

> > Yeah, I do this a lot, too.  The interface you propose makes sense to
> > me, though I'm not sure how much I would use it, as I often do not know
> > the specifier of the commit I want to change (was it "HEAD~3 or
> > HEAD~4?"). I guess using ":/" could make that easier.
> 
> In my case, I just copy/paste the commit ID from "git log -lp". I
> think :/ already works with rebase..

I think it should work. I just meant "I will have to get in the habit of
starting to use :/". :)

-Peff

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

[Philip Oakley]

From: "Jeff King" <peff@peff.net>
> I'd expect "-$n" to mean "rebase the last $n commits" (as opposed to
> everything not in the upstream). That does not work currently, of
> course, but:
>
>  1. It has the potential to confuse people who read it, since it's
>     unlike what "-1" means in most of the rest of git.
>
>  2. It closes the door if we want to support "-$n" in the future.
>

Yeah, "rebase the last $n commits" would be a nice touch.

git rebase -i -10 --onto v1.9.0  # rebase the last 10 commits in this 
branch etc.

Philip 

[PATCH 0/3] rebase's convenient options

[Nguyễn Thái Ngọc Duy]

A polished version from the RFC. Now you can do

git rebase -i -10 -> git rebase -i HEAD~10
git rebase -e XYZ -> send you to commit XYZ for editing

Nguyễn Thái Ngọc Duy (3):
  rev-parse: support OPT_NUMBER_CALLBACK in --parseopt
  rebase: accept -<number> as another way of saying HEAD~<number>
  rebase: new convenient option to edit a single commit

 Documentation/git-rebase.txt |  7 +++++++
 builtin/rev-parse.c          |  9 +++++++--
 git-rebase--interactive.sh   | 17 ++++++++++++++---
 git-rebase.sh                | 19 +++++++++++++++++++
 t/t3400-rebase.sh            |  6 ++++++
 5 files changed, 53 insertions(+), 5 deletions(-)

-- 
1.9.0.40.gaa8c3ea

[PATCH 1/3] rev-parse: support OPT_NUMBER_CALLBACK in --parseopt

[Nguyễn Thái Ngọc Duy]

If the option spec is

-NUM Help string

then rev-parse will accept and parse -([0-9]+) and return "-NUM $1"

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/rev-parse.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 45901df..b37676f 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -331,6 +331,8 @@ static int parseopt_dump(const struct option *o, const char *arg, int unset)
 	struct strbuf *parsed = o->value;
 	if (unset)
 		strbuf_addf(parsed, " --no-%s", o->long_name);
+	else if (o->type == OPTION_NUMBER)
+		strbuf_addf(parsed, " -NUM");
 	else if (o->short_name && (o->long_name == NULL || !stuck_long))
 		strbuf_addf(parsed, " -%c", o->short_name);
 	else
@@ -338,7 +340,7 @@ static int parseopt_dump(const struct option *o, const char *arg, int unset)
 	if (arg) {
 		if (!stuck_long)
 			strbuf_addch(parsed, ' ');
-		else if (o->long_name)
+		else if (o->long_name || o->type == OPTION_NUMBER)
 			strbuf_addch(parsed, '=');
 		sq_quote_buf(parsed, arg);
 	}
@@ -439,7 +441,10 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
 
 		if (s - sb.buf == 1) /* short option only */
 			o->short_name = *sb.buf;
-		else if (sb.buf[1] != ',') /* long option only */
+		else if (s - sb.buf == 4 && !strncmp(sb.buf, "-NUM", 4)) {
+			o->type = OPTION_NUMBER;
+			o->flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG;
+		} else if (sb.buf[1] != ',') /* long option only */
 			o->long_name = xmemdupz(sb.buf, s - sb.buf);
 		else {
 			o->short_name = *sb.buf;
-- 
1.9.0.40.gaa8c3ea

Re: [PATCH 1/3] rev-parse: support OPT_NUMBER_CALLBACK in --parseopt

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> If the option spec is
>
> -NUM Help string
>
> then rev-parse will accept and parse -([0-9]+) and return "-NUM $1"

Even though the hardcoded "NUM" token initially gave me a knee-jerk
"Yuck" reaction, that literal option name is very unlikely to be
desired by scripts/commands for their real option names, and being
in all uppercase it is very clear that it is magic convention
between the parsing mechanism and the script it uses.

It however felt funny to me without a matching (possibly hidden)
mechanism to allow parse-options machinery to consume such an output
as its input.  In a script that uses this mechanism to parse out the
numeric option "-NUM 3" out of "git script -3" and uses that "three"
to drive an underlying command (e.g. "git grep -3"), wouldn't it be
more natural if that underlying command can be told to accept the
same notation (i.e. "git grep -NUM 3")?  For that to be consistent
with the rest of the system, "-NUM" would not be a good token; being
it multi-character, it must be "--NUM" or something with double-dash
prefix.

I kind of like the basic idea, the capability it tries to give
scripted Porcelain implementations.  But my impression is that
"rebase -i -4", which this mechanism was invented for, is not
progressing, so perhaps we should wait until the real user of this
feature appears.

Thanks.

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  builtin/rev-parse.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
> index 45901df..b37676f 100644
> --- a/builtin/rev-parse.c
> +++ b/builtin/rev-parse.c
> @@ -331,6 +331,8 @@ static int parseopt_dump(const struct option *o, const char *arg, int unset)
>  	struct strbuf *parsed = o->value;
>  	if (unset)
>  		strbuf_addf(parsed, " --no-%s", o->long_name);
> +	else if (o->type == OPTION_NUMBER)
> +		strbuf_addf(parsed, " -NUM");
>  	else if (o->short_name && (o->long_name == NULL || !stuck_long))
>  		strbuf_addf(parsed, " -%c", o->short_name);
>  	else
> @@ -338,7 +340,7 @@ static int parseopt_dump(const struct option *o, const char *arg, int unset)
>  	if (arg) {
>  		if (!stuck_long)
>  			strbuf_addch(parsed, ' ');
> -		else if (o->long_name)
> +		else if (o->long_name || o->type == OPTION_NUMBER)
>  			strbuf_addch(parsed, '=');
>  		sq_quote_buf(parsed, arg);
>  	}
> @@ -439,7 +441,10 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
>  
>  		if (s - sb.buf == 1) /* short option only */
>  			o->short_name = *sb.buf;
> -		else if (sb.buf[1] != ',') /* long option only */
> +		else if (s - sb.buf == 4 && !strncmp(sb.buf, "-NUM", 4)) {
> +			o->type = OPTION_NUMBER;
> +			o->flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG;
> +		} else if (sb.buf[1] != ',') /* long option only */
>  			o->long_name = xmemdupz(sb.buf, s - sb.buf);
>  		else {
>  			o->short_name = *sb.buf;

[PATCH 2/3] rebase: accept -<number> as another way of saying HEAD~<number>

[Nguyễn Thái Ngọc Duy]

This is "rev-list style", where people can do "git rev-list -3" in
addition to "git rev-list HEAD~3". A lot of commands are driven by the
revision machinery and also accept this form. This addition to rebase
is just for convenience.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-rebase.txt | 3 +++
 git-rebase.sh                | 9 +++++++++
 t/t3400-rebase.sh            | 6 ++++++
 3 files changed, 18 insertions(+)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 2a93c64..52c3561 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -223,6 +223,9 @@ As a special case, you may use "A\...B" as a shortcut for the
 merge base of A and B if there is exactly one merge base. You can
 leave out at most one of A and B, in which case it defaults to HEAD.
 
+-<number>::
+	Specify <upstream> as "HEAD~<number>".
+
 <upstream>::
 	Upstream branch to compare against.  May be any valid commit,
 	not just an existing branch name. Defaults to the configured
diff --git a/git-rebase.sh b/git-rebase.sh
index 5f6732b..33face1 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -43,6 +43,7 @@ continue!          continue
 abort!             abort and check out the original branch
 skip!              skip current patch and continue
 edit-todo!         edit the todo list during an interactive rebase
+-NUM               equivalent of HEAD~<NUM>
 "
 . git-sh-setup
 . git-sh-i18n
@@ -335,6 +336,9 @@ do
 	--gpg-sign=*)
 		gpg_sign_opt="-S${1#--gpg-sign=}"
 		;;
+	-NUM=*)
+		NUM="${1#-NUM=}"
+		;;
 	--)
 		shift
 		break
@@ -342,6 +346,11 @@ do
 	esac
 	shift
 done
+if [ -n "$NUM" ]
+then
+	test $# -gt 0 && usage
+	set -- "$@" "HEAD~$NUM"
+fi
 test $# -gt 2 && usage
 
 if test -n "$cmd" &&
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 6d94b1f..11db7ea 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -215,4 +215,10 @@ test_expect_success 'rebase commit with an ancient timestamp' '
 	grep "author .* 34567 +0600$" actual
 '
 
+test_expect_success 'rebase -<number>' '
+	git reset --hard &&
+	test_must_fail git rebase -2 HEAD^^ &&
+	git rebase -2
+'
+
 test_done
-- 
1.9.0.40.gaa8c3ea

Re: [PATCH 2/3] rebase: accept -<number> as another way of saying HEAD~<number>

[Eric Sunshine]

On Sat, Mar 1, 2014 at 9:53 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> This is "rev-list style", where people can do "git rev-list -3" in
> addition to "git rev-list HEAD~3". A lot of commands are driven by the
> revision machinery and also accept this form. This addition to rebase
> is just for convenience.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/git-rebase.sh b/git-rebase.sh
> index 5f6732b..33face1 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -342,6 +346,11 @@ do
>         esac
>         shift
>  done
> +if [ -n "$NUM" ]

With the exception of one errant "if [...]", git-rebase.sh uniformly
uses "if test ...".

> +then
> +       test $# -gt 0 && usage
> +       set -- "$@" "HEAD~$NUM"
> +fi
>  test $# -gt 2 && usage
>
>  if test -n "$cmd" &&
> diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
> index 6d94b1f..11db7ea 100755
> --- a/t/t3400-rebase.sh
> +++ b/t/t3400-rebase.sh
> @@ -215,4 +215,10 @@ test_expect_success 'rebase commit with an ancient timestamp' '
>         grep "author .* 34567 +0600$" actual
>  '
>
> +test_expect_success 'rebase -<number>' '
> +       git reset --hard &&
> +       test_must_fail git rebase -2 HEAD^^ &&
> +       git rebase -2
> +'
> +
>  test_done
> --
> 1.9.0.40.gaa8c3ea

Re: [PATCH 2/3] rebase: accept -<number> as another way of saying HEAD~<number>

[Duy Nguyen]

On Sun, Mar 2, 2014 at 3:37 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Sat, Mar 1, 2014 at 9:53 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>> This is "rev-list style", where people can do "git rev-list -3" in
>> addition to "git rev-list HEAD~3". A lot of commands are driven by the
>> revision machinery and also accept this form. This addition to rebase
>> is just for convenience.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>> diff --git a/git-rebase.sh b/git-rebase.sh
>> index 5f6732b..33face1 100755
>> --- a/git-rebase.sh
>> +++ b/git-rebase.sh
>> @@ -342,6 +346,11 @@ do
>>         esac
>>         shift
>>  done
>> +if [ -n "$NUM" ]
>
> With the exception of one errant "if [...]", git-rebase.sh uniformly
> uses "if test ...".

Besides that and  three more "if [...]" in git-rebase--interactive.sh,
git-rebase*.sh uses "if test..." only. Will reroll with a cleanup
patch to make them all "if test.."
-- 
Duy

Re: [PATCH 2/3] rebase: accept -<number> as another way of saying HEAD~<number>

[Eric Sunshine]

On Sat, Mar 1, 2014 at 9:53 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> This is "rev-list style", where people can do "git rev-list -3" in
> addition to "git rev-list HEAD~3". A lot of commands are driven by the
> revision machinery and also accept this form. This addition to rebase
> is just for convenience.

I'm seeing some pretty strange results with this. If I use -1, -2, or
-3 then it rebases the expected commits, however, -4 gives me 9
commits, and -5 rebases 35 commits. Am I misunderstanding how this
works?

I'm testing on a branch based on master with these three patches applied.

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  Documentation/git-rebase.txt | 3 +++
>  git-rebase.sh                | 9 +++++++++
>  t/t3400-rebase.sh            | 6 ++++++
>  3 files changed, 18 insertions(+)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 2a93c64..52c3561 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -223,6 +223,9 @@ As a special case, you may use "A\...B" as a shortcut for the
>  merge base of A and B if there is exactly one merge base. You can
>  leave out at most one of A and B, in which case it defaults to HEAD.
>
> +-<number>::
> +       Specify <upstream> as "HEAD~<number>".
> +
>  <upstream>::
>         Upstream branch to compare against.  May be any valid commit,
>         not just an existing branch name. Defaults to the configured
> diff --git a/git-rebase.sh b/git-rebase.sh
> index 5f6732b..33face1 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -43,6 +43,7 @@ continue!          continue
>  abort!             abort and check out the original branch
>  skip!              skip current patch and continue
>  edit-todo!         edit the todo list during an interactive rebase
> +-NUM               equivalent of HEAD~<NUM>
>  "
>  . git-sh-setup
>  . git-sh-i18n
> @@ -335,6 +336,9 @@ do
>         --gpg-sign=*)
>                 gpg_sign_opt="-S${1#--gpg-sign=}"
>                 ;;
> +       -NUM=*)
> +               NUM="${1#-NUM=}"
> +               ;;
>         --)
>                 shift
>                 break
> @@ -342,6 +346,11 @@ do
>         esac
>         shift
>  done
> +if [ -n "$NUM" ]
> +then
> +       test $# -gt 0 && usage
> +       set -- "$@" "HEAD~$NUM"
> +fi
>  test $# -gt 2 && usage
>
>  if test -n "$cmd" &&
> diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
> index 6d94b1f..11db7ea 100755
> --- a/t/t3400-rebase.sh
> +++ b/t/t3400-rebase.sh
> @@ -215,4 +215,10 @@ test_expect_success 'rebase commit with an ancient timestamp' '
>         grep "author .* 34567 +0600$" actual
>  '
>
> +test_expect_success 'rebase -<number>' '
> +       git reset --hard &&
> +       test_must_fail git rebase -2 HEAD^^ &&
> +       git rebase -2
> +'
> +
>  test_done
> --
> 1.9.0.40.gaa8c3ea
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] rebase: accept -<number> as another way of saying HEAD~<number>

[Eric Sunshine]

On Sun, Mar 2, 2014 at 3:53 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Sat, Mar 1, 2014 at 9:53 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>> This is "rev-list style", where people can do "git rev-list -3" in
>> addition to "git rev-list HEAD~3". A lot of commands are driven by the
>> revision machinery and also accept this form. This addition to rebase
>> is just for convenience.
>
> I'm seeing some pretty strange results with this. If I use -1, -2, or
> -3 then it rebases the expected commits, however, -4 gives me 9
> commits, and -5 rebases 35 commits. Am I misunderstanding how this
> works?

Nevermind. I wasn't paying attention to the fact that I was attempting
to rebase merges.

Re: [PATCH 2/3] rebase: accept -<number> as another way of saying HEAD~<number>

[Matthieu Moy]

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Sun, Mar 2, 2014 at 3:53 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> On Sat, Mar 1, 2014 at 9:53 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>>> This is "rev-list style", where people can do "git rev-list -3" in
>>> addition to "git rev-list HEAD~3". A lot of commands are driven by the
>>> revision machinery and also accept this form. This addition to rebase
>>> is just for convenience.
>>
>> I'm seeing some pretty strange results with this. If I use -1, -2, or
>> -3 then it rebases the expected commits, however, -4 gives me 9
>> commits, and -5 rebases 35 commits. Am I misunderstanding how this
>> works?
>
> Nevermind. I wasn't paying attention to the fact that I was attempting
> to rebase merges.

Your remark is actually interesting. Most (all?) Git commands taking
-<n> as parameters act on n commits, regardless of merges.

So, this commit creates an inconsistency between e.g. "git log -3" (show
last 3 commits) and "git rebase -3" (rebase up to HEAD~3, but there may
be more commits in case there are merges).

I don't have a better proposal, but at least the inconsistancy should be
documented (e.g. "note that this is different from what other commands
like 'git log' do when used with a -<number> option since ..." in the
manpage).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH 2/3] rebase: accept -<number> as another way of saying HEAD~<number>

[Michael Haggerty]

On 03/02/2014 04:55 PM, Matthieu Moy wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> 
>> On Sun, Mar 2, 2014 at 3:53 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>>> On Sat, Mar 1, 2014 at 9:53 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>>>> This is "rev-list style", where people can do "git rev-list -3" in
>>>> addition to "git rev-list HEAD~3". A lot of commands are driven by the
>>>> revision machinery and also accept this form. This addition to rebase
>>>> is just for convenience.
>>>
>>> I'm seeing some pretty strange results with this. If I use -1, -2, or
>>> -3 then it rebases the expected commits, however, -4 gives me 9
>>> commits, and -5 rebases 35 commits. Am I misunderstanding how this
>>> works?
>>
>> Nevermind. I wasn't paying attention to the fact that I was attempting
>> to rebase merges.
> 
> Your remark is actually interesting. Most (all?) Git commands taking
> -<n> as parameters act on n commits, regardless of merges.
> 
> So, this commit creates an inconsistency between e.g. "git log -3" (show
> last 3 commits) and "git rebase -3" (rebase up to HEAD~3, but there may
> be more commits in case there are merges).
> 
> I don't have a better proposal, but at least the inconsistancy should be
> documented (e.g. "note that this is different from what other commands
> like 'git log' do when used with a -<number> option since ..." in the
> manpage).

This might be a reason that "-NUM" is a bad idea.

Or perhaps "-NUM" should fail with an error message if any of the last
NUM commits are merges.  In that restricted scenario (which probably
accounts for 99% of rebases), "-NUM" is equivalent to "HEAD~NUM".

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: [PATCH 2/3] rebase: accept -<number> as another way of saying HEAD~<number>

[Matthieu Moy]

Michael Haggerty <mhagger@alum.mit.edu> writes:

> Or perhaps "-NUM" should fail with an error message if any of the last
> NUM commits are merges.  In that restricted scenario (which probably
> accounts for 99% of rebases), "-NUM" is equivalent to "HEAD~NUM".

Makes sense to me. So, -NUM would actually mean "rebase the last NUM
commits" (as well as being an alias for HEAD~NUM), but would fail when
it does not make sense (with an error message explaining the situation
and pointing the user to HEAD~N if this is what he wanted).

This would actually be a feature for me: I often want to rebase "recent
enough" history, and when my @{upstream} isn't well positionned, I
randomly type HEAD~N without remembering what N should be. When N is too
small, the rebase doesn't reach the interesting commit, and when N is
too big, it reaches a merge commit and I get a bunch of commits I'm not
allowed to edit in my todo-list. Then I have to abort the commit
manually. With -N failing on merge commits, the rebase would abort
itself automatically.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH 2/3] rebase: accept -<number> as another way of saying HEAD~<number>

[Duy Nguyen]

On Mon, Mar 3, 2014 at 4:37 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
>
>> Or perhaps "-NUM" should fail with an error message if any of the last
>> NUM commits are merges.  In that restricted scenario (which probably
>> accounts for 99% of rebases), "-NUM" is equivalent to "HEAD~NUM".
>
> Makes sense to me. So, -NUM would actually mean "rebase the last NUM
> commits" (as well as being an alias for HEAD~NUM), but would fail when
> it does not make sense (with an error message explaining the situation
> and pointing the user to HEAD~N if this is what he wanted).

Agreed, but..

> This would actually be a feature for me: I often want to rebase "recent
> enough" history, and when my @{upstream} isn't well positionned, I
> randomly type HEAD~N without remembering what N should be. When N is too
> small, the rebase doesn't reach the interesting commit, and when N is
> too big, it reaches a merge commit and I get a bunch of commits I'm not
> allowed to edit in my todo-list. Then I have to abort the commit
> manually. With -N failing on merge commits, the rebase would abort
> itself automatically.

would "git rebase -i --fork-point" be what you need instead? It's a
new thing, but may be what we actually should use, not this -NUM..
-- 
Duy

Re: [PATCH 2/3] rebase: accept -<number> as another way of saying HEAD~<number>

[David Kastrup]

Duy Nguyen <pclouds@gmail.com> writes:

> On Mon, Mar 3, 2014 at 4:37 PM, Matthieu Moy
> <Matthieu.Moy@grenoble-inp.fr> wrote:
>> Michael Haggerty <mhagger@alum.mit.edu> writes:
>>
>>> Or perhaps "-NUM" should fail with an error message if any of the last
>>> NUM commits are merges.  In that restricted scenario (which probably
>>> accounts for 99% of rebases), "-NUM" is equivalent to "HEAD~NUM".
>>
>> Makes sense to me. So, -NUM would actually mean "rebase the last NUM
>> commits" (as well as being an alias for HEAD~NUM), but would fail when
>> it does not make sense (with an error message explaining the situation
>> and pointing the user to HEAD~N if this is what he wanted).
>
> Agreed, but..
>
>> This would actually be a feature for me: I often want to rebase "recent
>> enough" history, and when my @{upstream} isn't well positionned, I
>> randomly type HEAD~N without remembering what N should be. When N is too
>> small, the rebase doesn't reach the interesting commit, and when N is
>> too big, it reaches a merge commit and I get a bunch of commits I'm not
>> allowed to edit in my todo-list. Then I have to abort the commit
>> manually. With -N failing on merge commits, the rebase would abort
>> itself automatically.
>
> would "git rebase -i --fork-point" be what you need instead? It's a
> new thing, but may be what we actually should use, not this -NUM..

-0 might be a good mnemonic for --fork-point, though.

Of course, when using --preserve-merges explicitly it would appear that
-NUM should not error out.

-- 
David Kastrup

Re: [PATCH 2/3] rebase: accept -<number> as another way of saying HEAD~<number>

[Matthieu Moy]

Duy Nguyen <pclouds@gmail.com> writes:

> On Mon, Mar 3, 2014 at 4:37 PM, Matthieu Moy
>
>> This would actually be a feature for me: I often want to rebase "recent
>> enough" history, and when my @{upstream} isn't well positionned, I
>> randomly type HEAD~N without remembering what N should be. When N is too
>> small, the rebase doesn't reach the interesting commit, and when N is
>> too big, it reaches a merge commit and I get a bunch of commits I'm not
>> allowed to edit in my todo-list. Then I have to abort the commit
>> manually. With -N failing on merge commits, the rebase would abort
>> itself automatically.
>
> would "git rebase -i --fork-point" be what you need instead?

I don't think so. My use case is when I did a manual merge, so
@{upstream} is helpless or even not positionned. There is for sure an
accurate command (remember the branch I just merged and put it on the
command-line), but my fingers prefer typing quick and dirty commands and
hope I won't get too many trial and error cycles ;-).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH 2/3] rebase: accept -<number> as another way of saying HEAD~<number>

[Jeff King]

On Mon, Mar 03, 2014 at 10:37:11AM +0100, Matthieu Moy wrote:

> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
> > Or perhaps "-NUM" should fail with an error message if any of the last
> > NUM commits are merges.  In that restricted scenario (which probably
> > accounts for 99% of rebases), "-NUM" is equivalent to "HEAD~NUM".
> 
> Makes sense to me. So, -NUM would actually mean "rebase the last NUM
> commits" (as well as being an alias for HEAD~NUM), but would fail when
> it does not make sense (with an error message explaining the situation
> and pointing the user to HEAD~N if this is what he wanted).

Yeah, I agree with this. I think "-NUM" is only useful for linear
string-of-pearls history. With merges, it either means:

  - linearize history (a la git-log), go back NUM commits, and treat the
    result as the upstream. This is useless, because it's difficult to
    predict where you're going to end up. Using explicit "~" and "^"
    relative-commit operators to specify the upstream is more flexible
    if you really want to do this (but I don't know why you would).

  - do not use an UNINTERESTING upstream as the cutoff, but instead
    try to rebase exactly NUM commits. In the face of non-linear
    history, I'm not even sure what this would mean, or how we would
    implement it.

Neither of those is useful, so I don't think erroring out on them is a
bad thing. And by erroring out (rather than documenting some weird and
useless behavior), we don't have to worry about backwards compatibility
if we later _do_ come up with a useful behavior (the best I can think of
is that "--first-parent -3" might have a use, but I think that should
already be covered, as the results of --first-parent are by-definition
linear).

> This would actually be a feature for me: I often want to rebase "recent
> enough" history, and when my @{upstream} isn't well positionned, I
> randomly type HEAD~N without remembering what N should be. When N is too
> small, the rebase doesn't reach the interesting commit, and when N is
> too big, it reaches a merge commit and I get a bunch of commits I'm not
> allowed to edit in my todo-list. Then I have to abort the commit
> manually. With -N failing on merge commits, the rebase would abort
> itself automatically.

I do the same thing.

-Peff

Re: [PATCH 2/3] rebase: accept -<number> as another way of saying HEAD~<number>

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Michael Haggerty <mhagger@alum.mit.edu> writes:
>
>> Or perhaps "-NUM" should fail with an error message if any of the last
>> NUM commits are merges.  In that restricted scenario (which probably
>> accounts for 99% of rebases), "-NUM" is equivalent to "HEAD~NUM".
>
> Makes sense to me. So, -NUM would actually mean "rebase the last NUM
> commits" (as well as being an alias for HEAD~NUM), but would fail when
> it does not make sense (with an error message explaining the situation
> and pointing the user to HEAD~N if this is what he wanted).
>
> This would actually be a feature for me: I often want to rebase "recent
> enough" history, and when my @{upstream} isn't well positionned,...

Could you elaborate on this a bit?  What does "isn't well
positioned" mean?  Do you mean "the upstream has advanced but there
is no reason for my topic to build on that---I'd rather want to make
sure I can view 'diff @{1} HEAD' and understand what my changes
before the rebase was"?  That is, what you really want is

	git rebase -i --onto $(git merge-base @{upstream} HEAD) @{upstream}

but that is too long to type?

If it is very common (and I suspect it is), we may want to support
such a short-hand---the above does not make any sense without '-i',
but I would say with '-i' you do not want to reBASE on an updated
base most of the time.  "git rebase -i @{upstream}...HEAD" or
something?

Re: [PATCH 2/3] rebase: accept -<number> as another way of saying HEAD~<number>

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> Michael Haggerty <mhagger@alum.mit.edu> writes:
>>
>>> Or perhaps "-NUM" should fail with an error message if any of the last
>>> NUM commits are merges.  In that restricted scenario (which probably
>>> accounts for 99% of rebases), "-NUM" is equivalent to "HEAD~NUM".
>>
>> Makes sense to me. So, -NUM would actually mean "rebase the last NUM
>> commits" (as well as being an alias for HEAD~NUM), but would fail when
>> it does not make sense (with an error message explaining the situation
>> and pointing the user to HEAD~N if this is what he wanted).
>>
>> This would actually be a feature for me: I often want to rebase "recent
>> enough" history, and when my @{upstream} isn't well positionned,...
>
> Could you elaborate on this a bit?  What does "isn't well
> positioned" mean?

The most common case is when @{upstream} is not positionned at all,
because I'm on a temporary branch on which I did not configure it. The
other case is when I did a manual merge of a branch other than upstream.

As I said in another message, there would probably be cleaner solutions,
but the trial and error one does not work that bad.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH 2/3] rebase: accept -<number> as another way of saying HEAD~<number>

[Junio C Hamano]

Michael Haggerty <mhagger@alum.mit.edu> writes:

> This might be a reason that "-NUM" is a bad idea.
>
> Or perhaps "-NUM" should fail with an error message if any of the last
> NUM commits are merges.  In that restricted scenario (which probably
> accounts for 99% of rebases), "-NUM" is equivalent to "HEAD~NUM".

That sounds like one possible way out; the opposite would be to
enable mode that preserges merges when we see any.

If "rebase" had a "--first-parent" mode that simply replays only the
commits on the first parent chain, merging the same second and later
parents without looking at their history when dealing with merge
commits, and always used that mode unless "--flatten-history" was
given, the world might have been a better place.  We cannot go there
in a single step, but we could plan such a backward incompatible
migration if we wanted to.

[PATCH 3/3] rebase: new convenient option to edit a single commit

[Nguyễn Thái Ngọc Duy]

"git rebase -e XYZ" is basically the same as

EDITOR="sed -i '1s/pick XYZ/edit XYZ/' $@" \
git rebase -i XYZ^

In English, it prepares the todo list for you to edit only commit XYZ
to save your time. The time saving is only significant when you edit a
lot of commits separately.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-rebase.txt |  4 ++++
 git-rebase--interactive.sh   | 17 ++++++++++++++---
 git-rebase.sh                | 10 ++++++++++
 3 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 52c3561..b8c263d 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -359,6 +359,10 @@ 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::
+--edit-one::
+	Prepare the todo list to edit only the commit at <upstream>
+
 -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..4762d57 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 "$edit_one"
+then
+	edit_one="`git rev-parse --short $edit_one`"
+	sed "1s/pick $edit_one /edit $edit_one /" "$todo" > "$todo.new" ||
+		die_abort "failed to update todo list"
+	grep "^edit $edit_one " "$todo.new" >/dev/null ||
+		die_abort "expected to find 'edit $edit_one' line but did not"
+	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 33face1..b8b6aa9 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -32,6 +32,7 @@ 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-one!        generate todo list to edit this commit
 committer-date-is-author-date! passed to 'git am'
 ignore-date!       passed to 'git am'
 whitespace=!       passed to 'git apply'
@@ -339,6 +340,10 @@ do
 	-NUM=*)
 		NUM="${1#-NUM=}"
 		;;
+	--edit-one)
+		interactive_rebase=explicit
+		edit_one=t
+		;;
 	--)
 		shift
 		break
@@ -463,6 +468,11 @@ then
 		;;
 	*)	upstream_name="$1"
 		shift
+		if test -n "$edit_one"
+		then
+			edit_one="$upstream_name"
+			upstream_name="$upstream_name^"
+		fi
 		;;
 	esac
 	upstream=$(peel_committish "${upstream_name}") ||
-- 
1.9.0.40.gaa8c3ea

Re: [PATCH 3/3] rebase: new convenient option to edit a single commit

[Eric Sunshine]

On Sat, Mar 1, 2014 at 9:53 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> "git rebase -e XYZ" is basically the same as
>
> EDITOR="sed -i '1s/pick XYZ/edit XYZ/' $@" \
> git rebase -i XYZ^
>
> In English, it prepares the todo list for you to edit only commit XYZ
> to save your time. The time saving is only significant when you edit a
> lot of commits separately.

Should this accept multiple -e arguments? Based upon the above
justification, it sounds like it should, and I think that would be the
intuitive expectation (at least for me).

The current implementation, however, is broken with multiple -e arguments. With:

    git rebase -i -e older -e newer

'newer' is ignored entirely. However, with:

    git rebase -i -e newer -e older

it errors out when rewriting the todo list:

    "expected to find 'edit older' line but did not"

An implementation supporting multiple -e arguments would need to
ensure that the todo list extends to the "oldest" rev specified by any
-e argument.

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  Documentation/git-rebase.txt |  4 ++++
>  git-rebase--interactive.sh   | 17 ++++++++++++++---
>  git-rebase.sh                | 10 ++++++++++
>  3 files changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 52c3561..b8c263d 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -359,6 +359,10 @@ 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::
> +--edit-one::
> +       Prepare the todo list to edit only the commit at <upstream>
> +
>  -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..4762d57 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 "$edit_one"
> +then
> +       edit_one="`git rev-parse --short $edit_one`"
> +       sed "1s/pick $edit_one /edit $edit_one /" "$todo" > "$todo.new" ||
> +               die_abort "failed to update todo list"
> +       grep "^edit $edit_one " "$todo.new" >/dev/null ||
> +               die_abort "expected to find 'edit $edit_one' line but did not"
> +       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 33face1..b8b6aa9 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -32,6 +32,7 @@ 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-one!        generate todo list to edit this commit
>  committer-date-is-author-date! passed to 'git am'
>  ignore-date!       passed to 'git am'
>  whitespace=!       passed to 'git apply'
> @@ -339,6 +340,10 @@ do
>         -NUM=*)
>                 NUM="${1#-NUM=}"
>                 ;;
> +       --edit-one)
> +               interactive_rebase=explicit
> +               edit_one=t
> +               ;;
>         --)
>                 shift
>                 break
> @@ -463,6 +468,11 @@ then
>                 ;;
>         *)      upstream_name="$1"
>                 shift
> +               if test -n "$edit_one"
> +               then
> +                       edit_one="$upstream_name"
> +                       upstream_name="$upstream_name^"
> +               fi
>                 ;;
>         esac
>         upstream=$(peel_committish "${upstream_name}") ||
> --
> 1.9.0.40.gaa8c3ea
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] rebase: new convenient option to edit a single commit

[Eric Sunshine]

On Sun, Mar 2, 2014 at 4:04 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Sat, Mar 1, 2014 at 9:53 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>> "git rebase -e XYZ" is basically the same as
>>
>> EDITOR="sed -i '1s/pick XYZ/edit XYZ/' $@" \
>> git rebase -i XYZ^
>>
>> In English, it prepares the todo list for you to edit only commit XYZ
>> to save your time. The time saving is only significant when you edit a
>> lot of commits separately.
>
> Should this accept multiple -e arguments? Based upon the above
> justification, it sounds like it should, and I think that would be the
> intuitive expectation (at least for me).
>
> The current implementation, however, is broken with multiple -e arguments. With:
>
>     git rebase -i -e older -e newer
>
> 'newer' is ignored entirely. However, with:
>
>     git rebase -i -e newer -e older
>
> it errors out when rewriting the todo list:
>
>     "expected to find 'edit older' line but did not"
>
> An implementation supporting multiple -e arguments would need to
> ensure that the todo list extends to the "oldest" rev specified by any
> -e argument.

Of course, I'm misreading and abusing the intention of -e as if it is
"-e <arg>".

Re: [PATCH 3/3] rebase: new convenient option to edit a single commit

[Michael Haggerty]

On 03/02/2014 10:09 AM, Eric Sunshine wrote:
> On Sun, Mar 2, 2014 at 4:04 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> On Sat, Mar 1, 2014 at 9:53 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>>> "git rebase -e XYZ" is basically the same as
>>>
>>> EDITOR="sed -i '1s/pick XYZ/edit XYZ/' $@" \
>>> git rebase -i XYZ^
>>>
>>> In English, it prepares the todo list for you to edit only commit XYZ
>>> to save your time. The time saving is only significant when you edit a
>>> lot of commits separately.
>>
>> Should this accept multiple -e arguments? Based upon the above
>> justification, it sounds like it should, and I think that would be the
>> intuitive expectation (at least for me).
>>
>> The current implementation, however, is broken with multiple -e arguments. With:
>>
>>     git rebase -i -e older -e newer
>>
>> 'newer' is ignored entirely. However, with:
>>
>>     git rebase -i -e newer -e older
>>
>> it errors out when rewriting the todo list:
>>
>>     "expected to find 'edit older' line but did not"
>>
>> An implementation supporting multiple -e arguments would need to
>> ensure that the todo list extends to the "oldest" rev specified by any
>> -e argument.
> 
> Of course, I'm misreading and abusing the intention of -e as if it is
> "-e <arg>".

I think that your misreading is more consistent than the feature as
implemented.

    git rebase -e OLDER

does not mean "do 'git rebase -i OLDER' and oh, by the way, also set up
commit OLDER to be edited".  It means "do 'git rebase -i OLDER^' ..."
(note: "OLDER^" and not "OLDER").  So it is confusing to think as "-e"
as a modifier on an otherwise normal "git rebase -i" invocation.
Rather, it seems to me that "-e" and "-i" should be mutually exclusive
(and consider it an implementation detail that the former is implemented
using the latter).

And if that is our point of view, then is perfectly logical to allow it
to be specified multiple times.  OTOH there is no reason that v1 has to
allow multiple "-e", as long as it properly rejects that usage.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: [PATCH 3/3] rebase: new convenient option to edit a single commit

[Duy Nguyen]

On Mon, Mar 3, 2014 at 5:10 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> On 03/02/2014 10:09 AM, Eric Sunshine wrote:
>> On Sun, Mar 2, 2014 at 4:04 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>>> On Sat, Mar 1, 2014 at 9:53 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>>>> "git rebase -e XYZ" is basically the same as
>>>>
>>>> EDITOR="sed -i '1s/pick XYZ/edit XYZ/' $@" \
>>>> git rebase -i XYZ^
>>>>
>>>> In English, it prepares the todo list for you to edit only commit XYZ
>>>> to save your time. The time saving is only significant when you edit a
>>>> lot of commits separately.
>>>
>>> Should this accept multiple -e arguments? Based upon the above
>>> justification, it sounds like it should, and I think that would be the
>>> intuitive expectation (at least for me).
>>>
>>> The current implementation, however, is broken with multiple -e arguments. With:
>>>
>>>     git rebase -i -e older -e newer
>>>
>>> 'newer' is ignored entirely. However, with:
>>>
>>>     git rebase -i -e newer -e older
>>>
>>> it errors out when rewriting the todo list:
>>>
>>>     "expected to find 'edit older' line but did not"
>>>
>>> An implementation supporting multiple -e arguments would need to
>>> ensure that the todo list extends to the "oldest" rev specified by any
>>> -e argument.
>>
>> Of course, I'm misreading and abusing the intention of -e as if it is
>> "-e <arg>".
>
> I think that your misreading is more consistent than the feature as
> implemented.
>
>     git rebase -e OLDER
>
> does not mean "do 'git rebase -i OLDER' and oh, by the way, also set up
> commit OLDER to be edited".  It means "do 'git rebase -i OLDER^' ..."
> (note: "OLDER^" and not "OLDER").  So it is confusing to think as "-e"
> as a modifier on an otherwise normal "git rebase -i" invocation.
> Rather, it seems to me that "-e" and "-i" should be mutually exclusive
> (and consider it an implementation detail that the former is implemented
> using the latter).
>
> And if that is our point of view, then is perfectly logical to allow it
> to be specified multiple times.

Logically, yes. Practically, no. If you have to put multiple -e and
some hashes in one line, wouldn't editing to-do list in your favorite
editor be faster?

> OTOH there is no reason that v1 has to
> allow multiple "-e", as long as it properly rejects that usage.
-- 
Duy

Re: [PATCH 3/3] rebase: new convenient option to edit a single commit

[David Kastrup]

Duy Nguyen <pclouds@gmail.com> writes:

> Logically, yes. Practically, no. If you have to put multiple -e and
> some hashes in one line, wouldn't editing to-do list in your favorite
> editor be faster?

An editor is the last resort when the card puncher is broken.

-- 
David Kastrup

Re: [PATCH 3/3] rebase: new convenient option to edit a single commit

[Eric Sunshine]

On Sat, Mar 1, 2014 at 9:53 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> "git rebase -e XYZ" is basically the same as
>
> EDITOR="sed -i '1s/pick XYZ/edit XYZ/' $@" \
> git rebase -i XYZ^
>
> In English, it prepares the todo list for you to edit only commit XYZ
> to save your time. The time saving is only significant when you edit a
> lot of commits separately.

Is it correct to single out only "edit" for special treatment? If
allowing "edit" on the command-line, then shouldn't command-line
"reword" also be supported? I, for one, often need to reword a commit
message (or two or three); far more frequently than I need to edit a
commit.

(This is a genuine question about perceived favoritism of "edit", as
opposed to a request to further bloat the interface.)

Re: [PATCH 3/3] rebase: new convenient option to edit a single commit

[Duy Nguyen]

On Tue, Mar 4, 2014 at 3:28 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Sat, Mar 1, 2014 at 9:53 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>> "git rebase -e XYZ" is basically the same as
>>
>> EDITOR="sed -i '1s/pick XYZ/edit XYZ/' $@" \
>> git rebase -i XYZ^
>>
>> In English, it prepares the todo list for you to edit only commit XYZ
>> to save your time. The time saving is only significant when you edit a
>> lot of commits separately.
>
> Is it correct to single out only "edit" for special treatment? If
> allowing "edit" on the command-line, then shouldn't command-line
> "reword" also be supported? I, for one, often need to reword a commit
> message (or two or three); far more frequently than I need to edit a
> commit.
>
> (This is a genuine question about perceived favoritism of "edit", as
> opposed to a request to further bloat the interface.)

Heh I had the same thought yesterday. The same thing could be asked
for "git commit --fixup" to send us back to the fixed up commit so we
can do something about it. If we go along that line, then "git commit"
may be a better interface to reword older commits..
-- 
Duy

Re: [PATCH 3/3] rebase: new convenient option to edit a single commit

[Michael Haggerty]

On 03/04/2014 03:08 AM, Duy Nguyen wrote:
> On Tue, Mar 4, 2014 at 3:28 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> On Sat, Mar 1, 2014 at 9:53 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>>> "git rebase -e XYZ" is basically the same as
>>>
>>> EDITOR="sed -i '1s/pick XYZ/edit XYZ/' $@" \
>>> git rebase -i XYZ^
>>>
>>> In English, it prepares the todo list for you to edit only commit XYZ
>>> to save your time. The time saving is only significant when you edit a
>>> lot of commits separately.
>>
>> Is it correct to single out only "edit" for special treatment? If
>> allowing "edit" on the command-line, then shouldn't command-line
>> "reword" also be supported? I, for one, often need to reword a commit
>> message (or two or three); far more frequently than I need to edit a
>> commit.
>>
>> (This is a genuine question about perceived favoritism of "edit", as
>> opposed to a request to further bloat the interface.)
> 
> Heh I had the same thought yesterday. The same thing could be asked
> for "git commit --fixup" to send us back to the fixed up commit so we
> can do something about it. If we go along that line, then "git commit"
> may be a better interface to reword older commits..

I disagree.  "git commit --fixup" doesn't rewrite history.  It just adds
a new commit with a special commit message that will make it easier to
rewrite history later.  I think it would be prudent to keep the
history-rewriting functionality segregated in "git rebase", which users
already know they have to use with care [1].

But the next question is whether "git rebase" should have shortcuts for
*most* of its line commands.  All of the following seem to make sense:

    git rebase --edit COMMIT

        A long-form for the -e option we have been talking about.
        It is unfortunately that this spelling sounds like the
        "--edit" option on "git commit --edit" and "git merge --edit",
        so people might use it when they really mean
        "git rebase --reword COMMIT".

    git rebase --reword COMMIT
    git rebase --fixup COMMIT
    git rebase --squash COMMIT

    git rebase --kill COMMIT

        Remove the commit from history, like running "git rebase
        --interactive" then deleting that line.

I'm quite confident that I would use all of these commands.

Moreover, it would logically be reasonable to allow multiple of these
options, at least as long as they have distinct COMMIT arguments.
Though, as Duy points out, it might in practice be easier to edit the
todo list in an editor rather than trying to do multiple "edits" at a
time via the command line.

Some thought would have to go into the question of if/how these commands
should interact with "git rebase --autosquash" (which, don't forget, can
also be requested via rebase.autosquash).

Michael

[1] OK, granted, there is "git commit --amend", which rewrites history
too.  But it rewrites only the last commit, which is less likely to be
problematic.

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: [PATCH 3/3] rebase: new convenient option to edit a single commit

[Duy Nguyen]

On Tue, Mar 4, 2014 at 3:59 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>> On Tue, Mar 4, 2014 at 3:28 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>>> Is it correct to single out only "edit" for special treatment? If
>>> allowing "edit" on the command-line, then shouldn't command-line
>>> "reword" also be supported? I, for one, often need to reword a commit
>>> message (or two or three); far more frequently than I need to edit a
>>> commit.
>>>
>>> (This is a genuine question about perceived favoritism of "edit", as
>>> opposed to a request to further bloat the interface.)
>>
>> Heh I had the same thought yesterday. The same thing could be asked
>> for "git commit --fixup" to send us back to the fixed up commit so we
>> can do something about it. If we go along that line, then "git commit"
>> may be a better interface to reword older commits..
>
> I disagree.  "git commit --fixup" doesn't rewrite history.  It just adds
> a new commit with a special commit message that will make it easier to
> rewrite history later.  I think it would be prudent to keep the
> history-rewriting functionality segregated in "git rebase", which users
> already know they have to use with care [1].

Just to be clear I didn't mean to modify --fixup behavior. It could be
--amend-old-commit or something like that. It's actually --amend that
made me want to put the UI in "git commit". But it's a bad idea
(besides what you pointed out) because after you're done, you still
need to do "git rebase --continue".

> But the next question is whether "git rebase" should have shortcuts for
> *most* of its line commands.  All of the following seem to make sense:
>
>     git rebase --edit COMMIT
>
>         A long-form for the -e option we have been talking about.
>         It is unfortunately that this spelling sounds like the
>         "--edit" option on "git commit --edit" and "git merge --edit",
>         so people might use it when they really mean
>         "git rebase --reword COMMIT".
>
>     git rebase --reword COMMIT

Sounds good.

>     git rebase --fixup COMMIT
>     git rebase --squash COMMIT

This is not interactive (except when merge conflicts occur), is it?

A bit off topic. I sometimes want to fix up a commit and make it stop
there for me to test it again but there is no such command, is there?
Maybe we could add support for "fixup/edit" (or "fe" for short) and
"squash/edit" ("se"). Not really familiar with the code base to do
that myself quickly though.

>     git rebase --kill COMMIT
>
>         Remove the commit from history, like running "git rebase
>         --interactive" then deleting that line.

Yes! Done this sometimes (not so often) but a definitely nice thing to
have. I'd go with --remove or --delete though.
-- 
Duy

Re: [PATCH 3/3] rebase: new convenient option to edit a single commit

[Michael Haggerty]

On 03/04/2014 11:24 AM, Duy Nguyen wrote:
> On Tue, Mar 4, 2014 at 3:59 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>>     git rebase --fixup COMMIT
>>     git rebase --squash COMMIT
> 
> This is not interactive (except when merge conflicts occur), is it?

--fixup would not be interactive (is that a problem?), but --squash does
open an editor to allow you to merge the commit messages.

> A bit off topic. I sometimes want to fix up a commit and make it stop
> there for me to test it again but there is no such command, is there?
> Maybe we could add support for "fixup/edit" (or "fe" for short) and
> "squash/edit" ("se"). Not really familiar with the code base to do
> that myself quickly though.

Maybe we should allow "edit" to appear on a line by itself, without a
SHA-1, in which case it would stop after all previous lines had been
processed.  Then you could change one line to "fixup" or "squash", and
then add a blank "edit" line after it.  Though there is no really
obvious way to do this using the hypothetical new command line options
that we have been discussing.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: [PATCH 3/3] rebase: new convenient option to edit a single commit

[Junio C Hamano]

Michael Haggerty <mhagger@alum.mit.edu> writes:

> ...  All of the following seem to make sense:
>
>     git rebase --edit COMMIT
>
>         A long-form for the -e option we have been talking about.
>         It is unfortunately that this spelling sounds like the
>         "--edit" option on "git commit --edit" and "git merge --edit",
>         so people might use it when they really mean
>         "git rebase --reword COMMIT".

I agree, so the "--edit" does *not* make sense as it invites confusion.

>     git rebase --reword COMMIT

Yes, that would make sense.

>     git rebase --fixup COMMIT
>     git rebase --squash COMMIT

I am not sure about these.  What does it even mean to "--fixup" (or
"--squash" for that matter) a single commit without specifying what
it is squashed into?  Or are you assuming that all of these is only
to affect pre-populated rebase-i insn sheet that is to be further
edited before the actual rebasing starts?  I somehow had an impression
that the reason to have these new options is to skip the editing of
the insn sheet in the editor altogether.

>     git rebase --kill COMMIT

This _does_ make sense under my assumption: "remove this commit from
the insn-sheet and go ahead with it, without bothering me to edit
the insn-sheet further".

> I'm quite confident that I would use all of these commands.

If "--kill" takes only one, I would probably do "rebase --onto"
without bothering with "-i" at all, but if it lets me drop multiple
non-consecutive commits, by accepting more than one "--kill", I see
how I would be using it myself.  I can see how "--reword"/"--amend"
would be useful even when dealing with only a single commit.

I do not know about --fixup/--squash though.

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

[Nguyễn Thái Ngọc Duy]

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?

 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.
+
 -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" ||
+		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"
+	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
 		;;
 	--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#--*=}"
+		;;
 	--)
 		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"
 then
 	case "$#" in
 	0)
-- 
1.9.0.40.gaa8c3ea

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

[Matthieu Moy]

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:

>  Documentation/git-rebase.txt | 11 +++++++++++
>  git-rebase--interactive.sh   | 17 ++++++++++++++---
>  git-rebase.sh                | 22 +++++++++++++++++++++-
>  3 files changed, 46 insertions(+), 4 deletions(-)

This would deserve a few tests ...

>  'git rebase' --continue | --skip | --abort | --edit-todo
> +'git rebase' [--edit | -E | --reword | -R | --delete | -D ] <commit-ish>
[...]
> +-E=<commit>::
> +--edit=<commit>::
> +-R=<commit>::
> +--reword=<commit>::
> +-D=<commit>::
> +--delete=<commit>::

I first thought the two versions were inconsistant, but they're
technically correct since the current patch allows only one instance of
the option. I find it a bit weird to have two different descriptions,
but that may be just me.

> +	Configuration variable `rebase.autostash` is
> +	ignored.

I first found it really strange, and then understood it's a typo.

s/autostash/autosquash/

;-)

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

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

[Michael Haggerty]

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 <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.

>  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.

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

> +	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"?

>  		;;
>  	--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".

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

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

[Matthieu Moy]

Michael Haggerty <mhagger@alum.mit.edu> writes:

>> @@ -290,6 +294,7 @@ do
>>  		;;
>>  	--autostash)
>>  		autostash=true
>> +		explicit_autosquash=t
>
> Should that be "explicit_autostash"?

My guess is: no, but it should be below the --autoquash case, not this
one.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/