Re: [PATCH 2/2] init-db: change --template type to OPTION_FILENAME

From: Jeff King <peff@peff.net>
Date: Feb 14, 2018, 14:08 UTC
Message-ID: <20180214140808.GB27850@sigill.intra.peff.net>
In-Reply-To: <20180214105149.28896-2-pclouds@gmail.com>

On Wed, Feb 14, 2018 at 05:51:49PM +0700, Nguyễn Thái Ngọc Duy wrote:

> OPTION_FILENAME has some magic behind the scene, like prefixing which is
> useless for init-db. The $HOME expansion though does come handy and
> makes --template more consistent with the rest (both env and config var
> get $HOME expansion).

Yep, makes sense.

Show 13 quoted lines

> diff --git a/builtin/init-db.c b/builtin/init-db.c
> index 68ff4ad75a..d6bd9f19cb 100644
> --- a/builtin/init-db.c
> +++ b/builtin/init-db.c
> @@ -473,8 +473,9 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
>  	const char *template_dir = NULL;
>  	unsigned int flags = 0;
>  	const struct option init_db_options[] = {
> -		OPT_STRING(0, "template", &template_dir, N_("template-directory"),
> -				N_("directory from which templates will be used")),
> +		{ OPTION_FILENAME, 0, "template", &template_dir,
> +			N_("template-directory"),
> +			N_("directory from which templates will be used")},

It's a shame we can't use the slightly more readable OPT_FILENAME(), but it forces the use of "file" for the argument name. I wonder if it really ought to be OPT_PATH(), and say "path", which would work more universally.

At any rate, I'm fine with this until somebody feels like fiddling with the macros.

-Peff