Re: [PATCHv3 6/9] hash-object: Replace stdin parsing OPT_BOOLEAN by OPT_COUNTUP

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 5, 2013, 18:50 UTC
Message-ID: <7vob9cj7k4.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1375530686-2309-7-git-send-email-stefanbeller@googlemail.com>

Stefan Beller <stefanbeller@googlemail.com> writes:

Show 12 quoted lines

> This task emerged from b04ba2bb (parse-options: deprecate OPT_BOOLEAN,
> 2011-09-27). hash-object is a plumbing layer command, so better
> not change the input/output behavior for now.
>
> Unfortunately we have these lines relying on the count up mechanism of
> OPT_BOOLEAN:
>
> 	if (hashstdin > 1)
> 		errstr = "Multiple --stdin arguments are not supported";
>
> Maybe later, when the plumbing is refined (git 2.0?), we can drop that
> error message and replace the OPT_COUNTUP by OPT_BOOL.

I am of two minds about that as a future direction.

The original motivation of this is that it was too easy to see

	git hash-object Makefile COPYING

to work as expected, and extend that knowledge to expect this

	git hash-objects --stdin --stdin

to somehow work without thinking things through.

So it is not about refining plumbing, but is about educating users. Because a popular system will always have influx of users yet to be educated, I do not think it makes sense to drop this safety.

The patch itself, and others so far except 1 and 2 which are too big for me to carefully review right now, seem to make sense.

Thanks.

Show 18 quoted lines

> Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>
> ---
>  builtin/hash-object.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/hash-object.c b/builtin/hash-object.c
> index 4aea5bb..d7fcf4c 100644
> --- a/builtin/hash-object.c
> +++ b/builtin/hash-object.c
> @@ -71,7 +71,7 @@ static const char *vpath;
>  static const struct option hash_object_options[] = {
>  	OPT_STRING('t', NULL, &type, N_("type"), N_("object type")),
>  	OPT_BOOL('w', NULL, &write_object, N_("write the object into the object database")),
> -	OPT_BOOLEAN( 0 , "stdin", &hashstdin, N_("read the object from stdin")),
> +	OPT_COUNTUP( 0 , "stdin", &hashstdin, N_("read the object from stdin")),
>  	OPT_BOOL( 0 , "stdin-paths", &stdin_paths, N_("read file names from stdin")),
>  	OPT_BOOL( 0 , "no-filters", &no_filters, N_("store file as is without filters")),
>  	OPT_STRING( 0 , "path", &vpath, N_("file"), N_("process file as it were from this path")),