Re: [PATCH] remote: resolve "simple" @{push} when pushing to another remote

[Junio C Hamano <gitster@pobox.com>]

"D. Ben Knoble" <ben.knoble@gmail.com> writes:

>> diff --git a/remote.c b/remote.c
>> index 49ee2ea318..fd434a3a10 100644
>> --- a/remote.c
>> +++ b/remote.c
>> @@ -2016,6 +2016,10 @@ static char *branch_get_push_1(struct repository *repo,
>>                         const char *up;
>>                         char *cur;
>>
>> +                       if (strcmp(remote->name,
>> +                                  remotes_remote_for_branch(remote_state, branch, NULL)))
>> +                               return tracking_for_push_dest(repo, remote, branch->refname, err);
>
> This certainly matches the logic described in the log message, and I
> think it's probably right based on that description. One thing I can't
> decide is whether it should come before or after the current set of
> cases, and I'm undecided. On one hand, it's a new case, so I would
> think it should come after. OTOH, it's a fairly straightforward case,
> and I usually like discharging those early so I can focus on the
> complex stuff.

I actually wondered if the existing case for "simple" becomes a
subset of this new code?  After all, tracking_for_push_dest() asks
where the thing gets pushed to, no?

In any case, all the case arms in this function is getting a bit
unmanageably long and this block is, with an extra artificial block,
unnecessarily deeply nested by one extra level.  I wonder if we can
move the code for "case PUSH_DEFAULT_SIMPLE" case to a separate
function and make it a bit more readable?

I agree that the end result the commit wants to see is sensible and
consistent with the earlier "what should happen with 'simple' when
triangular?" topic.

Thanks.