Re: [PATCH 3/3] rebase: new convenient option to edit a single commit
Michael Haggerty 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@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
On 03/04/2014 11:24 AM, Duy Nguyen wrote: > On Tue, Mar 4, 2014 at 3:59 PM, Michael Haggerty 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 mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@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
On Tue, Mar 4, 2014 at 3:59 PM, Michael Haggerty wrote: >> On Tue, Mar 4, 2014 at 3:28 AM, Eric Sunshine >> 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@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
On 03/04/2014 03:08 AM, Duy Nguyen wrote: > On Tue, Mar 4, 2014 at 3:28 AM, Eric Sunshine wrote: >> On Sat, Mar 1, 2014 at 9:53 PM, Nguyễn Thái Ngọc Duy >> 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 mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@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
On Tue, Mar 4, 2014 at 3:28 AM, Eric Sunshine wrote: > On Sat, Mar 1, 2014 at 9:53 PM, Nguyễn Thái Ngọc Duy > 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@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
On Sat, Mar 1, 2014 at 9:53 PM, Nguyễn Thái Ngọc Duy 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.) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@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
Duy Nguyen 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@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
On Mon, Mar 3, 2014 at 5:10 PM, Michael Haggerty wrote: > On 03/02/2014 10:09 AM, Eric Sunshine wrote: >> On Sun, Mar 2, 2014 at 4:04 AM, Eric Sunshine >> wrote: >>> On Sat, Mar 1, 2014 at 9:53 PM, Nguyễn Thái Ngọc Duy >>> 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 ". > > 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@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
On 03/02/2014 10:09 AM, Eric Sunshine wrote: > On Sun, Mar 2, 2014 at 4:04 AM, Eric Sunshine wrote: >> On Sat, Mar 1, 2014 at 9:53 PM, Nguyễn Thái Ngọc Duy >> 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 ". 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 mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@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
On Sun, Mar 2, 2014 at 4:04 AM, Eric Sunshine wrote: > On Sat, Mar 1, 2014 at 9:53 PM, Nguyễn Thái Ngọc Duy > 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 ". -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@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
On Sat, Mar 1, 2014 at 9:53 PM, Nguyễn Thái Ngọc Duy 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 > --- > 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 > + > -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 majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] rebase: new convenient option to edit a single commit
"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 --- 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 + -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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html