Ævar Arnfjörð Bjarmason <ava...@gmail.com> writes:
> Change the core.abbrev config variable and the corresponding --abbrev
> command-line option to support relative values such as +1 or -1.
>
> Before Linus's e6c587c733 ("abbrev: auto size the default
> abbreviation", 2016-09-30) git would default to abbreviating object
> names to 7-hexdigits, and only picking longer SHA-1s as needed if that
> was ambiguous.
>
> That change instead set the default length as a function of the
> estimated current count of objects:
>
> Based on the expectation that we would see collision in a
> repository with 2^(2N) objects when using object names shortened
> to first N bits, use sufficient number of hexdigits to cover the
> number of objects in the repository. Each hexdigit (4-bits) we
> add to the shortened name allows us to have four times (2-bits) as
> many objects in the repository.
>
> By supporting relative values for core.abbrev we can allow users to
> consistently opt-in for either a higher or lower probability of
> collision, without needing to hardcode a given numeric value like
> "10", which would be overkill on some repositories, and far to small
> on others.
Nicely explained and calculated ;-)
> test_expect_success 'describe core.abbrev=[-+]1 and --abbrev=[-+]1' '
> - test_must_fail git -c core.abbrev=+1 describe | sed_g_tr_d_n >describe
> &&
> - test_must_fail git -c core.abbrev=-1 describe | sed_g_tr_d_n >describe
> &&
> + git -c core.abbrev=-1 describe | sed_g_tr_d_n >describe &&
> + test_byte_count = 6 describe &&
> +
> + git -c core.abbrev=+1 describe | sed_g_tr_d_n >describe &&
> + test_byte_count = 8 describe &&
Even though I see the point of supporting absurdly small absolute
values like 4, I do not quite see the point of supporting negative
relative values here. What's the expected use case?
> git log --abbrev=+1 --pretty=format:%h -1 | tr_d_n >log &&
> - test_byte_count = 4 log &&
> + test_byte_count = 8 log &&
This, together with many many others in the rest of the patch, is
cute but confusing in that the diff shows change from 4 to 8 due to
the redefinition of what abbrev=+1 means. I have a feeling that it
may not be worth doing it "right", but if we were doing it "right",
we would probably have done it in multiple steps:
- the earlier patches in this series that demonstrates
--abbrev=+1 is --abbrev=1 and core.abbrev=+1 is an error.
- ensure --abbrev=+1 is rejected as syntax error just like
core.abbrev=+1 was, without introducing relative values
- introduce relative value.
That way, the last step (which corresponds to this patch) would show
change from "log --abbrev=+1" failing due to syntax error to showing
abbreviated value that is slightly longer than the default.
But a I said, it may not be worth doing so. "Is it worth supporting
negative relative length?" still stands, though.
