From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 27 Feb 2012 18:00:57 GMT
Subject: Re: [PATCH 05/11] show: use streaming interface for showing blobs
Message-ID: <7vvcms9mw6.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1330329315-11407-6-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  builtin/log.c    |    9 ++++++++-
>  t/t1050-large.sh |    2 +-
>  2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/log.c b/builtin/log.c
> index 7d1f6f8..4c4b17a 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -386,13 +386,20 @@ static int show_object(const unsigned char *sha1, int show_tag_object,
>  {
>  	unsigned long size;
>  	enum object_type type;
> -	char *buf = read_sha1_file(sha1, &type, &size);
> +	char *buf;
>  	int offset = 0;
>  
> +	if (!show_tag_object) {
> +		fflush(stdout);
> +		return streaming_write_sha1(1, 0, sha1, OBJ_ANY, NULL);
> +	}
> +
> +	buf = read_sha1_file(sha1, &type, &size);
>  	if (!buf)
>  		return error(_("Could not read object %s"), sha1_to_hex(sha1));
>  
>  	if (show_tag_object)
> +		assert(type == OBJ_TAG);
>  		while (offset < size && buf[offset] != '\n') {
>  			int new_offset = offset + 1;
>  			while (new_offset < size && buf[new_offset++] != '\n')

Yuck.

The two callsites to this static function are to do BLOB to do TAG.  And
after you start handing all the blob handling to streaming_write_sha1(),
there is no shared code between the two callers for this function.

So why not remove this function, create one show_blob_object() and the
other show_tag_object(), and update the callers to call the appropriate
one?

> diff --git a/t/t1050-large.sh b/t/t1050-large.sh
> index 39a3e77..66acb3b 100755
> --- a/t/t1050-large.sh
> +++ b/t/t1050-large.sh
> @@ -118,7 +118,7 @@ test_expect_success 'cat-file a large file' '
>  	git cat-file blob :large1 >/dev/null
>  '
>  
> -test_expect_failure 'git-show a large file' '
> +test_expect_success 'git-show a large file' '
>  	git show :large1 >/dev/null
>  
>  '