Subject: Re: [PATCH 7/8] fetch: fix inconsistent summary width for pruned and updated refs

From: Patrick Steinhardt <[email-masked]>
Date: Thu, 16 Mar 2023 16:06:44 +0100

On Wed, Mar 15, 2023 at 04:12:59PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <[email-masked]> writes:
> 
> > As the abbreviated hashes may have different lengths in order to be
> > unique we thus need to precompute the width of the summary's column by
> > iterating through all the objects. This is done in two locations: once
> > to compute the width for references that are to be pruned, and once for
> > all the other references. Consequentially, it can happen that the width
> > as calculated for these sets of references is different.
> 
> Hmph.  Use of ref_map vs stale_refs as the parameter to call
> transport_summary_width() is to come up with an appropriate width
> for showing the list of stored refs vs the list of pruned refs, so
> from that point of view, an appropriate width for each list is
> calculated to a different number may even be a feature, no?

I'd say it's not. Look at the following output generated by a `git fetch
--prune --no-progress` with a deleted and an updated reference:

    From /tmp/repo
     - [deleted]         (none)     -> origin/to-be-deleted
       82307bb..107b50a  main       -> origin/main

Before my change, the width of the deletion and the reference update are
calculated separately. Given that:

    - we don't even display the object IDs for deleted references

    - the width of the deleted reference's column is static anyway.

I'd argue that it's not a feature that the widths are computed
separately. If it was, you could just skip calculating the width of
deleted references and just print them with a static column width.

The current implementation tends to work in most cases as the column
width is based on the minimum length where all abbreviated object IDs
become unique. And I assume that it's the same for both sets of refs in
the majority of cases. And in the other cases I guess that nobody cares
much anyway.

Practically speaking we could go even further than the current version,
as I now compute the width across _all_ reference updates, even those
which are deletions. But theoretically speaking, we could just skip over
any deletions completely as they won't ever contribute to the column
width anyway.

> I do not mind either way all that much, but a change like this to
> update the presentation may want to be protected with a test from
> future breakages.

Fair, having a test for this would be great. But what kept me from
adding one here is that the column width depends on the length of the
longest shared prefix of two object IDs that are about to be updated.
And I just have no clue how to generate those without brute forcing them
for both SHA1 and SHA256.

Do we have any mechanism for this?

Patrick