Re: [PATCH 3/3] abbrev: auto size the default abbreviation
From: Jeff King <hidden>
Date: 2016-11-02 02:12:13
Possibly related (same subject, not in this thread)
- 2016-10-04 · Re: [PATCH 3/3] abbrev: auto size the default abbreviation · Jeff King <hidden>
- 2016-10-04 · Re: [PATCH 3/3] abbrev: auto size the default abbreviation · Junio C Hamano <hidden>
- 2016-10-03 · Re: [PATCH 3/3] abbrev: auto size the default abbreviation · Jeff King <hidden>
- 2016-10-03 · Re: [PATCH 3/3] abbrev: auto size the default abbreviation · Junio C Hamano <hidden>
- 2016-10-03 · Re: [PATCH 3/3] abbrev: auto size the default abbreviation · Jeff King <hidden>
On Tue, Nov 01, 2016 at 06:33:38PM -0700, Junio C Hamano wrote:
Jeff King [off-list ref] writes:quoted
On Fri, Sep 30, 2016 at 05:19:37PM -0700, Junio C Hamano wrote:quoted
Introduce a mechanism, where we estimate the number of objects in the repository upon the first request to abbreviate an object name with the default setting and come up with a sane default for the repository. 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.I was idly browsing the draft release notes and then documentation and noticed that, even though the new default is to auto-scale, there is no mention of that in the documentation and there is no way to explicitly ask for auto-scaling. I wonder if we want to have something like this. I actually am inclined to drop the change to config.c and remove the new mention of "auto" in the documentation.
I doubt anybody cares that much either way, but in theory core.abbrev=auto is a way to override core.abbrev=10 in /etc/gitconfig or something. Though I'm having trouble envisioning a case where anybody would set it in /etc/gitconfig, or why somebody would then want to override that back to auto. So I think it is fine either way (but I do agree that the core.abbrev needs _some_ update to mention the auto-scaling behavior).
quoted hunk ↗ jump to hunk
diff --git a/config.c b/config.c index 83fdecb1bc..c363cca4a9 100644 --- a/config.c +++ b/config.c@@ -834,10 +834,16 @@ static int git_default_core_config(const char *var, const char *value) } if (!strcmp(var, "core.abbrev")) { - int abbrev = git_config_int(var, value); - if (abbrev < minimum_abbrev || abbrev > 40) - return -1; - default_abbrev = abbrev; + if (!value) + return config_error_nonbool(var); + if (!strcasecmp(value, "auto")) + default_abbrev = -1; + else { + int abbrev = git_config_int(var, value); + if (abbrev < minimum_abbrev || abbrev > 40) + return -1; + default_abbrev = abbrev; + }
This isn't a new problem you added, but that "return -1" would probably
leave people confused:
$ git -c core.abbrev=2 log
fatal: unable to parse 'core.abbrev' from command-line config
Probably something like:
return error("abbrev length out of range: %d", abbrev);
would be more descriptive. I doubt it's a big deal in practice, though
(why would you set core.abbrev to something silly in the first place?).
-Peff