Wink Saville <w...@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 <w...@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.

Reply via email to