Derrick Stolee <stolee@xxxxxxxxx> wrote:
> On 9/1/2020 3:43 AM, Eric Wong wrote:
> > index 2bdff4457b..f2e09c72ca 100644
> > --- a/config.c
> > +++ b/config.c
> > @@ -1217,6 +1217,10 @@ static int git_default_core_config(const char *var, const char *value, void *cb)
> > return config_error_nonbool(var);
> > if (!strcasecmp(value, "auto"))
> > default_abbrev = -1;
> > + else if (!strcasecmp(value, "false") ||
> > + !strcasecmp(value, "no") ||
> > + !strcasecmp(value, "off"))
> > + default_abbrev = the_hash_algo->hexsz;
>
> I'm not sure we need three synonyms for "no-abbrev" here.
I just used the accepted synonyms since I figured users
would be used to them, already.
> "false" would be natural, except I think in a few places
> the config value "0" is also interpreted as "false", but
> as seen below a value of "0" snaps up to the minimum
> allowed abbreviation.
>
> > else {
> > int abbrev = git_config_int(var, value);
> > if (abbrev < minimum_abbrev || abbrev > the_hash_algo->hexsz)
It actually errors out on the next line, here.
Perhaps adopting parse_opt_abbrev_cb behavior of clamping to
the minimum and maximum supported values is more consistent?
> Perhaps "core.abbrev = never" would be a good option?
*shrug*
> After we decide on the word, this patch needs:
>
> * Updates to Documentation/config/core.txt
Will do.
> * A test that works with both hash versions.
Will do, though not too sure where the tests for this should be.
Thanks for the comments, will wait a few days for comments
before sending out v2.
