Re: [PATCH/RFC] Unify argument and option notation in the docs
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Oct 8, 2010, 07:43 UTC
- Message-ID
- <20101008074320.GB4671@burratino>
- In-Reply-To
- <20101008005256.GA21738@headley>
Štěpán Němec wrote:
Show 7 quoted lines
> Some examples of what this patch is based on (i.e., the current > prevalent usage) follow (all coming from existing documentation): > > Placeholders are enclosed in angle brackets: > <file> > --sort=<key> > --abbrev[=<n>]
[etc]
All sane.
> [It is conceivable I could submit this as a series of smaller patches, > but the problems this is solving didn't seem diverse enough to me to > warrant that.
Since the documentation processor is known to be, um, picky, could you do that? That way after bisecting a formatting problem, one has a diff addressing a single issue to look at.
On the other hand, I am happy enough to comment on a single, monolithic patch on list if you publish the smaller patches making it up in a git repository somewhere.
> 1. Is `[--refs [--unpacked | --all]]' in `git-pack-object' documentation > correct? From my reading of builtin/pack-objects.c, `--unpacked' and > `--all' do the same thing and both imply --refs, so perhaps [--refs | > --unpacked | --all] would make more sense?
Doesn't the OPTIONS section explain what --revs, --unpacked, and --all mean?
I suspect
[--revs] [--unpacked] [--all]
would be clearer, but
[--revs [(--unpacked|--all)...]]
seems fine, too.
By the way, shouldn't that code path use ALLOC_GROW? [1]
> (I also noticed that the > --reflog option is shown in the usage string but undocumented.)
Looks like someone forgot to add it to the man page.
> 2. I left in one special case, namely the GIT_* variables in `git(1)' > synopsis section as values for the `--exec-path' and other options.
Hmm, --exec-path=GIT_EXEC_PATH currently serves as a reminder of the name of the corresponding environment variable, but I don't think that's very important. --exec-path[=<path>] should be fine.
[1] -- 8< -- Subject: pack-objects: use ALLOC_GROW
Invoke ALLOC_GROW from cache.h instead of recaping its definition verbatim. When this code was first written, the ALLOC_GROW macro didn't exist yet; now that the macro does exist, it can make the source a little shorter and more readable.
No functional change intended.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- builtin/pack-objects.c | 16 ++++------------ 1 files changed, 4 insertions(+), 12 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 3756cf3..6ab2878 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -896,13 +896,9 @@ static int check_pbase_path(unsigned hash) if (0 <= pos) return 1; pos = -pos - 1; - if (done_pbase_paths_alloc <= done_pbase_paths_num) { - done_pbase_paths_alloc = alloc_nr(done_pbase_paths_alloc); - done_pbase_paths = xrealloc(done_pbase_paths, - done_pbase_paths_alloc * - sizeof(unsigned)); - } - done_pbase_paths_num++; + ALLOC_GROW(done_pbase_paths, + ++done_pbase_paths_num, + done_pbase_paths_alloc); if (pos < done_pbase_paths_num) memmove(done_pbase_paths + pos + 1, done_pbase_paths + pos, @@ -2248,11 +2244,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) !strcmp("--reflog", arg) || !strcmp("--all", arg)) { use_internal_rev_list = 1; - if (rp_ac >= rp_ac_alloc - 1) { - rp_ac_alloc = alloc_nr(rp_ac_alloc); - rp_av = xrealloc(rp_av, - rp_ac_alloc * sizeof(*rp_av)); - } + ALLOC_GROW(rp_av, rp_ac + 2, rp_ac_alloc); rp_av[rp_ac++] = arg; continue; }
-- 1.7.2.3