Re: [PATCH 6/9] builtin-init-db.c: use parse_options()
- From
Michele Ballabio <barra_cuda@katamail.com>
- Date
- Jul 24, 2008, 20:07 UTC
- Message-ID
- <200807242207.02195.barra_cuda@katamail.com>
- In-Reply-To
- <4888AAB2.5050007@free.fr>
On Thursday 24 July 2008, Olivier Marin wrote:
Show 7 quoted lines
> Michele Ballabio a écrit :
> >
> > + const struct option options[] = {
> > + OPT_STRING(0, "template", &template_dir, "dir",
> > + "directory from which templates will be used"),
>
> Perhaps "path", "path to the template repository" to stay consistent with clone.Ok.
> > + OPT_BOOLEAN(0, "bare", &bare, "set up a bare repo"), > > s/set up/setup/ and s/repo/repository/?
I think "set up a bare repository" will be fine.
> > + { OPTION_CALLBACK, 0, "shared", &shared_repository,
> > + "type", "type of shared repository",
>
> What about "permissions", "setup a shared repository"?Ok, but with s/setup/set up/.
> > + PARSE_OPT_OPTARG, parse_opt_shared_cb, PERM_GROUP }, > > Are you sure the default value is really used here?
Yes. Perhaps I don't understand your question. Can you explain what you mean?
Show 7 quoted lines
> Also, perhaps we can play it safer by avoiding changing "share_repository" > directly. > > $ git init -> shared_repository == PERM_UMASK > $ git init --shared --no-shared -> shared_repository == 0 > > It works because PERM_UMASK == 0, but it is a side effect. Don't you think?
Would you like this better, with PARSE_OPT_NONEG?
+ { OPTION_CALLBACK, 0, "shared", &shared_repository, + "permissions", "set up a shared repository", + PARSE_OPT_OPTARG | PARSE_OPT_NONEG, parse_opt_shared_cb, PERM_GROUP },
Or do you prefer changing the callback like this:
+static int parse_opt_shared_cb(const struct option *opt, const char *arg, + int unset) +{ + *(int *)(opt->value) = unset ? PERM_UMASK : git_config_perm("arg", arg); + return 0; +}
Show 8 quoted lines
> > + OPT_BIT('q', "quiet", &flags, "be quiet", INIT_DB_QUIET),
>
> OPT__QUIET(&quiet),
>
> if (quiet)
> flags |= INIT_DB_QUIET;
>
> to use the same quiet option everywhere?I thought about it and decided against ;) And it's one line vs four (counting "int quiet = 0;"). But I see your point.