On 11/3/2020 8:44 PM, Junio C Hamano wrote: > Early text written in 2006 explains the "--abbrev=<n>" option to > "show only a partial prefix", without saying that the length of the > partial prefix is not necessarily the number given to the option to > ensure that the output names the object uniquely. > > Luckily, the text written for "git describe" in 2011 explains that > the output is made to name the object uniquely using at least <n> > hexdigits much clearly. > > Model the explanation and update documentation for the diff family > of commands, "blame", "branch --verbose", "ls-files" and "ls-tree". Thanks for being thorough here! Your patch is already an improvement on what we have. I hope that my suggestions below help to improve it further. > --- a/Documentation/diff-options.txt > +++ b/Documentation/diff-options.txt > @@ -446,7 +446,8 @@ endif::git-format-patch[] > --abbrev[=<n>]:: > Instead of showing the full 40-byte hexadecimal object > name in diff-raw format output and diff-tree header > - lines, show only a partial prefix. > + lines, show <n> hexadecimal digits, or as many digits > + as needed to form a unique object name. A reoccurring theme in this patch is that the --abbrev option provides a lower bound, even if fewer digits are needed for a unique object name. Using these two phrases with an "or" has some ambiguity about which is chosen. Perhaps we should try to describe it as "<n> is the default, but can grow as needed". Here is an attempt: - lines, show only a partial prefix. + lines, show at least <n> hexadecimal digits, increasing the + prefix length as needed to uniquely identify an object. If you prefer wording like this, then there are a few very similar snippets later in git-branch.txt and git-ls-files.txt. > --- a/Documentation/git-blame.txt > +++ b/Documentation/git-blame.txt > @@ -87,7 +87,9 @@ include::blame-options.txt[] > > --abbrev=<n>:: > Instead of using the default 7+1 hexadecimal digits as the > - abbreviated object name, use <n>+1 digits. Note that 1 column > + abbreviated object name, use <m>+1 digits, where <m> is at > + least <n> but ensures the commit object names are unique. > + Note that 1 column > is used for a caret to mark the boundary commit. This captures the "at least <n>" pretty well, especially considering the complications around "+1". > ---abbrev=<length>:: > - Alter the sha1's minimum display length in the output listing. > +--abbrev=<n>:: > + In the verbose listing that show the commit object name, > + use <n>, or as many hexdigits as needed to form a unique > + object name. Adapting from your wording here made me think that this might be a simpler way to phrase it: +--abbrev=<n>:: + In the verbose listing that show the commit object name, + use at least <n> hexdigits to form a unique object name. > diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt > index 17c5aac4b7..da6ab0de0c 100644 > --- a/Documentation/pretty-options.txt > +++ b/Documentation/pretty-options.txt > @@ -16,7 +16,8 @@ configuration (see linkgit:git-config[1]). > > --abbrev-commit:: > Instead of showing the full 40-byte hexadecimal commit object > - name, show only a partial prefix. Non default number of > + name, show only a partial prefix that names the object uniquely. Perhaps "show a prefix that uniquely identifies the object" ? > + Non default number of > digits can be specified with "--abbrev=<n>" (which also modifies > diff output, if it is displayed). I know this is in the existing docs, but this last sentence should probably start with "A non-default number of digits..." Alternatively, we could rephrase the entire thing to use more active voice: + The `--abbrev=<n>` option modifies the minimum number of digits + for these prefixes, including objects reported in displayed diff + output. Thanks, -Stolee
