Jeff Hostetler <[email protected]> writes:
> + sti = stat_tracking_info(branch, &nr_ahead, &nr_behind,
> + &base, s->ahead_behind_flags);
> if (base) {
> base = shorten_unambiguous_ref(base, 0);
> fprintf(s->fp, "# branch.upstream %s%c", base, eol);
> free((char *)base);
>
> - if (ab_info)
> - fprintf(s->fp, "# branch.ab +%d -%d%c",
> nr_ahead, nr_behind, eol);
> + if (sti >= 0) {
> + if (s->ahead_behind_flags == AHEAD_BEHIND_FULL)
> + fprintf(s->fp, "# branch.ab +%d -%d%c",
> + nr_ahead, nr_behind, eol);
> + else if (s->ahead_behind_flags ==
> AHEAD_BEHIND_QUICK)
> + fprintf(s->fp, "# branch.equal %s%c",
> + sti ? "neq" : "eq", eol);
This is related to what I said in the review of [1/5], but this
arrangement to require the caller to know that _QUICK request
results in incomplete information smells like a bit of maintenance
hassle.
We'd rather allow the caller to tell if it was given incomplete
information from the values returned by stat_tracking_info()
function (notice the plural "values" here; what is returned in
nr_{ahead,behind} also counts). For example, we can keep the (-1 =>
"no relation", 0 => "identical", 1 => "different") as return values
of the function, but have it clear nr_{ahead,behind} when it only
knows the two are different but not by how many commits. That way,
this step can instead do:
ab_info = stat_tracking_info(...);
if (base) {
... do the base thing ...
if (ab_info > 0) {
/* different */
if (nr_ahead || nr_behind)
fprintf(... +%d -%d ... nr_ahead, nr_behind,
...);
else
fprintf(... "neq" ...);
} else if (!ab_info) {
/* same */
fprintf(... +%d -%d ... nr_ahead, nr_behind, ...);
}
...
That would allow us to later add different kinds of _QUICK that do
not give full information without having to update this consumer
with something like
- else if (s->ahead_behind_flags == AHEAD_BEHIND_QUICK)
+ else if (s->ahead_behind_flags == AHEAD_BEHIND_QUICK ||
+ s->ahead_behind_flags == AHEAD_BEHIND_QUICK_DIFFERENTLY)
when it happens.
Two related tangents.
- I do not see why "eq" need to exist at all. Even in _QUICK mode,
when you determine that two are identical, you know your branch
is ahead and behind by 0 commit each.
- A short-format that is easy to parse by machines does not have to
be cryptic to humans. s/neq/not-equal/ or something may be an
improvement.
Thanks.
