* [RFC PATCH v3 1/9] Simplify pick_on_preserving_merges
2018-03-22 16:57 [RFC PATCH v3 0/9] rebase-interactive: Wink Saville
@ 2018-03-22 16:57 ` Wink Saville
2018-03-22 16:57 ` [RFC PATCH v3 2/9] Call git_rebase__interactive from run_specific_rebase Wink Saville
` (7 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Wink Saville @ 2018-03-22 16:57 UTC (permalink / raw)
To: git; +Cc: Wink Saville, sunshine, Johannes.Schindelin, gitster
Signed-off-by: Wink Saville <wink@saville.com>
---
git-rebase--interactive.sh | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 331c8dfea..561e2660e 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -307,17 +307,14 @@ pick_one_preserving_merges () {
esac
sha1=$(git rev-parse $sha1)
- if test -f "$state_dir"/current-commit
+ if test -f "$state_dir"/current-commit && test "$fast_forward" = t
then
- if test "$fast_forward" = t
- then
- while read current_commit
- do
- git rev-parse HEAD > "$rewritten"/$current_commit
- done <"$state_dir"/current-commit
- rm "$state_dir"/current-commit ||
- die "$(gettext "Cannot write current commit's replacement sha1")"
- fi
+ while read current_commit
+ do
+ git rev-parse HEAD > "$rewritten"/$current_commit
+ done <"$state_dir"/current-commit
+ rm "$state_dir"/current-commit ||
+ die "$(gettext "Cannot write current commit's replacement sha1")"
fi
echo $sha1 >> "$state_dir"/current-commit
--
2.16.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [RFC PATCH v3 2/9] Call git_rebase__interactive from run_specific_rebase
2018-03-22 16:57 [RFC PATCH v3 0/9] rebase-interactive: Wink Saville
2018-03-22 16:57 ` [RFC PATCH v3 1/9] Simplify pick_on_preserving_merges Wink Saville
@ 2018-03-22 16:57 ` Wink Saville
2018-03-22 18:27 ` Junio C Hamano
2018-03-22 16:57 ` [RFC PATCH v3 3/9] Indent function git_rebase__interactive Wink Saville
` (6 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Wink Saville @ 2018-03-22 16:57 UTC (permalink / raw)
To: git; +Cc: Wink Saville, sunshine, Johannes.Schindelin, gitster
Instead of indirectly invoking git_rebase__interactive this invokes
it directly after sourcing.
Signed-off-by: Wink Saville <wink@saville.com>
---
git-rebase--interactive.sh | 11 -----------
git-rebase.sh | 11 +++++++++--
2 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 561e2660e..213d75f43 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -740,15 +740,6 @@ get_missing_commit_check_level () {
printf '%s' "$check_level" | tr 'A-Z' 'a-z'
}
-# The whole contents of this file is run by dot-sourcing it from
-# inside a shell function. It used to be that "return"s we see
-# below were not inside any function, and expected to return
-# to the function that dot-sourced us.
-#
-# However, older (9.x) versions of FreeBSD /bin/sh misbehave on such a
-# construct and continue to run the statements that follow such a "return".
-# As a work-around, we introduce an extra layer of a function
-# here, and immediately call it after defining it.
git_rebase__interactive () {
case "$action" in
@@ -1029,5 +1020,3 @@ fi
do_rest
}
-# ... and then we call the whole thing.
-git_rebase__interactive
diff --git a/git-rebase.sh b/git-rebase.sh
index a1f6e5de6..c4ec7c21b 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -196,8 +196,15 @@ run_specific_rebase () {
export GIT_EDITOR
autosquash=
fi
- . git-rebase--$type
- ret=$?
+ if test "$type" = interactive
+ then
+ . git-rebase--interactive
+ git_rebase__interactive
+ ret=$?
+ else
+ . git-rebase--$type
+ ret=$?
+ fi
if test $ret -eq 0
then
finish_rebase
--
2.16.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [RFC PATCH v3 2/9] Call git_rebase__interactive from run_specific_rebase
2018-03-22 16:57 ` [RFC PATCH v3 2/9] Call git_rebase__interactive from run_specific_rebase Wink Saville
@ 2018-03-22 18:27 ` Junio C Hamano
2018-03-22 19:28 ` Wink Saville
0 siblings, 1 reply; 20+ messages in thread
From: Junio C Hamano @ 2018-03-22 18:27 UTC (permalink / raw)
To: Wink Saville; +Cc: git, sunshine, Johannes.Schindelin
Wink Saville <wink@saville.com> writes:
> Instead of indirectly invoking git_rebase__interactive this invokes
> it directly after sourcing.
>
> Signed-off-by: Wink Saville <wink@saville.com>
> ---
> git-rebase--interactive.sh | 11 -----------
> git-rebase.sh | 11 +++++++++--
> 2 files changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 561e2660e..213d75f43 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -740,15 +740,6 @@ get_missing_commit_check_level () {
> printf '%s' "$check_level" | tr 'A-Z' 'a-z'
> }
>
> -# The whole contents of this file is run by dot-sourcing it from
> -# inside a shell function. It used to be that "return"s we see
> -# below were not inside any function, and expected to return
> -# to the function that dot-sourced us.
> -#
> -# However, older (9.x) versions of FreeBSD /bin/sh misbehave on such a
> -# construct and continue to run the statements that follow such a "return".
> -# As a work-around, we introduce an extra layer of a function
> -# here, and immediately call it after defining it.
We still enclose the whole thing (including the returns that are
problematic for older FreeBSD shells) in a shell function, so it's
not like we are dropping the workaround for these systems. It's
just the caller of the function moved.
I think the removal of this large comment is justifiable, but the
structure still needs a bit of explanation, especially given that
the caller in git-rebase.sh needs to treat this scriptlet a bit
differently from others.
If we were not in the (longer term) process of getting rid of
git-rebase.sh, it might even make sense to port the same
"dot-sourced scriptlet defines a shell function to be called, and
the caller calls it after dot-sourcing it" pattern to other rebase
backends, so that the calling side can be unified again to become
something like:
. git-rebase--$type
git_rebase__$type
ret=$?
> git_rebase__interactive () {
>
> case "$action" in
> @@ -1029,5 +1020,3 @@ fi
> do_rest
>
> }
> -# ... and then we call the whole thing.
> -git_rebase__interactive
> diff --git a/git-rebase.sh b/git-rebase.sh
> index a1f6e5de6..c4ec7c21b 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -196,8 +196,15 @@ run_specific_rebase () {
> export GIT_EDITOR
> autosquash=
> fi
> - . git-rebase--$type
> - ret=$?
> + if test "$type" = interactive
> + then
> + . git-rebase--interactive
> + git_rebase__interactive
> + ret=$?
> + else
> + . git-rebase--$type
> + ret=$?
> + fi
> if test $ret -eq 0
> then
> finish_rebase
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH v3 2/9] Call git_rebase__interactive from run_specific_rebase
2018-03-22 18:27 ` Junio C Hamano
@ 2018-03-22 19:28 ` Wink Saville
2018-03-22 20:03 ` [RFC PATCH v3.1 2/9 2/2] rebase-interactive: Do not automatically run code Wink Saville
0 siblings, 1 reply; 20+ messages in thread
From: Wink Saville @ 2018-03-22 19:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List, Eric Sunshine, Johannes Schindelin
On Thu, Mar 22, 2018 at 11:27 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Wink Saville <wink@saville.com> writes:
>
>> Instead of indirectly invoking git_rebase__interactive this invokes
>> it directly after sourcing.
>>
>> Signed-off-by: Wink Saville <wink@saville.com>
>> ---
>> git-rebase--interactive.sh | 11 -----------
>> git-rebase.sh | 11 +++++++++--
>> 2 files changed, 9 insertions(+), 13 deletions(-)
>>
>> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
>> index 561e2660e..213d75f43 100644
>> --- a/git-rebase--interactive.sh
>> +++ b/git-rebase--interactive.sh
>> @@ -740,15 +740,6 @@ get_missing_commit_check_level () {
>> printf '%s' "$check_level" | tr 'A-Z' 'a-z'
>> }
>>
>> -# The whole contents of this file is run by dot-sourcing it from
>> -# inside a shell function. It used to be that "return"s we see
>> -# below were not inside any function, and expected to return
>> -# to the function that dot-sourced us.
>> -#
>> -# However, older (9.x) versions of FreeBSD /bin/sh misbehave on such a
>> -# construct and continue to run the statements that follow such a "return".
>> -# As a work-around, we introduce an extra layer of a function
>> -# here, and immediately call it after defining it.
>
> We still enclose the whole thing (including the returns that are
> problematic for older FreeBSD shells) in a shell function, so it's
> not like we are dropping the workaround for these systems. It's
> just the caller of the function moved.
>
> I think the removal of this large comment is justifiable, but the
> structure still needs a bit of explanation, especially given that
> the caller in git-rebase.sh needs to treat this scriptlet a bit
> differently from others.
>
> If we were not in the (longer term) process of getting rid of
> git-rebase.sh, it might even make sense to port the same
> "dot-sourced scriptlet defines a shell function to be called, and
> the caller calls it after dot-sourcing it" pattern to other rebase
> backends, so that the calling side can be unified again to become
> something like:
>
> . git-rebase--$type
> git_rebase__$type
> ret=$?
I've gone with the above suggestion and added back some
of the header.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC PATCH v3.1 2/9 2/2] rebase-interactive: Do not automatically run code
2018-03-22 19:28 ` Wink Saville
@ 2018-03-22 20:03 ` Wink Saville
2018-03-22 20:46 ` Junio C Hamano
0 siblings, 1 reply; 20+ messages in thread
From: Wink Saville @ 2018-03-22 20:03 UTC (permalink / raw)
To: git; +Cc: Wink Saville, sunshine, Johannes.Schindelin
At Junio's suggestion have git-rebase--am and git-rebase--merge work the
same way as git-rebase--interactive. This makes the code more consistent.
Signed-off-by: Wink Saville <wink@saville.com>
---
git-rebase--am.sh | 17 ++++++-----------
git-rebase--interactive.sh | 8 +++++++-
git-rebase--merge.sh | 17 ++++++-----------
git-rebase.sh | 13 ++++---------
4 files changed, 23 insertions(+), 32 deletions(-)
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index be3f06892..47dc69ed9 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -4,17 +4,14 @@
# Copyright (c) 2010 Junio C Hamano.
#
-# The whole contents of this file is run by dot-sourcing it from
-# inside a shell function. It used to be that "return"s we see
-# below were not inside any function, and expected to return
-# to the function that dot-sourced us.
+# The whole contents of this file is loaded by dot-sourcing it from
+# inside another shell function, hence no shebang on the first line
+# and then the caller invokes git_rebase__am.
#
-# However, older (9.x) versions of FreeBSD /bin/sh misbehave on such a
-# construct and continue to run the statements that follow such a "return".
-# As a work-around, we introduce an extra layer of a function
-# here, and immediately call it after defining it.
+# Previously this file was sourced and it called itself to get this
+# was to get around a bug in older (9.x) versions of FreeBSD.
git_rebase__am () {
-
+echo "git_rebase_am:+" 1>&5
case "$action" in
continue)
git am --resolved --resolvemsg="$resolvemsg" \
@@ -105,5 +102,3 @@ fi
move_to_original_branch
}
-# ... and then we call the whole thing.
-git_rebase__am
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 213d75f43..48f358333 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -740,8 +740,14 @@ get_missing_commit_check_level () {
printf '%s' "$check_level" | tr 'A-Z' 'a-z'
}
+# The whole contents of this file is loaded by dot-sourcing it from
+# inside another shell function, hence no shebang on the first line
+# and then the caller invokes git_rebase__interactive.
+#
+# Previously this file was sourced and it called itself to get this
+# was to get around a bug in older (9.x) versions of FreeBSD.
git_rebase__interactive () {
-
+echo "git_rebase_interactive:+" 1>&5
case "$action" in
continue)
if test ! -d "$rewritten"
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index ceb715453..71de80788 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -104,17 +104,14 @@ finish_rb_merge () {
say All done.
}
-# The whole contents of this file is run by dot-sourcing it from
-# inside a shell function. It used to be that "return"s we see
-# below were not inside any function, and expected to return
-# to the function that dot-sourced us.
+# The whole contents of this file is loaded by dot-sourcing it from
+# inside another shell function, hence no shebang on the first line
+# and then the caller invokes git_rebase__merge.
#
-# However, older (9.x) versions of FreeBSD /bin/sh misbehave on such a
-# construct and continue to run the statements that follow such a "return".
-# As a work-around, we introduce an extra layer of a function
-# here, and immediately call it after defining it.
+# Previously this file was sourced and it called itself to get this
+# was to get around a bug in older (9.x) versions of FreeBSD.
git_rebase__merge () {
-
+echo "git_rebase_merge:+" 1>&5
case "$action" in
continue)
read_state
@@ -171,5 +168,3 @@ done
finish_rb_merge
}
-# ... and then we call the whole thing.
-git_rebase__merge
diff --git a/git-rebase.sh b/git-rebase.sh
index c4ec7c21b..4595a316a 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -196,15 +196,10 @@ run_specific_rebase () {
export GIT_EDITOR
autosquash=
fi
- if test "$type" = interactive
- then
- . git-rebase--interactive
- git_rebase__interactive
- ret=$?
- else
- . git-rebase--$type
- ret=$?
- fi
+ # Source the code and invoke it
+ . git-rebase--$type
+ git_rebase__$type
+ ret=$?
if test $ret -eq 0
then
finish_rebase
--
2.16.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [RFC PATCH v3.1 2/9 2/2] rebase-interactive: Do not automatically run code
2018-03-22 20:03 ` [RFC PATCH v3.1 2/9 2/2] rebase-interactive: Do not automatically run code Wink Saville
@ 2018-03-22 20:46 ` Junio C Hamano
2018-03-22 22:45 ` Wink Saville
0 siblings, 1 reply; 20+ messages in thread
From: Junio C Hamano @ 2018-03-22 20:46 UTC (permalink / raw)
To: Wink Saville; +Cc: git, sunshine, Johannes.Schindelin
Wink Saville <wink@saville.com> writes:
> At Junio's suggestion have git-rebase--am and git-rebase--merge work the
> same way as git-rebase--interactive. This makes the code more consistent.
I mumbled about making git_rebase__$type functions for all $type in
my previous response, but that was done without even looking at
git-rebase--$type.sh scriptlets. It seems that they all shared the
same structure (i.e. define git_rebase__$type function and then at
the end clla it) and were consistent already. It was the v3 that
changed the calling convention only for interactive, which made it
inconsistent. If you are making git-rebase.sh call the helper shell
function for all backend $type, you are keeping the existing
consistency.
This is no longer about "interactive" alone, though, and need to be
retitled ;-)
> Signed-off-by: Wink Saville <wink@saville.com>
> ---
> git-rebase--am.sh | 17 ++++++-----------
> git-rebase--interactive.sh | 8 +++++++-
> git-rebase--merge.sh | 17 ++++++-----------
> git-rebase.sh | 13 ++++---------
> 4 files changed, 23 insertions(+), 32 deletions(-)
>
> diff --git a/git-rebase--am.sh b/git-rebase--am.sh
> index be3f06892..47dc69ed9 100644
> --- a/git-rebase--am.sh
> +++ b/git-rebase--am.sh
> @@ -4,17 +4,14 @@
> # Copyright (c) 2010 Junio C Hamano.
> #
>
> +# The whole contents of this file is loaded by dot-sourcing it from
> +# inside another shell function, hence no shebang on the first line
> +# and then the caller invokes git_rebase__am.
Is this comment necessary?
> +# Previously this file was sourced and it called itself to get this
> +# was to get around a bug in older (9.x) versions of FreeBSD.
ECANTPARSE. But this probably is no longer needed here, even though
it may make sense to explain why this comment is no longer relevant
in the log message. E.g.
The backend scriptlets for "git rebase" are structured in a
bit unusual way for historical reasons. Originally, it was
designed in such a way that dot-sourcing them from "git
rebase" would be sufficient to invoke the specific backend.
When it was discovered that some shell implementations
(e.g. FreeBSD 9.x) misbehaved by exiting when "return" is
executed at the top level of a dot-sourced script (the
original was expecting that the control returns to the next
command in "git rebase" after dot-sourcing the scriptlet),
the whole body of git-rebase--$backend.sh was made into a
shell function git_rebase__$backend and then the scriptlet
was made to call this function at the end as a workaround.
Move the call to "git rebase" side, instead of at the end of
each scriptlet. This would give us a more normal
arrangement where a function library lives in a scriptlet
that is dot-sourced, and then these helper functions are
called by the script that dot-sourced the scriptlet.
While at it, remove the large comment that explains why this
rather unusual structure was used from these scriptlets.
or something like that in the log message, and then we can get rid
of these in-code comments, I would think.
> git_rebase__am () {
> -
> +echo "git_rebase_am:+" 1>&5
debuggin'? I see similar stuff left in other parts (snipped) of
this patch.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH v3.1 2/9 2/2] rebase-interactive: Do not automatically run code
2018-03-22 20:46 ` Junio C Hamano
@ 2018-03-22 22:45 ` Wink Saville
2018-03-23 2:06 ` Junio C Hamano
0 siblings, 1 reply; 20+ messages in thread
From: Wink Saville @ 2018-03-22 22:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List, Eric Sunshine, Johannes Schindelin
On Thu, Mar 22, 2018 at 1:46 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Wink Saville <wink@saville.com> writes:
>
>> At Junio's suggestion have git-rebase--am and git-rebase--merge work the
>> same way as git-rebase--interactive. This makes the code more consistent.
>
> I mumbled about making git_rebase__$type functions for all $type in
> my previous response, but that was done without even looking at
> git-rebase--$type.sh scriptlets. It seems that they all shared the
> same structure (i.e. define git_rebase__$type function and then at
> the end clla it) and were consistent already. It was the v3 that
> changed the calling convention only for interactive, which made it
> inconsistent. If you are making git-rebase.sh call the helper shell
> function for all backend $type, you are keeping the existing
> consistency.
>
> This is no longer about "interactive" alone, though, and need to be
> retitled ;-)
>
>> Signed-off-by: Wink Saville <wink@saville.com>
>> ---
>> git-rebase--am.sh | 17 ++++++-----------
>> git-rebase--interactive.sh | 8 +++++++-
>> git-rebase--merge.sh | 17 ++++++-----------
>> git-rebase.sh | 13 ++++---------
>> 4 files changed, 23 insertions(+), 32 deletions(-)
>>
>> diff --git a/git-rebase--am.sh b/git-rebase--am.sh
>> index be3f06892..47dc69ed9 100644
>> --- a/git-rebase--am.sh
>> +++ b/git-rebase--am.sh
>> @@ -4,17 +4,14 @@
>> # Copyright (c) 2010 Junio C Hamano.
>> #
>>
>> +# The whole contents of this file is loaded by dot-sourcing it from
>> +# inside another shell function, hence no shebang on the first line
>> +# and then the caller invokes git_rebase__am.
>
> Is this comment necessary?
Removed
>
>> +# Previously this file was sourced and it called itself to get this
>> +# was to get around a bug in older (9.x) versions of FreeBSD.
>
> ECANTPARSE. But this probably is no longer needed here, even though
> it may make sense to explain why this comment is no longer relevant
> in the log message. E.g.
>
> The backend scriptlets for "git rebase" are structured in a
> bit unusual way for historical reasons. Originally, it was
> designed in such a way that dot-sourcing them from "git
> rebase" would be sufficient to invoke the specific backend.
> When it was discovered that some shell implementations
> (e.g. FreeBSD 9.x) misbehaved by exiting when "return" is
> executed at the top level of a dot-sourced script (the
> original was expecting that the control returns to the next
> command in "git rebase" after dot-sourcing the scriptlet),
> the whole body of git-rebase--$backend.sh was made into a
> shell function git_rebase__$backend and then the scriptlet
> was made to call this function at the end as a workaround.
>
> Move the call to "git rebase" side, instead of at the end of
> each scriptlet. This would give us a more normal
> arrangement where a function library lives in a scriptlet
> that is dot-sourced, and then these helper functions are
> called by the script that dot-sourced the scriptlet.
>
> While at it, remove the large comment that explains why this
> rather unusual structure was used from these scriptlets.
>
> or something like that in the log message, and then we can get rid
> of these in-code comments, I would think.
Updated commit message
>> git_rebase__am () {
>> -
>> +echo "git_rebase_am:+" 1>&5
>
> debuggin'? I see similar stuff left in other parts (snipped) of
> this patch.
Removed debugging :(
Currently I'm not rebasing the other commits (3..9)
to reduce the amount of work I have to do in each
review cycle, is that OK?
Also, will you merge commits 1 and 2 before the other
commits or is the procedure to merge the complete set
at once?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH v3.1 2/9 2/2] rebase-interactive: Do not automatically run code
2018-03-22 22:45 ` Wink Saville
@ 2018-03-23 2:06 ` Junio C Hamano
0 siblings, 0 replies; 20+ messages in thread
From: Junio C Hamano @ 2018-03-23 2:06 UTC (permalink / raw)
To: Wink Saville; +Cc: Git List, Eric Sunshine, Johannes Schindelin
Wink Saville <wink@saville.com> writes:
> Currently I'm not rebasing the other commits (3..9)
> to reduce the amount of work I have to do in each
> review cycle, is that OK?
Yeah, I want to see others more heavily involved in this part of the
system to comment on your patches. As to the organization of the
changes, I have some more opinions of my own, primarily regarding to
reviewability, but they are of secondary importance than reviews by
area experts. I think it would be helpful to give them a target
that is not moving too rapidly ;-)
> Also, will you merge commits 1 and 2 before the other
> commits or is the procedure to merge the complete set
> at once?
I am inclined to take the early preliminary clean-up steps before
the remainder.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC PATCH v3 3/9] Indent function git_rebase__interactive
2018-03-22 16:57 [RFC PATCH v3 0/9] rebase-interactive: Wink Saville
2018-03-22 16:57 ` [RFC PATCH v3 1/9] Simplify pick_on_preserving_merges Wink Saville
2018-03-22 16:57 ` [RFC PATCH v3 2/9] Call git_rebase__interactive from run_specific_rebase Wink Saville
@ 2018-03-22 16:57 ` Wink Saville
2018-03-23 17:43 ` Johannes Schindelin
2018-03-22 16:57 ` [RFC PATCH v3 4/9] Extract functions out of git_rebase__interactive Wink Saville
` (5 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Wink Saville @ 2018-03-22 16:57 UTC (permalink / raw)
To: git; +Cc: Wink Saville, sunshine, Johannes.Schindelin, gitster
Signed-off-by: Wink Saville <wink@saville.com>
---
git-rebase--interactive.sh | 432 ++++++++++++++++++++++-----------------------
1 file changed, 215 insertions(+), 217 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 213d75f43..a79330f45 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -741,27 +741,26 @@ get_missing_commit_check_level () {
}
git_rebase__interactive () {
-
-case "$action" in
-continue)
- if test ! -d "$rewritten"
- then
- exec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \
- --continue
- fi
- # do we have anything to commit?
- if git diff-index --cached --quiet HEAD --
- then
- # Nothing to commit -- skip this commit
-
- test ! -f "$GIT_DIR"/CHERRY_PICK_HEAD ||
- rm "$GIT_DIR"/CHERRY_PICK_HEAD ||
- die "$(gettext "Could not remove CHERRY_PICK_HEAD")"
- else
- if ! test -f "$author_script"
+ case "$action" in
+ continue)
+ if test ! -d "$rewritten"
then
- gpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse --sq-quote "$gpg_sign_opt")}
- die "$(eval_gettext "\
+ exec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \
+ --continue
+ fi
+ # do we have anything to commit?
+ if git diff-index --cached --quiet HEAD --
+ then
+ # Nothing to commit -- skip this commit
+
+ test ! -f "$GIT_DIR"/CHERRY_PICK_HEAD ||
+ rm "$GIT_DIR"/CHERRY_PICK_HEAD ||
+ die "$(gettext "Could not remove CHERRY_PICK_HEAD")"
+ else
+ if ! test -f "$author_script"
+ then
+ gpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse --sq-quote "$gpg_sign_opt")}
+ die "$(eval_gettext "\
You have staged changes in your working tree.
If these changes are meant to be
squashed into the previous commit, run:
@@ -776,197 +775,197 @@ In both cases, once you're done, continue with:
git rebase --continue
")"
- fi
- . "$author_script" ||
- die "$(gettext "Error trying to find the author identity to amend commit")"
- if test -f "$amend"
- then
- current_head=$(git rev-parse --verify HEAD)
- test "$current_head" = $(cat "$amend") ||
- die "$(gettext "\
+ fi
+ . "$author_script" ||
+ die "$(gettext "Error trying to find the author identity to amend commit")"
+ if test -f "$amend"
+ then
+ current_head=$(git rev-parse --verify HEAD)
+ test "$current_head" = $(cat "$amend") ||
+ die "$(gettext "\
You have uncommitted changes in your working tree. Please commit them
first and then run 'git rebase --continue' again.")"
- do_with_author git commit --amend --no-verify -F "$msg" -e \
- ${gpg_sign_opt:+"$gpg_sign_opt"} $allow_empty_message ||
- die "$(gettext "Could not commit staged changes.")"
- else
- do_with_author git commit --no-verify -F "$msg" -e \
- ${gpg_sign_opt:+"$gpg_sign_opt"} $allow_empty_message ||
- die "$(gettext "Could not commit staged changes.")"
+ do_with_author git commit --amend --no-verify -F "$msg" -e \
+ ${gpg_sign_opt:+"$gpg_sign_opt"} $allow_empty_message ||
+ die "$(gettext "Could not commit staged changes.")"
+ else
+ do_with_author git commit --no-verify -F "$msg" -e \
+ ${gpg_sign_opt:+"$gpg_sign_opt"} $allow_empty_message ||
+ die "$(gettext "Could not commit staged changes.")"
+ fi
fi
- fi
- if test -r "$state_dir"/stopped-sha
- then
- record_in_rewritten "$(cat "$state_dir"/stopped-sha)"
- fi
+ if test -r "$state_dir"/stopped-sha
+ then
+ record_in_rewritten "$(cat "$state_dir"/stopped-sha)"
+ fi
- require_clean_work_tree "rebase"
- do_rest
- return 0
- ;;
-skip)
- git rerere clear
+ require_clean_work_tree "rebase"
+ do_rest
+ return 0
+ ;;
+ skip)
+ git rerere clear
- if test ! -d "$rewritten"
- then
- exec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \
- --continue
- fi
- do_rest
- return 0
- ;;
-edit-todo)
- git stripspace --strip-comments <"$todo" >"$todo".new
- mv -f "$todo".new "$todo"
- collapse_todo_ids
- append_todo_help
- gettext "
+ if test ! -d "$rewritten"
+ then
+ exec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \
+ --continue
+ fi
+ do_rest
+ return 0
+ ;;
+ edit-todo)
+ git stripspace --strip-comments <"$todo" >"$todo".new
+ mv -f "$todo".new "$todo"
+ collapse_todo_ids
+ append_todo_help
+ gettext "
You are editing the todo file of an ongoing interactive rebase.
To continue rebase after editing, run:
git rebase --continue
" | git stripspace --comment-lines >>"$todo"
- git_sequence_editor "$todo" ||
- die "$(gettext "Could not execute editor")"
- expand_todo_ids
-
- exit
- ;;
-show-current-patch)
- exec git show REBASE_HEAD --
- ;;
-esac
-
-comment_for_reflog start
+ git_sequence_editor "$todo" ||
+ die "$(gettext "Could not execute editor")"
+ expand_todo_ids
-if test ! -z "$switch_to"
-then
- GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $switch_to"
- output git checkout "$switch_to" -- ||
- die "$(eval_gettext "Could not checkout \$switch_to")"
+ exit
+ ;;
+ show-current-patch)
+ exec git show REBASE_HEAD --
+ ;;
+ esac
comment_for_reflog start
-fi
-
-orig_head=$(git rev-parse --verify HEAD) || die "$(gettext "No HEAD?")"
-mkdir -p "$state_dir" || die "$(eval_gettext "Could not create temporary \$state_dir")"
-rm -f "$(git rev-parse --git-path REBASE_HEAD)"
-: > "$state_dir"/interactive || die "$(gettext "Could not mark as interactive")"
-write_basic_state
-if test t = "$preserve_merges"
-then
- if test -z "$rebase_root"
+ if test ! -z "$switch_to"
then
- mkdir "$rewritten" &&
- for c in $(git merge-base --all $orig_head $upstream)
- do
- echo $onto > "$rewritten"/$c ||
- die "$(gettext "Could not init rewritten commits")"
- done
- else
- mkdir "$rewritten" &&
- echo $onto > "$rewritten"/root ||
- die "$(gettext "Could not init rewritten commits")"
+ GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $switch_to"
+ output git checkout "$switch_to" -- ||
+ die "$(eval_gettext "Could not checkout \$switch_to")"
+
+ comment_for_reflog start
fi
- # No cherry-pick because our first pass is to determine
- # parents to rewrite and skipping dropped commits would
- # prematurely end our probe
- merges_option=
-else
- merges_option="--no-merges --cherry-pick"
-fi
-
-shorthead=$(git rev-parse --short $orig_head)
-shortonto=$(git rev-parse --short $onto)
-if test -z "$rebase_root"
- # this is now equivalent to ! -z "$upstream"
-then
- shortupstream=$(git rev-parse --short $upstream)
- revisions=$upstream...$orig_head
- shortrevisions=$shortupstream..$shorthead
-else
- revisions=$onto...$orig_head
- shortrevisions=$shorthead
-fi
-if test t != "$preserve_merges"
-then
- git rebase--helper --make-script ${keep_empty:+--keep-empty} \
- $revisions ${restrict_revision+^$restrict_revision} >"$todo" ||
- die "$(gettext "Could not generate todo list")"
-else
- format=$(git config --get rebase.instructionFormat)
- # the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse
- git rev-list $merges_option --format="%m%H ${format:-%s}" \
- --reverse --left-right --topo-order \
- $revisions ${restrict_revision+^$restrict_revision} | \
- sed -n "s/^>//p" |
- while read -r sha1 rest
- do
- if test -z "$keep_empty" && is_empty_commit $sha1 && ! is_merge_commit $sha1
- then
- comment_out="$comment_char "
- else
- comment_out=
- fi
+ orig_head=$(git rev-parse --verify HEAD) || die "$(gettext "No HEAD?")"
+ mkdir -p "$state_dir" || die "$(eval_gettext "Could not create temporary \$state_dir")"
+ rm -f "$(git rev-parse --git-path REBASE_HEAD)"
+ : > "$state_dir"/interactive || die "$(gettext "Could not mark as interactive")"
+ write_basic_state
+ if test t = "$preserve_merges"
+ then
if test -z "$rebase_root"
then
- preserve=t
- for p in $(git rev-list --parents -1 $sha1 | cut -d' ' -s -f2-)
+ mkdir "$rewritten" &&
+ for c in $(git merge-base --all $orig_head $upstream)
do
- if test -f "$rewritten"/$p
- then
- preserve=f
- fi
+ echo $onto > "$rewritten"/$c ||
+ die "$(gettext "Could not init rewritten commits")"
done
else
- preserve=f
- fi
- if test f = "$preserve"
- then
- touch "$rewritten"/$sha1
- printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo"
- fi
- done
-fi
-
-# Watch for commits that been dropped by --cherry-pick
-if test t = "$preserve_merges"
-then
- mkdir "$dropped"
- # Save all non-cherry-picked changes
- git rev-list $revisions --left-right --cherry-pick | \
- sed -n "s/^>//p" > "$state_dir"/not-cherry-picks
- # Now all commits and note which ones are missing in
- # not-cherry-picks and hence being dropped
- git rev-list $revisions |
- while read rev
- do
- if test -f "$rewritten"/$rev &&
- ! sane_grep "$rev" "$state_dir"/not-cherry-picks >/dev/null
- then
- # Use -f2 because if rev-list is telling us this commit is
- # not worthwhile, we don't want to track its multiple heads,
- # just the history of its first-parent for others that will
- # be rebasing on top of it
- git rev-list --parents -1 $rev | cut -d' ' -s -f2 > "$dropped"/$rev
- sha1=$(git rev-list -1 $rev)
- sane_grep -v "^[a-z][a-z]* $sha1" <"$todo" > "${todo}2" ; mv "${todo}2" "$todo"
- rm "$rewritten"/$rev
+ mkdir "$rewritten" &&
+ echo $onto > "$rewritten"/root ||
+ die "$(gettext "Could not init rewritten commits")"
fi
- done
-fi
+ # No cherry-pick because our first pass is to determine
+ # parents to rewrite and skipping dropped commits would
+ # prematurely end our probe
+ merges_option=
+ else
+ merges_option="--no-merges --cherry-pick"
+ fi
+
+ shorthead=$(git rev-parse --short $orig_head)
+ shortonto=$(git rev-parse --short $onto)
+ if test -z "$rebase_root"
+ # this is now equivalent to ! -z "$upstream"
+ then
+ shortupstream=$(git rev-parse --short $upstream)
+ revisions=$upstream...$orig_head
+ shortrevisions=$shortupstream..$shorthead
+ else
+ revisions=$onto...$orig_head
+ shortrevisions=$shorthead
+ fi
+ if test t != "$preserve_merges"
+ then
+ git rebase--helper --make-script ${keep_empty:+--keep-empty} \
+ $revisions ${restrict_revision+^$restrict_revision} >"$todo" ||
+ die "$(gettext "Could not generate todo list")"
+ else
+ format=$(git config --get rebase.instructionFormat)
+ # the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse
+ git rev-list $merges_option --format="%m%H ${format:-%s}" \
+ --reverse --left-right --topo-order \
+ $revisions ${restrict_revision+^$restrict_revision} | \
+ sed -n "s/^>//p" |
+ while read -r sha1 rest
+ do
+
+ if test -z "$keep_empty" && is_empty_commit $sha1 && ! is_merge_commit $sha1
+ then
+ comment_out="$comment_char "
+ else
+ comment_out=
+ fi
+
+ if test -z "$rebase_root"
+ then
+ preserve=t
+ for p in $(git rev-list --parents -1 $sha1 | cut -d' ' -s -f2-)
+ do
+ if test -f "$rewritten"/$p
+ then
+ preserve=f
+ fi
+ done
+ else
+ preserve=f
+ fi
+ if test f = "$preserve"
+ then
+ touch "$rewritten"/$sha1
+ printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo"
+ fi
+ done
+ fi
+
+ # Watch for commits that been dropped by --cherry-pick
+ if test t = "$preserve_merges"
+ then
+ mkdir "$dropped"
+ # Save all non-cherry-picked changes
+ git rev-list $revisions --left-right --cherry-pick | \
+ sed -n "s/^>//p" > "$state_dir"/not-cherry-picks
+ # Now all commits and note which ones are missing in
+ # not-cherry-picks and hence being dropped
+ git rev-list $revisions |
+ while read rev
+ do
+ if test -f "$rewritten"/$rev &&
+ ! sane_grep "$rev" "$state_dir"/not-cherry-picks >/dev/null
+ then
+ # Use -f2 because if rev-list is telling us this commit is
+ # not worthwhile, we don't want to track its multiple heads,
+ # just the history of its first-parent for others that will
+ # be rebasing on top of it
+ git rev-list --parents -1 $rev | cut -d' ' -s -f2 > "$dropped"/$rev
+ sha1=$(git rev-list -1 $rev)
+ sane_grep -v "^[a-z][a-z]* $sha1" <"$todo" > "${todo}2" ; mv "${todo}2" "$todo"
+ rm "$rewritten"/$rev
+ fi
+ done
+ fi
-test -s "$todo" || echo noop >> "$todo"
-test -z "$autosquash" || git rebase--helper --rearrange-squash || exit
-test -n "$cmd" && git rebase--helper --add-exec-commands "$cmd"
+ test -s "$todo" || echo noop >> "$todo"
+ test -z "$autosquash" || git rebase--helper --rearrange-squash || exit
+ test -n "$cmd" && git rebase--helper --add-exec-commands "$cmd"
-todocount=$(git stripspace --strip-comments <"$todo" | wc -l)
-todocount=${todocount##* }
+ todocount=$(git stripspace --strip-comments <"$todo" | wc -l)
+ todocount=${todocount##* }
cat >>"$todo" <<EOF
@@ -975,48 +974,47 @@ $comment_char $(eval_ngettext \
"Rebase \$shortrevisions onto \$shortonto (\$todocount commands)" \
"$todocount")
EOF
-append_todo_help
-gettext "
-However, if you remove everything, the rebase will be aborted.
-
-" | git stripspace --comment-lines >>"$todo"
+ append_todo_help
+ gettext "
+ However, if you remove everything, the rebase will be aborted.
-if test -z "$keep_empty"
-then
- printf '%s\n' "$comment_char $(gettext "Note that empty commits are commented out")" >>"$todo"
-fi
+ " | git stripspace --comment-lines >>"$todo"
+ if test -z "$keep_empty"
+ then
+ printf '%s\n' "$comment_char $(gettext "Note that empty commits are commented out")" >>"$todo"
+ fi
-has_action "$todo" ||
- return 2
-cp "$todo" "$todo".backup
-collapse_todo_ids
-git_sequence_editor "$todo" ||
- die_abort "$(gettext "Could not execute editor")"
+ has_action "$todo" ||
+ return 2
-has_action "$todo" ||
- return 2
+ cp "$todo" "$todo".backup
+ collapse_todo_ids
+ git_sequence_editor "$todo" ||
+ die_abort "$(gettext "Could not execute editor")"
-git rebase--helper --check-todo-list || {
- ret=$?
- checkout_onto
- exit $ret
-}
+ has_action "$todo" ||
+ return 2
-expand_todo_ids
+ git rebase--helper --check-todo-list || {
+ ret=$?
+ checkout_onto
+ exit $ret
+ }
-test -d "$rewritten" || test -n "$force_rebase" ||
-onto="$(git rebase--helper --skip-unnecessary-picks)" ||
-die "Could not skip unnecessary pick commands"
+ expand_todo_ids
-checkout_onto
-if test -z "$rebase_root" && test ! -d "$rewritten"
-then
- require_clean_work_tree "rebase"
- exec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \
- --continue
-fi
-do_rest
+ test -d "$rewritten" || test -n "$force_rebase" ||
+ onto="$(git rebase--helper --skip-unnecessary-picks)" ||
+ die "Could not skip unnecessary pick commands"
+ checkout_onto
+ if test -z "$rebase_root" && test ! -d "$rewritten"
+ then
+ require_clean_work_tree "rebase"
+ exec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \
+ --continue
+ fi
+ do_rest
}
--
2.16.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [RFC PATCH v3 4/9] Extract functions out of git_rebase__interactive
2018-03-22 16:57 [RFC PATCH v3 0/9] rebase-interactive: Wink Saville
` (2 preceding siblings ...)
2018-03-22 16:57 ` [RFC PATCH v3 3/9] Indent function git_rebase__interactive Wink Saville
@ 2018-03-22 16:57 ` Wink Saville
2018-03-22 16:57 ` [RFC PATCH v3 5/9] Use new functions in git_rebase__interactive Wink Saville
` (4 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Wink Saville @ 2018-03-22 16:57 UTC (permalink / raw)
To: git; +Cc: Wink Saville, sunshine, Johannes.Schindelin, gitster
The extracted functions are:
- initiate_action
- setup_reflog_action
- init_basic_state
- init_revisions_and_shortrevisions
- complete_action
Signed-off-by: Wink Saville <wink@saville.com>
---
git-rebase--interactive.sh | 211 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 211 insertions(+)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index a79330f45..b72f80ae8 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -740,6 +740,217 @@ get_missing_commit_check_level () {
printf '%s' "$check_level" | tr 'A-Z' 'a-z'
}
+# Initiate an action. If the cannot be any
+# further action it may exec a command
+# or exit and not return.
+#
+# TODO: Consider a cleaner return model so it
+# never exits and always return 0 if process
+# is complete.
+#
+# Parameter 1 is the action to initiate.
+#
+# Returns 0 if the action was able to complete
+# and if 1 if further processing is required.
+initiate_action () {
+ case "$1" in
+ continue)
+ if test ! -d "$rewritten"
+ then
+ exec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \
+ --continue
+ fi
+ # do we have anything to commit?
+ if git diff-index --cached --quiet HEAD --
+ then
+ # Nothing to commit -- skip this commit
+
+ test ! -f "$GIT_DIR"/CHERRY_PICK_HEAD ||
+ rm "$GIT_DIR"/CHERRY_PICK_HEAD ||
+ die "$(gettext "Could not remove CHERRY_PICK_HEAD")"
+ else
+ if ! test -f "$author_script"
+ then
+ gpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse --sq-quote "$gpg_sign_opt")}
+ die "$(eval_gettext "\
+You have staged changes in your working tree.
+If these changes are meant to be
+squashed into the previous commit, run:
+
+ git commit --amend \$gpg_sign_opt_quoted
+
+If they are meant to go into a new commit, run:
+
+ git commit \$gpg_sign_opt_quoted
+
+In both cases, once you're done, continue with:
+
+ git rebase --continue
+")"
+ fi
+ . "$author_script" ||
+ die "$(gettext "Error trying to find the author identity to amend commit")"
+ if test -f "$amend"
+ then
+ current_head=$(git rev-parse --verify HEAD)
+ test "$current_head" = $(cat "$amend") ||
+ die "$(gettext "\
+You have uncommitted changes in your working tree. Please commit them
+first and then run 'git rebase --continue' again.")"
+ do_with_author git commit --amend --no-verify -F "$msg" -e \
+ ${gpg_sign_opt:+"$gpg_sign_opt"} $allow_empty_message ||
+ die "$(gettext "Could not commit staged changes.")"
+ else
+ do_with_author git commit --no-verify -F "$msg" -e \
+ ${gpg_sign_opt:+"$gpg_sign_opt"} $allow_empty_message ||
+ die "$(gettext "Could not commit staged changes.")"
+ fi
+ fi
+
+ if test -r "$state_dir"/stopped-sha
+ then
+ record_in_rewritten "$(cat "$state_dir"/stopped-sha)"
+ fi
+
+ require_clean_work_tree "rebase"
+ do_rest
+ return 0
+ ;;
+ skip)
+ git rerere clear
+
+ if test ! -d "$rewritten"
+ then
+ exec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \
+ --continue
+ fi
+ do_rest
+ return 0
+ ;;
+ edit-todo)
+ git stripspace --strip-comments <"$todo" >"$todo".new
+ mv -f "$todo".new "$todo"
+ collapse_todo_ids
+ append_todo_help
+ gettext "
+You are editing the todo file of an ongoing interactive rebase.
+To continue rebase after editing, run:
+ git rebase --continue
+
+" | git stripspace --comment-lines >>"$todo"
+
+ git_sequence_editor "$todo" ||
+ die "$(gettext "Could not execute editor")"
+ expand_todo_ids
+
+ exit
+ ;;
+ show-current-patch)
+ exec git show REBASE_HEAD --
+ ;;
+ *)
+ return 1 # continue
+ ;;
+ esac
+}
+
+setup_reflog_action () {
+ comment_for_reflog start
+
+ if test ! -z "$switch_to"
+ then
+ GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $switch_to"
+ output git checkout "$switch_to" -- ||
+ die "$(eval_gettext "Could not checkout \$switch_to")"
+
+ comment_for_reflog start
+ fi
+}
+
+init_basic_state () {
+ orig_head=$(git rev-parse --verify HEAD) || die "$(gettext "No HEAD?")"
+ mkdir -p "$state_dir" || die "$(eval_gettext "Could not create temporary \$state_dir")"
+ rm -f "$(git rev-parse --git-path REBASE_HEAD)"
+
+ : > "$state_dir"/interactive || die "$(gettext "Could not mark as interactive")"
+ write_basic_state
+}
+
+init_revisions_and_shortrevisions () {
+ shorthead=$(git rev-parse --short $orig_head)
+ shortonto=$(git rev-parse --short $onto)
+ if test -z "$rebase_root"
+ # this is now equivalent to ! -z "$upstream"
+ then
+ shortupstream=$(git rev-parse --short $upstream)
+ revisions=$upstream...$orig_head
+ shortrevisions=$shortupstream..$shorthead
+ else
+ revisions=$onto...$orig_head
+ shortrevisions=$shorthead
+ fi
+}
+
+complete_action() {
+ test -s "$todo" || echo noop >> "$todo"
+ test -z "$autosquash" || git rebase--helper --rearrange-squash || exit
+ test -n "$cmd" && git rebase--helper --add-exec-commands "$cmd"
+
+ todocount=$(git stripspace --strip-comments <"$todo" | wc -l)
+ todocount=${todocount##* }
+
+cat >>"$todo" <<EOF
+
+$comment_char $(eval_ngettext \
+ "Rebase \$shortrevisions onto \$shortonto (\$todocount command)" \
+ "Rebase \$shortrevisions onto \$shortonto (\$todocount commands)" \
+ "$todocount")
+EOF
+ append_todo_help
+ gettext "
+ However, if you remove everything, the rebase will be aborted.
+
+ " | git stripspace --comment-lines >>"$todo"
+
+ if test -z "$keep_empty"
+ then
+ printf '%s\n' "$comment_char $(gettext "Note that empty commits are commented out")" >>"$todo"
+ fi
+
+
+ has_action "$todo" ||
+ return 2
+
+ cp "$todo" "$todo".backup
+ collapse_todo_ids
+ git_sequence_editor "$todo" ||
+ die_abort "$(gettext "Could not execute editor")"
+
+ has_action "$todo" ||
+ return 2
+
+ git rebase--helper --check-todo-list || {
+ ret=$?
+ checkout_onto
+ exit $ret
+ }
+
+ expand_todo_ids
+
+ test -d "$rewritten" || test -n "$force_rebase" ||
+ onto="$(git rebase--helper --skip-unnecessary-picks)" ||
+ die "Could not skip unnecessary pick commands"
+
+ checkout_onto
+ if test -z "$rebase_root" && test ! -d "$rewritten"
+ then
+ require_clean_work_tree "rebase"
+ exec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \
+ --continue
+ fi
+ do_rest
+}
+
git_rebase__interactive () {
case "$action" in
continue)
--
2.16.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [RFC PATCH v3 5/9] Use new functions in git_rebase__interactive
2018-03-22 16:57 [RFC PATCH v3 0/9] rebase-interactive: Wink Saville
` (3 preceding siblings ...)
2018-03-22 16:57 ` [RFC PATCH v3 4/9] Extract functions out of git_rebase__interactive Wink Saville
@ 2018-03-22 16:57 ` Wink Saville
2018-03-23 17:42 ` Johannes Schindelin
2018-03-22 16:57 ` [RFC PATCH v3 6/9] Add and use git_rebase__interactive__preserve_merges Wink Saville
` (3 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Wink Saville @ 2018-03-22 16:57 UTC (permalink / raw)
To: git; +Cc: Wink Saville, sunshine, Johannes.Schindelin, gitster
Use initiate_action, setup_reflog_action, init_basic_state,
init_revisions_and_shortrevisions and complete_action.
Signed-off-by: Wink Saville <wink@saville.com>
---
git-rebase--interactive.sh | 187 ++-------------------------------------------
1 file changed, 8 insertions(+), 179 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index b72f80ae8..2c10a7f1a 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -952,120 +952,15 @@ EOF
}
git_rebase__interactive () {
- case "$action" in
- continue)
- if test ! -d "$rewritten"
- then
- exec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \
- --continue
- fi
- # do we have anything to commit?
- if git diff-index --cached --quiet HEAD --
- then
- # Nothing to commit -- skip this commit
-
- test ! -f "$GIT_DIR"/CHERRY_PICK_HEAD ||
- rm "$GIT_DIR"/CHERRY_PICK_HEAD ||
- die "$(gettext "Could not remove CHERRY_PICK_HEAD")"
- else
- if ! test -f "$author_script"
- then
- gpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse --sq-quote "$gpg_sign_opt")}
- die "$(eval_gettext "\
-You have staged changes in your working tree.
-If these changes are meant to be
-squashed into the previous commit, run:
-
- git commit --amend \$gpg_sign_opt_quoted
-
-If they are meant to go into a new commit, run:
-
- git commit \$gpg_sign_opt_quoted
-
-In both cases, once you're done, continue with:
-
- git rebase --continue
-")"
- fi
- . "$author_script" ||
- die "$(gettext "Error trying to find the author identity to amend commit")"
- if test -f "$amend"
- then
- current_head=$(git rev-parse --verify HEAD)
- test "$current_head" = $(cat "$amend") ||
- die "$(gettext "\
-You have uncommitted changes in your working tree. Please commit them
-first and then run 'git rebase --continue' again.")"
- do_with_author git commit --amend --no-verify -F "$msg" -e \
- ${gpg_sign_opt:+"$gpg_sign_opt"} $allow_empty_message ||
- die "$(gettext "Could not commit staged changes.")"
- else
- do_with_author git commit --no-verify -F "$msg" -e \
- ${gpg_sign_opt:+"$gpg_sign_opt"} $allow_empty_message ||
- die "$(gettext "Could not commit staged changes.")"
- fi
- fi
-
- if test -r "$state_dir"/stopped-sha
- then
- record_in_rewritten "$(cat "$state_dir"/stopped-sha)"
- fi
-
- require_clean_work_tree "rebase"
- do_rest
- return 0
- ;;
- skip)
- git rerere clear
-
- if test ! -d "$rewritten"
- then
- exec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \
- --continue
- fi
- do_rest
+ initiate_action "$action"
+ ret=$?
+ if test $ret = 0; then
return 0
- ;;
- edit-todo)
- git stripspace --strip-comments <"$todo" >"$todo".new
- mv -f "$todo".new "$todo"
- collapse_todo_ids
- append_todo_help
- gettext "
-You are editing the todo file of an ongoing interactive rebase.
-To continue rebase after editing, run:
- git rebase --continue
-
-" | git stripspace --comment-lines >>"$todo"
-
- git_sequence_editor "$todo" ||
- die "$(gettext "Could not execute editor")"
- expand_todo_ids
-
- exit
- ;;
- show-current-patch)
- exec git show REBASE_HEAD --
- ;;
- esac
-
- comment_for_reflog start
-
- if test ! -z "$switch_to"
- then
- GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $switch_to"
- output git checkout "$switch_to" -- ||
- die "$(eval_gettext "Could not checkout \$switch_to")"
-
- comment_for_reflog start
fi
- orig_head=$(git rev-parse --verify HEAD) || die "$(gettext "No HEAD?")"
- mkdir -p "$state_dir" || die "$(eval_gettext "Could not create temporary \$state_dir")"
- rm -f "$(git rev-parse --git-path REBASE_HEAD)"
+ setup_reflog_action
+ init_basic_state
- : > "$state_dir"/interactive || die "$(gettext "Could not mark as interactive")"
- write_basic_state
if test t = "$preserve_merges"
then
if test -z "$rebase_root"
@@ -1089,18 +984,8 @@ To continue rebase after editing, run:
merges_option="--no-merges --cherry-pick"
fi
- shorthead=$(git rev-parse --short $orig_head)
- shortonto=$(git rev-parse --short $onto)
- if test -z "$rebase_root"
- # this is now equivalent to ! -z "$upstream"
- then
- shortupstream=$(git rev-parse --short $upstream)
- revisions=$upstream...$orig_head
- shortrevisions=$shortupstream..$shorthead
- else
- revisions=$onto...$orig_head
- shortrevisions=$shorthead
- fi
+ init_revisions_and_shortrevisions
+
if test t != "$preserve_merges"
then
git rebase--helper --make-script ${keep_empty:+--keep-empty} \
@@ -1171,61 +1056,5 @@ To continue rebase after editing, run:
done
fi
- test -s "$todo" || echo noop >> "$todo"
- test -z "$autosquash" || git rebase--helper --rearrange-squash || exit
- test -n "$cmd" && git rebase--helper --add-exec-commands "$cmd"
-
- todocount=$(git stripspace --strip-comments <"$todo" | wc -l)
- todocount=${todocount##* }
-
-cat >>"$todo" <<EOF
-
-$comment_char $(eval_ngettext \
- "Rebase \$shortrevisions onto \$shortonto (\$todocount command)" \
- "Rebase \$shortrevisions onto \$shortonto (\$todocount commands)" \
- "$todocount")
-EOF
- append_todo_help
- gettext "
- However, if you remove everything, the rebase will be aborted.
-
- " | git stripspace --comment-lines >>"$todo"
-
- if test -z "$keep_empty"
- then
- printf '%s\n' "$comment_char $(gettext "Note that empty commits are commented out")" >>"$todo"
- fi
-
-
- has_action "$todo" ||
- return 2
-
- cp "$todo" "$todo".backup
- collapse_todo_ids
- git_sequence_editor "$todo" ||
- die_abort "$(gettext "Could not execute editor")"
-
- has_action "$todo" ||
- return 2
-
- git rebase--helper --check-todo-list || {
- ret=$?
- checkout_onto
- exit $ret
- }
-
- expand_todo_ids
-
- test -d "$rewritten" || test -n "$force_rebase" ||
- onto="$(git rebase--helper --skip-unnecessary-picks)" ||
- die "Could not skip unnecessary pick commands"
-
- checkout_onto
- if test -z "$rebase_root" && test ! -d "$rewritten"
- then
- require_clean_work_tree "rebase"
- exec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \
- --continue
- fi
- do_rest
+ complete_action
}
--
2.16.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [RFC PATCH v3 5/9] Use new functions in git_rebase__interactive
2018-03-22 16:57 ` [RFC PATCH v3 5/9] Use new functions in git_rebase__interactive Wink Saville
@ 2018-03-23 17:42 ` Johannes Schindelin
2018-03-23 18:24 ` Junio C Hamano
0 siblings, 1 reply; 20+ messages in thread
From: Johannes Schindelin @ 2018-03-23 17:42 UTC (permalink / raw)
To: Wink Saville; +Cc: git, sunshine, gitster
Hi Wink,
On Thu, 22 Mar 2018, Wink Saville wrote:
> Use initiate_action, setup_reflog_action, init_basic_state,
> init_revisions_and_shortrevisions and complete_action.
>
> Signed-off-by: Wink Saville <wink@saville.com>
> ---
> git-rebase--interactive.sh | 187 ++-------------------------------------------
If you fold this into the previous patch, I am sure that after your 3/9
indenting the function properly, the splitting into functions will look
more or less like this:
-git_rebase__interactive () {
+initiate_action () {
+ action="$1"
[... unchanged code ...]
+}
+
+<next function> () {
+ [... setting up variables ...]
[.. unchanged code ...]
+}
[... more of the same ...]
+
+git_rebase__interactive () {
+ initiate_action "$action" &&
+ <next function> <arguments> &&
+ ...
+}
In other words, it would be easier to review and to verify that the
previous code is left unchanged (although that would have to be verified
manually by applying 3/9 and then looking at the diff with the -w option,
anyway).
Ciao,
Johannes
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH v3 5/9] Use new functions in git_rebase__interactive
2018-03-23 17:42 ` Johannes Schindelin
@ 2018-03-23 18:24 ` Junio C Hamano
2018-03-23 20:09 ` Wink Saville
0 siblings, 1 reply; 20+ messages in thread
From: Junio C Hamano @ 2018-03-23 18:24 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Wink Saville, git, sunshine
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> If you fold this into the previous patch, I am sure that after your 3/9
> indenting the function properly, the splitting into functions will look
> more or less like this:
>
> -git_rebase__interactive () {
> +initiate_action () {
> + action="$1"
>
> [... unchanged code ...]
> +}
> +
> +<next function> () {
> + [... setting up variables ...]
>
> [.. unchanged code ...]
> +}
> [... more of the same ...]
> +
> +git_rebase__interactive () {
> + initiate_action "$action" &&
> + <next function> <arguments> &&
> + ...
> +}
>
> In other words, it would be easier to review and to verify that the
> previous code is left unchanged (although that would have to be verified
> manually by applying 3/9 and then looking at the diff with the -w option,
> anyway).
We are on the same page on this. A series structured to have a step
that looks exactly like the above would be very pleasant to review.
Thanks for a great suggestion.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH v3 5/9] Use new functions in git_rebase__interactive
2018-03-23 18:24 ` Junio C Hamano
@ 2018-03-23 20:09 ` Wink Saville
0 siblings, 0 replies; 20+ messages in thread
From: Wink Saville @ 2018-03-23 20:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Git List, Eric Sunshine
On Fri, Mar 23, 2018 at 11:24 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> If you fold this into the previous patch, I am sure that after your 3/9
>> indenting the function properly, the splitting into functions will look
>> more or less like this:
>>
>> -git_rebase__interactive () {
>> +initiate_action () {
>> + action="$1"
>>
>> [... unchanged code ...]
>> +}
>> +
>> +<next function> () {
>> + [... setting up variables ...]
>>
>> [.. unchanged code ...]
>> +}
>> [... more of the same ...]
>> +
>> +git_rebase__interactive () {
>> + initiate_action "$action" &&
>> + <next function> <arguments> &&
>> + ...
>> +}
>>
>> In other words, it would be easier to review and to verify that the
>> previous code is left unchanged (although that would have to be verified
>> manually by applying 3/9 and then looking at the diff with the -w option,
>> anyway).
>
> We are on the same page on this. A series structured to have a step
> that looks exactly like the above would be very pleasant to review.
>
> Thanks for a great suggestion.
SG and I'll rebase on top of my v5 for the first two commits which
I rebased on master.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC PATCH v3 6/9] Add and use git_rebase__interactive__preserve_merges
2018-03-22 16:57 [RFC PATCH v3 0/9] rebase-interactive: Wink Saville
` (4 preceding siblings ...)
2018-03-22 16:57 ` [RFC PATCH v3 5/9] Use new functions in git_rebase__interactive Wink Saville
@ 2018-03-22 16:57 ` Wink Saville
2018-03-22 16:57 ` [RFC PATCH v3 7/9] Remove unused code paths from git_rebase__interactive Wink Saville
` (2 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Wink Saville @ 2018-03-22 16:57 UTC (permalink / raw)
To: git; +Cc: Wink Saville, sunshine, Johannes.Schindelin, gitster
At the moment it's an exact copy of git_rebase__interactive except
the name has changed.
Signed-off-by: Wink Saville <wink@saville.com>
---
git-rebase--interactive.sh | 108 +++++++++++++++++++++++++++++++++++++++++++++
git-rebase.sh | 7 ++-
2 files changed, 114 insertions(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 2c10a7f1a..ab5513d80 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -1058,3 +1058,111 @@ git_rebase__interactive () {
complete_action
}
+
+git_rebase__interactive__preserve_merges () {
+ initiate_action "$action"
+ ret=$?
+ if test $ret = 0; then
+ return 0
+ fi
+
+ setup_reflog_action
+ init_basic_state
+
+ if test t = "$preserve_merges"
+ then
+ if test -z "$rebase_root"
+ then
+ mkdir "$rewritten" &&
+ for c in $(git merge-base --all $orig_head $upstream)
+ do
+ echo $onto > "$rewritten"/$c ||
+ die "$(gettext "Could not init rewritten commits")"
+ done
+ else
+ mkdir "$rewritten" &&
+ echo $onto > "$rewritten"/root ||
+ die "$(gettext "Could not init rewritten commits")"
+ fi
+ # No cherry-pick because our first pass is to determine
+ # parents to rewrite and skipping dropped commits would
+ # prematurely end our probe
+ merges_option=
+ else
+ merges_option="--no-merges --cherry-pick"
+ fi
+
+ init_revisions_and_shortrevisions
+
+ if test t != "$preserve_merges"
+ then
+ git rebase--helper --make-script ${keep_empty:+--keep-empty} \
+ $revisions ${restrict_revision+^$restrict_revision} >"$todo" ||
+ die "$(gettext "Could not generate todo list")"
+ else
+ format=$(git config --get rebase.instructionFormat)
+ # the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse
+ git rev-list $merges_option --format="%m%H ${format:-%s}" \
+ --reverse --left-right --topo-order \
+ $revisions ${restrict_revision+^$restrict_revision} | \
+ sed -n "s/^>//p" |
+ while read -r sha1 rest
+ do
+
+ if test -z "$keep_empty" && is_empty_commit $sha1 && ! is_merge_commit $sha1
+ then
+ comment_out="$comment_char "
+ else
+ comment_out=
+ fi
+
+ if test -z "$rebase_root"
+ then
+ preserve=t
+ for p in $(git rev-list --parents -1 $sha1 | cut -d' ' -s -f2-)
+ do
+ if test -f "$rewritten"/$p
+ then
+ preserve=f
+ fi
+ done
+ else
+ preserve=f
+ fi
+ if test f = "$preserve"
+ then
+ touch "$rewritten"/$sha1
+ printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo"
+ fi
+ done
+ fi
+
+ # Watch for commits that been dropped by --cherry-pick
+ if test t = "$preserve_merges"
+ then
+ mkdir "$dropped"
+ # Save all non-cherry-picked changes
+ git rev-list $revisions --left-right --cherry-pick | \
+ sed -n "s/^>//p" > "$state_dir"/not-cherry-picks
+ # Now all commits and note which ones are missing in
+ # not-cherry-picks and hence being dropped
+ git rev-list $revisions |
+ while read rev
+ do
+ if test -f "$rewritten"/$rev &&
+ ! sane_grep "$rev" "$state_dir"/not-cherry-picks >/dev/null
+ then
+ # Use -f2 because if rev-list is telling us this commit is
+ # not worthwhile, we don't want to track its multiple heads,
+ # just the history of its first-parent for others that will
+ # be rebasing on top of it
+ git rev-list --parents -1 $rev | cut -d' ' -s -f2 > "$dropped"/$rev
+ sha1=$(git rev-list -1 $rev)
+ sane_grep -v "^[a-z][a-z]* $sha1" <"$todo" > "${todo}2" ; mv "${todo}2" "$todo"
+ rm "$rewritten"/$rev
+ fi
+ done
+ fi
+
+ complete_action
+}
diff --git a/git-rebase.sh b/git-rebase.sh
index c4ec7c21b..b32282f4c 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -199,7 +199,12 @@ run_specific_rebase () {
if test "$type" = interactive
then
. git-rebase--interactive
- git_rebase__interactive
+ if test "$preserve_merges" = t
+ then
+ git_rebase__interactive__preserve_merges
+ else
+ git_rebase__interactive
+ fi
ret=$?
else
. git-rebase--$type
--
2.16.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [RFC PATCH v3 7/9] Remove unused code paths from git_rebase__interactive
2018-03-22 16:57 [RFC PATCH v3 0/9] rebase-interactive: Wink Saville
` (5 preceding siblings ...)
2018-03-22 16:57 ` [RFC PATCH v3 6/9] Add and use git_rebase__interactive__preserve_merges Wink Saville
@ 2018-03-22 16:57 ` Wink Saville
2018-03-22 16:57 ` [RFC PATCH v3 8/9] Remove unused code paths from git_rebase__interactive__preserve_merges Wink Saville
2018-03-22 16:57 ` [RFC PATCH v3 9/9] Remove merges_option and a blank line Wink Saville
8 siblings, 0 replies; 20+ messages in thread
From: Wink Saville @ 2018-03-22 16:57 UTC (permalink / raw)
To: git; +Cc: Wink Saville, sunshine, Johannes.Schindelin, gitster
Since git_rebase__interactive is now never called with
$preserve_merges = t we can remove those code paths.
Signed-off-by: Wink Saville <wink@saville.com>
---
git-rebase--interactive.sh | 95 ++--------------------------------------------
1 file changed, 4 insertions(+), 91 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index ab5513d80..346da0f67 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -961,100 +961,13 @@ git_rebase__interactive () {
setup_reflog_action
init_basic_state
- if test t = "$preserve_merges"
- then
- if test -z "$rebase_root"
- then
- mkdir "$rewritten" &&
- for c in $(git merge-base --all $orig_head $upstream)
- do
- echo $onto > "$rewritten"/$c ||
- die "$(gettext "Could not init rewritten commits")"
- done
- else
- mkdir "$rewritten" &&
- echo $onto > "$rewritten"/root ||
- die "$(gettext "Could not init rewritten commits")"
- fi
- # No cherry-pick because our first pass is to determine
- # parents to rewrite and skipping dropped commits would
- # prematurely end our probe
- merges_option=
- else
- merges_option="--no-merges --cherry-pick"
- fi
+ merges_option="--no-merges --cherry-pick"
init_revisions_and_shortrevisions
- if test t != "$preserve_merges"
- then
- git rebase--helper --make-script ${keep_empty:+--keep-empty} \
- $revisions ${restrict_revision+^$restrict_revision} >"$todo" ||
- die "$(gettext "Could not generate todo list")"
- else
- format=$(git config --get rebase.instructionFormat)
- # the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse
- git rev-list $merges_option --format="%m%H ${format:-%s}" \
- --reverse --left-right --topo-order \
- $revisions ${restrict_revision+^$restrict_revision} | \
- sed -n "s/^>//p" |
- while read -r sha1 rest
- do
-
- if test -z "$keep_empty" && is_empty_commit $sha1 && ! is_merge_commit $sha1
- then
- comment_out="$comment_char "
- else
- comment_out=
- fi
-
- if test -z "$rebase_root"
- then
- preserve=t
- for p in $(git rev-list --parents -1 $sha1 | cut -d' ' -s -f2-)
- do
- if test -f "$rewritten"/$p
- then
- preserve=f
- fi
- done
- else
- preserve=f
- fi
- if test f = "$preserve"
- then
- touch "$rewritten"/$sha1
- printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo"
- fi
- done
- fi
-
- # Watch for commits that been dropped by --cherry-pick
- if test t = "$preserve_merges"
- then
- mkdir "$dropped"
- # Save all non-cherry-picked changes
- git rev-list $revisions --left-right --cherry-pick | \
- sed -n "s/^>//p" > "$state_dir"/not-cherry-picks
- # Now all commits and note which ones are missing in
- # not-cherry-picks and hence being dropped
- git rev-list $revisions |
- while read rev
- do
- if test -f "$rewritten"/$rev &&
- ! sane_grep "$rev" "$state_dir"/not-cherry-picks >/dev/null
- then
- # Use -f2 because if rev-list is telling us this commit is
- # not worthwhile, we don't want to track its multiple heads,
- # just the history of its first-parent for others that will
- # be rebasing on top of it
- git rev-list --parents -1 $rev | cut -d' ' -s -f2 > "$dropped"/$rev
- sha1=$(git rev-list -1 $rev)
- sane_grep -v "^[a-z][a-z]* $sha1" <"$todo" > "${todo}2" ; mv "${todo}2" "$todo"
- rm "$rewritten"/$rev
- fi
- done
- fi
+ git rebase--helper --make-script ${keep_empty:+--keep-empty} \
+ $revisions ${restrict_revision+^$restrict_revision} >"$todo" ||
+ die "$(gettext "Could not generate todo list")"
complete_action
}
--
2.16.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [RFC PATCH v3 8/9] Remove unused code paths from git_rebase__interactive__preserve_merges
2018-03-22 16:57 [RFC PATCH v3 0/9] rebase-interactive: Wink Saville
` (6 preceding siblings ...)
2018-03-22 16:57 ` [RFC PATCH v3 7/9] Remove unused code paths from git_rebase__interactive Wink Saville
@ 2018-03-22 16:57 ` Wink Saville
2018-03-22 16:57 ` [RFC PATCH v3 9/9] Remove merges_option and a blank line Wink Saville
8 siblings, 0 replies; 20+ messages in thread
From: Wink Saville @ 2018-03-22 16:57 UTC (permalink / raw)
To: git; +Cc: Wink Saville, sunshine, Johannes.Schindelin, gitster
Since git_rebase__interactive__preserve_merges is now always called with
$preserve_merges = t we can remove the unused code paths.
Signed-off-by: Wink Saville <wink@saville.com>
---
git-rebase--interactive.sh | 152 ++++++++++++++++++++-------------------------
1 file changed, 69 insertions(+), 83 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 346da0f67..ddbd126f2 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -982,100 +982,86 @@ git_rebase__interactive__preserve_merges () {
setup_reflog_action
init_basic_state
- if test t = "$preserve_merges"
+ if test -z "$rebase_root"
then
- if test -z "$rebase_root"
- then
- mkdir "$rewritten" &&
- for c in $(git merge-base --all $orig_head $upstream)
- do
- echo $onto > "$rewritten"/$c ||
- die "$(gettext "Could not init rewritten commits")"
- done
- else
- mkdir "$rewritten" &&
- echo $onto > "$rewritten"/root ||
+ mkdir "$rewritten" &&
+ for c in $(git merge-base --all $orig_head $upstream)
+ do
+ echo $onto > "$rewritten"/$c ||
die "$(gettext "Could not init rewritten commits")"
- fi
- # No cherry-pick because our first pass is to determine
- # parents to rewrite and skipping dropped commits would
- # prematurely end our probe
- merges_option=
+ done
else
- merges_option="--no-merges --cherry-pick"
+ mkdir "$rewritten" &&
+ echo $onto > "$rewritten"/root ||
+ die "$(gettext "Could not init rewritten commits")"
fi
+ # No cherry-pick because our first pass is to determine
+ # parents to rewrite and skipping dropped commits would
+ # prematurely end our probe
+ merges_option=
+
init_revisions_and_shortrevisions
- if test t != "$preserve_merges"
- then
- git rebase--helper --make-script ${keep_empty:+--keep-empty} \
- $revisions ${restrict_revision+^$restrict_revision} >"$todo" ||
- die "$(gettext "Could not generate todo list")"
- else
- format=$(git config --get rebase.instructionFormat)
- # the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse
- git rev-list $merges_option --format="%m%H ${format:-%s}" \
- --reverse --left-right --topo-order \
- $revisions ${restrict_revision+^$restrict_revision} | \
- sed -n "s/^>//p" |
- while read -r sha1 rest
- do
+ format=$(git config --get rebase.instructionFormat)
+ # the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse
+ git rev-list $merges_option --format="%m%H ${format:-%s}" \
+ --reverse --left-right --topo-order \
+ $revisions ${restrict_revision+^$restrict_revision} | \
+ sed -n "s/^>//p" |
+ while read -r sha1 rest
+ do
- if test -z "$keep_empty" && is_empty_commit $sha1 && ! is_merge_commit $sha1
- then
- comment_out="$comment_char "
- else
- comment_out=
- fi
+ if test -z "$keep_empty" && is_empty_commit $sha1 && ! is_merge_commit $sha1
+ then
+ comment_out="$comment_char "
+ else
+ comment_out=
+ fi
- if test -z "$rebase_root"
- then
- preserve=t
- for p in $(git rev-list --parents -1 $sha1 | cut -d' ' -s -f2-)
- do
- if test -f "$rewritten"/$p
- then
- preserve=f
- fi
- done
- else
- preserve=f
- fi
- if test f = "$preserve"
- then
- touch "$rewritten"/$sha1
- printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo"
- fi
- done
- fi
+ if test -z "$rebase_root"
+ then
+ preserve=t
+ for p in $(git rev-list --parents -1 $sha1 | cut -d' ' -s -f2-)
+ do
+ if test -f "$rewritten"/$p
+ then
+ preserve=f
+ fi
+ done
+ else
+ preserve=f
+ fi
+ if test f = "$preserve"
+ then
+ touch "$rewritten"/$sha1
+ printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo"
+ fi
+ done
# Watch for commits that been dropped by --cherry-pick
- if test t = "$preserve_merges"
- then
- mkdir "$dropped"
- # Save all non-cherry-picked changes
- git rev-list $revisions --left-right --cherry-pick | \
- sed -n "s/^>//p" > "$state_dir"/not-cherry-picks
- # Now all commits and note which ones are missing in
- # not-cherry-picks and hence being dropped
- git rev-list $revisions |
- while read rev
- do
- if test -f "$rewritten"/$rev &&
- ! sane_grep "$rev" "$state_dir"/not-cherry-picks >/dev/null
- then
- # Use -f2 because if rev-list is telling us this commit is
- # not worthwhile, we don't want to track its multiple heads,
- # just the history of its first-parent for others that will
- # be rebasing on top of it
- git rev-list --parents -1 $rev | cut -d' ' -s -f2 > "$dropped"/$rev
- sha1=$(git rev-list -1 $rev)
- sane_grep -v "^[a-z][a-z]* $sha1" <"$todo" > "${todo}2" ; mv "${todo}2" "$todo"
- rm "$rewritten"/$rev
- fi
- done
- fi
+ mkdir "$dropped"
+ # Save all non-cherry-picked changes
+ git rev-list $revisions --left-right --cherry-pick | \
+ sed -n "s/^>//p" > "$state_dir"/not-cherry-picks
+ # Now all commits and note which ones are missing in
+ # not-cherry-picks and hence being dropped
+ git rev-list $revisions |
+ while read rev
+ do
+ if test -f "$rewritten"/$rev &&
+ ! sane_grep "$rev" "$state_dir"/not-cherry-picks >/dev/null
+ then
+ # Use -f2 because if rev-list is telling us this commit is
+ # not worthwhile, we don't want to track its multiple heads,
+ # just the history of its first-parent for others that will
+ # be rebasing on top of it
+ git rev-list --parents -1 $rev | cut -d' ' -s -f2 > "$dropped"/$rev
+ sha1=$(git rev-list -1 $rev)
+ sane_grep -v "^[a-z][a-z]* $sha1" <"$todo" > "${todo}2" ; mv "${todo}2" "$todo"
+ rm "$rewritten"/$rev
+ fi
+ done
complete_action
}
--
2.16.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [RFC PATCH v3 9/9] Remove merges_option and a blank line
2018-03-22 16:57 [RFC PATCH v3 0/9] rebase-interactive: Wink Saville
` (7 preceding siblings ...)
2018-03-22 16:57 ` [RFC PATCH v3 8/9] Remove unused code paths from git_rebase__interactive__preserve_merges Wink Saville
@ 2018-03-22 16:57 ` Wink Saville
8 siblings, 0 replies; 20+ messages in thread
From: Wink Saville @ 2018-03-22 16:57 UTC (permalink / raw)
To: git; +Cc: Wink Saville, sunshine, Johannes.Schindelin, gitster
merges_option is unused in git_rebase__interactive and always empty in
git_rebase__interactive__preserve_merges so it can be removed.
Signed-off-by: Wink Saville <wink@saville.com>
---
git-rebase--interactive.sh | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index ddbd126f2..50323fc27 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -961,8 +961,6 @@ git_rebase__interactive () {
setup_reflog_action
init_basic_state
- merges_option="--no-merges --cherry-pick"
-
init_revisions_and_shortrevisions
git rebase--helper --make-script ${keep_empty:+--keep-empty} \
@@ -996,22 +994,16 @@ git_rebase__interactive__preserve_merges () {
die "$(gettext "Could not init rewritten commits")"
fi
- # No cherry-pick because our first pass is to determine
- # parents to rewrite and skipping dropped commits would
- # prematurely end our probe
- merges_option=
-
init_revisions_and_shortrevisions
format=$(git config --get rebase.instructionFormat)
# the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse
- git rev-list $merges_option --format="%m%H ${format:-%s}" \
+ git rev-list --format="%m%H ${format:-%s}" \
--reverse --left-right --topo-order \
$revisions ${restrict_revision+^$restrict_revision} | \
sed -n "s/^>//p" |
while read -r sha1 rest
do
-
if test -z "$keep_empty" && is_empty_commit $sha1 && ! is_merge_commit $sha1
then
comment_out="$comment_char "
--
2.16.2
^ permalink raw reply related [flat|nested] 20+ messages in thread