Re: [PATCH/RFC] Unify argument and option notation in the docs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



ÅtÄpÃn NÄmec wrote:

> 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@xxxxxxxxx>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]