Re: [RFC PATCH v3 2/9] Call git_rebase__interactive from run_specific_rebase
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 22, 2018, 18:27 UTC
- Message-ID
- <xmqqpo3wklt2.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <47b224af1c22900a69d0e292935f37a719b6a9f3.1521690197.git.wink@saville.com>
Wink Saville <wink@saville.com> writes:
Show 26 quoted lines
> 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=$?
Show 31 quoted lines
> 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