Re: [PATCH v17 1/2] refactor format_branch_comparison in preparation
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Jan 9, 2026, 14:56 UTC
- Message-ID
- <daab4e8f-cd9e-40af-bdfd-c5884d15d852@gmail.com>
- In-Reply-To
- <b62a9feb4ddefb34945a83fd7d76c81c871ea144.1767608269.git.gitgitgadget@gmail.com>
Hi Harald
On 05/01/2026 10:17, Harald Nordgren via GitGitGadget wrote:
> From: Harald Nordgren <haraldnordgren@gmail.com> > > +static void format_branch_comparison(struct strbuf *sb, > + int sti,
I wondered why we needed to pass sti as well as ours and theirs but it is because when we're using AHEAD_BEHIND_QUICK our and theirs are always zero and so we need to check sti to see if the branch is up to date. Perhaps we could make this a boolean called 'up_to_date' ?
> + int ours, int theirs, > + const char *branch_name, > + enum ahead_behind_flags abf, > + int show_divergence_advice)
This could be 'bool' not 'int'
Everything else looks fine - it is a faithful conversion from the original and it makes sense to check if the upstream is gone in the caller.
Thanks
Phillip
Show 81 quoted lines
> +{
> + if (!sti) {
> strbuf_addf(sb,
> _("Your branch is up to date with '%s'.\n"),
> - base);
> + branch_name);
> } else if (abf == AHEAD_BEHIND_QUICK) {
> strbuf_addf(sb,
> _("Your branch and '%s' refer to different commits.\n"),
> - base);
> + branch_name);
> if (advice_enabled(ADVICE_STATUS_HINTS))
> strbuf_addf(sb, _(" (use \"%s\" for details)\n"),
> "git status --ahead-behind");
> @@ -2281,7 +2260,7 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb,
> Q_("Your branch is ahead of '%s' by %d commit.\n",
> "Your branch is ahead of '%s' by %d commits.\n",
> ours),
> - base, ours);
> + branch_name, ours);
> if (advice_enabled(ADVICE_STATUS_HINTS))
> strbuf_addstr(sb,
> _(" (use \"git push\" to publish your local commits)\n"));
> @@ -2292,7 +2271,7 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb,
> "Your branch is behind '%s' by %d commits, "
> "and can be fast-forwarded.\n",
> theirs),
> - base, theirs);
> + branch_name, theirs);
> if (advice_enabled(ADVICE_STATUS_HINTS))
> strbuf_addstr(sb,
> _(" (use \"git pull\" to update your local branch)\n"));
> @@ -2305,12 +2284,47 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb,
> "and have %d and %d different commits each, "
> "respectively.\n",
> ours + theirs),
> - base, ours, theirs);
> + branch_name, ours, theirs);
> if (show_divergence_advice &&
> advice_enabled(ADVICE_STATUS_HINTS))
> strbuf_addstr(sb,
> _(" (use \"git pull\" if you want to integrate the remote branch with yours)\n"));
> }
> +}
> +
> +/*
> + * Return true when there is anything to report, otherwise false.
> + */
> +int format_tracking_info(struct branch *branch, struct strbuf *sb,
> + enum ahead_behind_flags abf,
> + int show_divergence_advice)
> +{
> + int ours, theirs, sti;
> + const char *full_base;
> + char *base;
> + int upstream_is_gone = 0;
> +
> + sti = stat_tracking_info(branch, &ours, &theirs, &full_base, 0, abf);
> + if (sti < 0) {
> + if (!full_base)
> + return 0;
> + upstream_is_gone = 1;
> + }
> +
> + base = refs_shorten_unambiguous_ref(get_main_ref_store(the_repository),
> + full_base, 0);
> +
> + if (upstream_is_gone) {
> + strbuf_addf(sb,
> + _("Your branch is based on '%s', but the upstream is gone.\n"),
> + base);
> + if (advice_enabled(ADVICE_STATUS_HINTS))
> + strbuf_addstr(sb,
> + _(" (use \"git branch --unset-upstream\" to fixup)\n"));
> + } else {
> + format_branch_comparison(sb, sti, ours, theirs, base, abf, show_divergence_advice);
> + }
> +
> free(base);
> return 1;
> }