On 09/01/2026 16:00, Harald Nordgren wrote:

quoted

Using an enum for a set of flags is a bit confusing.

quoted

On reflection I wonder if we should be calling branch_get_push() instead
of remote_ref_for_branch() and tracking_for_push_dest() as it respects
'push.default' and so the branch it returns is the one that "git push"
without any arguments would push to.

I'm not getting that to work without tests breaking.

It is hard to discuss this without knowing what actually breaks. Are you 
talking about the tests added in this series? If so that means we're 
expecting a different behavior to what "git push" actually does. As Ben 
has pointed out elsewhere in this thread, if you're pushing back to a 
different branch on the same remote as the upstream branch you need to 
set `push.default=current`.

The advantage would be that it simplifies the code and it means we 
respect `push.default=upstream`.

The current implementation returns NULL if there is a push refspec 
exists but it does not match the current branch - in that case we should 
return a copy of the branch name instead.

quoted

I think it would be simpler to just return the full refname and let the
caller shorten it.

We could to that but what's the benefit? Shouldn't a helper function reduce
the work for the caller, not the other way around? 🤗

The benefit is that you get a sane interface rather that returning two 
different versions of the same string in two different ways (one from 
the function's return value and the other from a function parameter). It 
also matches what we do for the upstream branch.

quoted

Why are we checking for BRANCH_MODE_PUSH here? Don't we want to show
this advice regardless of the mode?

Yeah, that's a good point. However, this will often lead to the advice
being shown twice, see this test:

status --no-ahead-behind shows diverged from origin/main and ahead of feature branch

I can't seem to see that test. If we're printing the advice once for the 
upstream branch and once for the default push remote I think that would 
be ok.

quoted

If we don't want to show this can't we set show_divergance_adivce to
false when we call this function - why is it guarded by BRANCH_MODE_PULL
as well?

This will already have been shown the the pull branch (the upstream
branch).

And in the cases when it's NOT shown for the pull branch but only the push
branch has diverged, then pulling won't solve the problem anyway. (Unless
you would try to pull from your push branch.)

But we set show_divergance_advice to false for the push branch so there 
is no need to check the flag.

Thanks

Phillip