From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 26 Oct 2007 20:59:24 GMT
Subject: Re: [PATCH 4/9] git-cat-file: Add --stdin option
Message-ID: <7vd4v1y4lv.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <1193307927-3592-5-git-send-email-aroben@apple.com>

Adam Roben <aroben@apple.com> writes:

> @@ -23,6 +23,10 @@ OPTIONS
>  	For a more complete list of ways to spell object names, see
>  	"SPECIFYING REVISIONS" section in gitlink:git-rev-parse[1].
>  
> +--stdin::
> +	Read object names from stdin instead of specifying one on the
> +	command line.
> +

This does not talk about modified output format: what the format
is, nor when that modified format is used.

> @@ -139,16 +139,26 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
>  	if (!buf)
>  		die("git-cat-file %s: bad file", obj_name);
>  
> +	if (print_size) {
> +		printf("%lu\n", size);
> +		fflush(stdout);
> +	}
>  	write_or_die(1, buf, size);
> +	if (print_size) {
> +		printf("\n");
> +		fflush(stdout);
> +	}
>  	return 0;
>  }
>  

Not that I object strongly to it, but do we need extra LF after
the contents?

  - "It would help readers written in typical scripting
    languages" is an acceptable answer, but I doubt that is the
    case --- the reader is given the number of bytes and is
    going to "read($pipe, $buf, $that_size)" anyway.

  - "The reader can assert that one-byte past the content is a
    LF to catch errors, and this LF would help re-synchronize
    after such an error" would be another acceptable answer, but
    for the re-synchronization to work, the output needs to tell
    which record each chunk is about (i.e. if the output were
    "<type> <sha1> <size>LF<contents>LF", the "re-sync" argument
    would make a bit more sense).

> +	print_size = !opt || opt == 'p';

Needs a bit of comment here, and in the documentation.  E.g.

	git-cat-file --stdin -t <list-of-sha1
        git-cat-file --stdin -s <list-of-sha1

	are ways to check types and sizes of the objects in the
	list.

How does --stdin interact with -e?

How does --stdin interact with -p when printing a tree or a tag
object?

How does "blob --stdin" do when input sequence contains a non
blob SHA1?

It almost feels that --stdin should be named something else,
such as --batch or --bulk, as it is not just affecting the
input.

Here is an alternative suggestion.

   Two new options, --batch and --batch-check, are introduced.
   These options are incompatible with -[tsep] or an object type
   given as the first parameter to git-cat-file.

   * git-cat-file --batch-check <list-of-sha1

     outputs a record of this form

          <sha1> SP <type> SP <size> LF

     for each of the input lines.

   * git-cat-file --batch <list-of-sha1

     outputs a record of this form

          <sha1> SP <type> SP <size> LF <contents> LF

     for each of the input lines.

  For a missing object, either option gives a record of form:

          <sha1> SP missing LF