On Mon, Feb 04 2019, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > >> @@ -773,6 +773,14 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) >> return 1; >> continue; >> } >> + if (opt_with_value(arg, "--abbrev-len", &arg)) { >> + unsigned long v; >> + if (!git_parse_ulong(arg, &v)) >> + return 1; >> + int len = abbrev_length_for_object_count(v); >> + printf("%d\n", len); >> + continue; >> + } > > Instead of exposing this pretty-much "test-only" feature as a new > option to t/helper/test-tool, I think it is OK, if not even better, > to have it in rev-parse proper like this patch does. While I mainly added this code so I could prove the docs correct with a test for both myself & others, I think having this exposed is probably useful. I've seen more than once some feature of a web frontend for git where there's both access to aggregate statistics (number of commits or objects), and SHA-1 shortening going on, but the latter is just done via substr(). Right now we have nothing directly exposed to answer "what length would git pick", you can of course e.g. "log --abbrev" a single commit, but if that commit happens to be more ambiguous than most you'll get the right answer. > I however have a mildly strong suspition that people would expect > "rev-parse --abbrev-len=<num>" to be a synonym of "--short=<num>" > > As this is pretty-much a test-only option, perhaps going longer but > more descriptive would make sense? > > git rev-parse --compute-abbrev-length-for <object-count> > > may be an overkill, but something along those lines. Yeah I think that's better. This is so rare that it's better to be verbose. > Oh by the way, the code has decl-after-stmt, and perhaps len needs > to be of type "const int" ;-) Thanks. Will fix.