Re: [PATCH 5/5] check-ref-format: New --stdin option

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Dec 19, 2016, 11:22 UTC
Message-ID: <9d2d25d8-e9cf-5d9f-8e7e-5d426e219344@alum.mit.edu>
In-Reply-To: <20161104191358.28812-6-ijackson@chiark.greenend.org.uk>

On 11/04/2016 08:13 PM, Ian Jackson wrote:

Show 64 quoted lines

> Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
> ---
>  Documentation/git-check-ref-format.txt | 10 ++++++++--
>  builtin/check-ref-format.c             | 34 +++++++++++++++++++++++++++++++---
>  2 files changed, 39 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt
> index e9a2657..5a213ce 100644
> --- a/Documentation/git-check-ref-format.txt
> +++ b/Documentation/git-check-ref-format.txt
> @@ -10,8 +10,9 @@ SYNOPSIS
>  [verse]
>  'git check-ref-format' [--report-errors] [--normalize]
>         [--[no-]allow-onelevel] [--refspec-pattern]
> -       <refname>
> -'git check-ref-format' [--report-errors] --branch <branchname-shorthand>
> +       <refname> | --stdin
> +'git check-ref-format' [--report-errors] --branch
> +       <branchname-shorthand> | --stdin
>  
>  DESCRIPTION
>  -----------
> @@ -109,6 +110,11 @@ OPTIONS
>  	If any ref does not check OK, print a message to stderr.
>          (By default, git check-ref-format is silent.)
>  
> +--stdin::
> +	Instead of checking on ref supplied on the command line,
> +	read refs, one per line, from stdin.  The exit status is
> +	0 if all the refs were OK.
> +
>  
>  EXAMPLES
>  --------
> diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
> index 559d5c2..87f52fa 100644
> --- a/builtin/check-ref-format.c
> +++ b/builtin/check-ref-format.c
> @@ -76,6 +76,7 @@ static int check_one_ref_format(const char *refname)
>  int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
>  {
>  	int i;
> +	int use_stdin = 0;
>  
>  	if (argc == 2 && !strcmp(argv[1], "-h"))
>  		usage(builtin_check_ref_format_usage);
> @@ -93,6 +94,8 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
>  			check_branch = 1;
>  		else if (!strcmp(argv[i], "--report-errors"))
>  			report_errors = 1;
> +		else if (!strcmp(argv[i], "--stdin"))
> +			use_stdin = 1;
>  		else
>  			usage(builtin_check_ref_format_usage);
>  	}
> @@ -100,8 +103,33 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
>  	if (check_branch && (flags || normalize))
>  		usage(builtin_check_ref_format_usage);
>  
> -	if (! (i == argc - 1))
> -		usage(builtin_check_ref_format_usage);
> +	if (!use_stdin) {
> +		if (! (i == argc - 1))
> +			usage(builtin_check_ref_format_usage);

Given the changes that you made to support `--stdin`, it would be pretty easy to support multiple command line arguments, now, too. (But this needn't be part of your patch series.)

Show 11 quoted lines

> +
> +		return check_one_ref_format(argv[i]);
> +	} else {
> +		char buffer[2048];
> +		int worst = 0;
>  
> -	return check_one_ref_format(argv[i]);
> +		if (! (i == argc))
> +			usage(builtin_check_ref_format_usage);
> +
> +		while (fgets(buffer, sizeof(buffer), stdin)) {

`strbuf_getline()` would make this a lot easier and also eliminate the need to specify a buffer size.

Show 18 quoted lines

> +			char *newline = strchr(buffer, '\n');
> +			if (!newline) {
> +				fprintf(stderr, "%s --stdin: missing final newline or line too long\n", *argv);
> +				exit(127);
> +			}
> +			*newline = 0;
> +			int got = check_one_ref_format(buffer);
> +			if (got > worst)
> +				worst = got;
> +		}
> +		if (!feof(stdin)) {
> +			perror("reading from stdin");
> +			exit(127);
> +		}
> +		return worst;
> +	}
>  }
>

Michael