On Thu, Mar 22, 2018 at 1:46 PM, Junio C Hamano <[email protected]> wrote:
> Wink Saville <[email protected]> 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 <[email protected]>
>> ---
>> 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?
