* [PATCH] core.abbrev <off|false|no> disables abbreviations
@ 2020-09-01 7:43 Eric Wong
2020-09-01 12:14 ` Derrick Stolee
0 siblings, 1 reply; 13+ messages in thread
From: Eric Wong @ 2020-09-01 7:43 UTC (permalink / raw)
To: git; +Cc: brian m. carlson
These allows users to write hash-agnostic scripts and configs to
disable abbreviations. Using "-c core.abbrev=40" will be
insufficient with SHA-256, and "-c core.abbrev=64" won't work
with SHA-1 repos today.
Signed-off-by: Eric Wong <e@80x24.org>
---
I kinda wanted to allow a value of "max", but I figured the existing
boolean falsiness words might make more sense with `--no-abbrev' in
for some commands... Naming is hard :x
config.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/config.c b/config.c
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;
else {
int abbrev = git_config_int(var, value);
if (abbrev < minimum_abbrev || abbrev > the_hash_algo->hexsz)
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] core.abbrev <off|false|no> disables abbreviations
2020-09-01 7:43 [PATCH] core.abbrev <off|false|no> disables abbreviations Eric Wong
@ 2020-09-01 12:14 ` Derrick Stolee
2020-09-01 14:43 ` Eric Wong
2020-09-01 15:49 ` Junio C Hamano
0 siblings, 2 replies; 13+ messages in thread
From: Derrick Stolee @ 2020-09-01 12:14 UTC (permalink / raw)
To: Eric Wong, git; +Cc: brian m. carlson
On 9/1/2020 3:43 AM, Eric Wong wrote:
> These allows users to write hash-agnostic scripts and configs to
> disable abbreviations. Using "-c core.abbrev=40" will be
> insufficient with SHA-256, and "-c core.abbrev=64" won't work
> with SHA-1 repos today.
>
> Signed-off-by: Eric Wong <e@80x24.org>
> ---
> I kinda wanted to allow a value of "max", but I figured the existing
> boolean falsiness words might make more sense with `--no-abbrev' in
> for some commands... Naming is hard :x
>
> config.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/config.c b/config.c
> 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.
"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)
Perhaps "core.abbrev = never" would be a good option?
After we decide on the word, this patch needs:
* Updates to Documentation/config/core.txt
* A test that works with both hash versions.
Thanks,
-Stolee
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] core.abbrev <off|false|no> disables abbreviations
2020-09-01 12:14 ` Derrick Stolee
@ 2020-09-01 14:43 ` Eric Wong
2020-09-01 14:59 ` Derrick Stolee
2020-12-22 19:42 ` Junio C Hamano
2020-09-01 15:49 ` Junio C Hamano
1 sibling, 2 replies; 13+ messages in thread
From: Eric Wong @ 2020-09-01 14:43 UTC (permalink / raw)
To: Derrick Stolee; +Cc: git, brian m. carlson
Derrick Stolee <stolee@gmail.com> 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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] core.abbrev <off|false|no> disables abbreviations
2020-09-01 14:43 ` Eric Wong
@ 2020-09-01 14:59 ` Derrick Stolee
2020-12-22 19:42 ` Junio C Hamano
1 sibling, 0 replies; 13+ messages in thread
From: Derrick Stolee @ 2020-09-01 14:59 UTC (permalink / raw)
To: Eric Wong; +Cc: git, brian m. carlson
On 9/1/2020 10:43 AM, Eric Wong wrote:
> Derrick Stolee <stolee@gmail.com> wrote:
>> After we decide on the word, this patch needs:
>>
>> * Updates to Documentation/config/core.txt
>
> Will do.
Thanks.
>> * A test that works with both hash versions.
>
> Will do, though not too sure where the tests for this should be.
t3404-rebase-interactive.sh seems to already test the
core.abbrev config value. Could be a good place to provide
an extra check.
t4205-log-pretty-formats.sh could also use some references
to core.abbrev.
Thanks,
-Stolee
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] core.abbrev <off|false|no> disables abbreviations
2020-09-01 12:14 ` Derrick Stolee
2020-09-01 14:43 ` Eric Wong
@ 2020-09-01 15:49 ` Junio C Hamano
2020-09-01 19:14 ` Junio C Hamano
2020-09-01 23:37 ` brian m. carlson
1 sibling, 2 replies; 13+ messages in thread
From: Junio C Hamano @ 2020-09-01 15:49 UTC (permalink / raw)
To: Derrick Stolee; +Cc: Eric Wong, git, brian m. carlson
Derrick Stolee <stolee@gmail.com> writes:
>> + 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 do not particularly mind, but if we imitate the variety of various
boolean false, I'd prefer to see the code to parse them shared to
avoid them drifting apart over time.
> "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.
I was in the vicinity of this code recently for reviewing another
topic, but IIRC, 0 came from the UI level does get rounded up to the
minimum accepted and never reach "default_abbrev", but if you manage
to place 0 or -1 in default_abbrev here (e.g. with additional code,
like the above part with the right hand side of the assignment
updated), I think the value will propagate throughout the codepath
and causes the downstream code to do the right thing. 0 will give
you no-abbreviation (i.e. full length depending on the length of the
hash) and -1 will give you the "scale as appropriate for the size of
the object store".
I have mild preference for using 0 over hardcoded single "full
length" here. Even though we currently do not plan to allow
multiple hashes in use simultaneously in a single invocation of Git,
if that ever happens, we will regret hardcoding the_hash_algo->hexsz
on the right hand side of the assignment here, like this patch does.
Telling the downstream code in the control flow that we want no
truncation by using 0 would keep both 40-hexdigit and 64-hexdigit
hashes to their original length (as opposed to telling it to
truncate at 40 or 64 by using the_hash_algo->hexsz).
Thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] core.abbrev <off|false|no> disables abbreviations
2020-09-01 15:49 ` Junio C Hamano
@ 2020-09-01 19:14 ` Junio C Hamano
2020-09-01 23:37 ` brian m. carlson
1 sibling, 0 replies; 13+ messages in thread
From: Junio C Hamano @ 2020-09-01 19:14 UTC (permalink / raw)
To: Derrick Stolee; +Cc: Eric Wong, git, brian m. carlson
Junio C Hamano <gitster@pobox.com> writes:
> Derrick Stolee <stolee@gmail.com> writes:
>
>>> + 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 do not particularly mind, but if we imitate the variety of various
> boolean false, I'd prefer to see the code to parse them shared to
> avoid them drifting apart over time.
Just a clarification.
- I do not particularly mind having multiple synonyms.
- I do mind these one-off strcasecmp that will cause them to drift
away from what we do for the boolean 'false'.
Thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] core.abbrev <off|false|no> disables abbreviations
2020-09-01 15:49 ` Junio C Hamano
2020-09-01 19:14 ` Junio C Hamano
@ 2020-09-01 23:37 ` brian m. carlson
1 sibling, 0 replies; 13+ messages in thread
From: brian m. carlson @ 2020-09-01 23:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Derrick Stolee, Eric Wong, git
[-- Attachment #1: Type: text/plain, Size: 1375 bytes --]
On 2020-09-01 at 15:49:55, Junio C Hamano wrote:
> I was in the vicinity of this code recently for reviewing another
> topic, but IIRC, 0 came from the UI level does get rounded up to the
> minimum accepted and never reach "default_abbrev", but if you manage
> to place 0 or -1 in default_abbrev here (e.g. with additional code,
> like the above part with the right hand side of the assignment
> updated), I think the value will propagate throughout the codepath
> and causes the downstream code to do the right thing. 0 will give
> you no-abbreviation (i.e. full length depending on the length of the
> hash) and -1 will give you the "scale as appropriate for the size of
> the object store".
>
> I have mild preference for using 0 over hardcoded single "full
> length" here. Even though we currently do not plan to allow
> multiple hashes in use simultaneously in a single invocation of Git,
> if that ever happens, we will regret hardcoding the_hash_algo->hexsz
> on the right hand side of the assignment here, like this patch does.
I think we have some commands that accept --abbrev=0 as a value meaning
"no abbreviation", because I've touched that code as part of the SHA-256
work. So as far as the option value is concerned, I think it may make
sense to use 0 for that meaning and just document it.
--
brian m. carlson: Houston, Texas, US
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] core.abbrev <off|false|no> disables abbreviations
2020-09-01 14:43 ` Eric Wong
2020-09-01 14:59 ` Derrick Stolee
@ 2020-12-22 19:42 ` Junio C Hamano
2020-12-22 23:17 ` Eric Wong
1 sibling, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2020-12-22 19:42 UTC (permalink / raw)
To: Eric Wong; +Cc: Derrick Stolee, git, brian m. carlson
Eric Wong <e@80x24.org> writes:
> Thanks for the comments, will wait a few days for comments
> before sending out v2.
This has seen some review suggestions and as far as I remember, can
be summarised as:
- there was a rough consensus that this was a desirable feature.
- a one-off hardcoded list of "false" would rather want to be
consistent with config.c::git_parse_maybe_bool_text().
- documentation is missing for the configuration variable.
It has been almost three months; has a v2 been posted that I missed?
Thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] core.abbrev <off|false|no> disables abbreviations
2020-12-22 19:42 ` Junio C Hamano
@ 2020-12-22 23:17 ` Eric Wong
2020-12-22 23:24 ` Junio C Hamano
0 siblings, 1 reply; 13+ messages in thread
From: Eric Wong @ 2020-12-22 23:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Derrick Stolee, git, brian m. carlson
Junio C Hamano <gitster@pobox.com> wrote:
> Eric Wong <e@80x24.org> writes:
>
> > Thanks for the comments, will wait a few days for comments
> > before sending out v2.
>
> This has seen some review suggestions and as far as I remember, can
> be summarised as:
>
> - there was a rough consensus that this was a desirable feature.
>
> - a one-off hardcoded list of "false" would rather want to be
> consistent with config.c::git_parse_maybe_bool_text().
>
> - documentation is missing for the configuration variable.
>
> It has been almost three months; has a v2 been posted that I missed?
Can somebody else take this over? Sorry my brain isn't working
well these months and I'm behind on other things :<
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] core.abbrev <off|false|no> disables abbreviations
2020-12-22 23:17 ` Eric Wong
@ 2020-12-22 23:24 ` Junio C Hamano
2020-12-23 0:10 ` Junio C Hamano
0 siblings, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2020-12-22 23:24 UTC (permalink / raw)
To: Eric Wong; +Cc: Derrick Stolee, git, brian m. carlson
Eric Wong <e@80x24.org> writes:
> Can somebody else take this over? Sorry my brain isn't working
> well these months and I'm behind on other things :<
Surely and thanks for letting us know. Take care and happy holidays
and new year to you.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] core.abbrev <off|false|no> disables abbreviations
2020-12-22 23:24 ` Junio C Hamano
@ 2020-12-23 0:10 ` Junio C Hamano
2020-12-23 14:38 ` Derrick Stolee
2020-12-23 20:21 ` brian m. carlson
0 siblings, 2 replies; 13+ messages in thread
From: Junio C Hamano @ 2020-12-23 0:10 UTC (permalink / raw)
To: git; +Cc: Eric Wong, Derrick Stolee, brian m. carlson
From: Eric Wong <e@80x24.org>
Date: Tue, 1 Sep 2020 07:43:55 +0000
Subject: [PATCH] core.abbrev=no disables abbreviations
These allows users to write hash-agnostic scripts and configs to
disable abbreviations. Using "-c core.abbrev=40" will be
insufficient with SHA-256, and "-c core.abbrev=64" won't work
with SHA-1 repos today.
Signed-off-by: Eric Wong <e@80x24.org>
[jc: tweaked implementation, added doc and a test]
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/config/core.txt | 2 ++
config.c | 2 ++
t/t3200-branch.sh | 2 ++
3 files changed, 6 insertions(+)
diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
index 160aacad84..c04f62a54a 100644
--- a/Documentation/config/core.txt
+++ b/Documentation/config/core.txt
@@ -625,4 +625,6 @@ core.abbrev::
computed based on the approximate number of packed objects
in your repository, which hopefully is enough for
abbreviated object names to stay unique for some time.
+ If set to "no", no abbreviation is made and the object names
+ are shown in their full length.
The minimum length is 4.
diff --git a/config.c b/config.c
index 1137bd73af..4c0cf3a1c1 100644
--- a/config.c
+++ b/config.c
@@ -1217,6 +1217,8 @@ 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 (!git_parse_maybe_bool_text(value))
+ default_abbrev = the_hash_algo->hexsz;
else {
int abbrev = git_config_int(var, value);
if (abbrev < minimum_abbrev || abbrev > the_hash_algo->hexsz)
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index a0b832d59e..67db316911 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -305,7 +305,9 @@ test_expect_success 'git branch --list -v with --abbrev' '
git branch -v --list --no-abbrev t >actual.noabbrev &&
git branch -v --list --abbrev=0 t >actual.0abbrev &&
+ git -c core.abbrev=no branch -v --list t >actual.noabbrev-conf &&
test_cmp actual.noabbrev actual.0abbrev &&
+ test_cmp actual.noabbrev actual.noabbrev-conf &&
git branch -v --list --abbrev=36 t >actual.36abbrev &&
# how many hexdigits are used?
--
2.30.0-rc1-197-gdf840da780
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] core.abbrev <off|false|no> disables abbreviations
2020-12-23 0:10 ` Junio C Hamano
@ 2020-12-23 14:38 ` Derrick Stolee
2020-12-23 20:21 ` brian m. carlson
1 sibling, 0 replies; 13+ messages in thread
From: Derrick Stolee @ 2020-12-23 14:38 UTC (permalink / raw)
To: Junio C Hamano, git; +Cc: Eric Wong, brian m. carlson
On 12/22/2020 7:10 PM, Junio C Hamano wrote:
> From: Eric Wong <e@80x24.org>
> Date: Tue, 1 Sep 2020 07:43:55 +0000
> Subject: [PATCH] core.abbrev=no disables abbreviations
>
> These allows users to write hash-agnostic scripts and configs to
> disable abbreviations. Using "-c core.abbrev=40" will be
> insufficient with SHA-256, and "-c core.abbrev=64" won't work
> with SHA-1 repos today.
>
> Signed-off-by: Eric Wong <e@80x24.org>
> [jc: tweaked implementation, added doc and a test]
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Thanks for picking this back up. Your version LGTM.
-Stolee
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] core.abbrev <off|false|no> disables abbreviations
2020-12-23 0:10 ` Junio C Hamano
2020-12-23 14:38 ` Derrick Stolee
@ 2020-12-23 20:21 ` brian m. carlson
1 sibling, 0 replies; 13+ messages in thread
From: brian m. carlson @ 2020-12-23 20:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Eric Wong, Derrick Stolee
[-- Attachment #1: Type: text/plain, Size: 723 bytes --]
On 2020-12-23 at 00:10:23, Junio C Hamano wrote:
> From: Eric Wong <e@80x24.org>
> Date: Tue, 1 Sep 2020 07:43:55 +0000
> Subject: [PATCH] core.abbrev=no disables abbreviations
>
> These allows users to write hash-agnostic scripts and configs to
> disable abbreviations. Using "-c core.abbrev=40" will be
> insufficient with SHA-256, and "-c core.abbrev=64" won't work
> with SHA-1 repos today.
>
> Signed-off-by: Eric Wong <e@80x24.org>
> [jc: tweaked implementation, added doc and a test]
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
I agree this looks good, and it's a feature I think is valuable. Thanks
to everyone involved.
--
brian m. carlson (he/him or they/them)
Houston, Texas, US
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-12-23 20:25 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-01 7:43 [PATCH] core.abbrev <off|false|no> disables abbreviations Eric Wong
2020-09-01 12:14 ` Derrick Stolee
2020-09-01 14:43 ` Eric Wong
2020-09-01 14:59 ` Derrick Stolee
2020-12-22 19:42 ` Junio C Hamano
2020-12-22 23:17 ` Eric Wong
2020-12-22 23:24 ` Junio C Hamano
2020-12-23 0:10 ` Junio C Hamano
2020-12-23 14:38 ` Derrick Stolee
2020-12-23 20:21 ` brian m. carlson
2020-09-01 15:49 ` Junio C Hamano
2020-09-01 19:14 ` Junio C Hamano
2020-09-01 23:37 ` brian m. carlson
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).