Re: [PATCH 2/3] added --batch option to mktree
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 14, 2009, 06:18 UTC
- Message-ID
- <7vhbzoxl5k.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <loom.20090514T051004-665@post.gmane.org>
Josh Micich <josh.micich@gmail.com> writes:
> This option enables creation of many tree objects with a single process
... which is desirable in what way? how does this justify the cost of maintenance? what is it used for?
Show 11 quoted lines
> DESCRIPTION > ----------- > @@ -25,6 +25,11 @@ OPTIONS > Allow missing objects. The default behaviour (without this option) > is to verify that each tree entry's sha1 identifies an existing > object. > +--batch:: > + Allow building of more than one tree object before exiting. Each > + tree is separated by as single blank line. The final new-line is > + optional. Note - if the '-z' option is used, lines are terminated > + with NUL.
I think you want an blank line before this.
Show 17 quoted lines
> Author
> ------
> diff --git a/builtin-mktree.c b/builtin-mktree.c
> index db647ce..a56c917 100644
> --- a/builtin-mktree.c
> +++ b/builtin-mktree.c
> @@ -63,7 +63,7 @@ static void write_tree(unsigned char *sha1)
> }
>
> static const char *mktree_usage[] = {
> - "git mktree [-z] [--missing]",
> + "git mktree [-z] [--missing] [--batch]",
> NULL
> };
>
> @@ -125,20 +125,42 @@ int cmd_mktree(int ac, const char **av, const char
> *prefix)Linewrapped and would not apply.
Show 12 quoted lines
> unsigned char sha1[20];
> int line_termination = '\n';
> int allow_missing = 0;
> + int is_batch_mode = 0;
> +
> const struct option option[] = {
> OPT_SET_INT('z', NULL, &line_termination, "input is NUL
> terminated", '\0'),
> OPT_SET_INT( 0 , "missing", &allow_missing, "allow missing
> objects", 1),
> + OPT_SET_INT( 0 , "batch", &is_batch_mode, "interactively create
> more than one tree", 1),What do you mean by "interactively"? Does anybody type from the standard input?
Show 6 quoted lines
> ac = parse_options(ac, av, option, mktree_usage, 0); > > - while (strbuf_getline(&sb, stdin, line_termination) != EOF) > - mktree_line(sb.buf, sb.len, line_termination, allow_missing); > - > + int got_eof = 0;
Decl-after-statement.
Show 10 quoted lines
> + while (!got_eof) {
> + while (1) {
> + if (strbuf_getline(&sb, stdin, line_termination) ==
> EOF) {
> + got_eof = 1;
> + break;
> + }
> + if (sb.buf[0] == '\0') {
> + // empty lines denote tree boundaries in batch
> modeNo C++ comment please;
> + if (is_batch_mode) {
> + break;
> + }Lose excess {} pair;> + if (is_batch_mode && got_eof && used < 1) {
> + // allow input to finish with a new-line (or not)Style: have an explicit ";" for an empty statement.
But more importantly, what does this comment mean? Why do you want to be loose in input format validation?
Show 6 quoted lines
> + } else {
> + write_tree(sha1);
> + puts(sha1_to_hex(sha1));
> + fflush(stdout);
> + }
> + used=0; // reset tree entry buffer for re-use in batch modeused = 0; /* .... */