Re: [PATCH v2 2/3] sha1_file: implement changes for "cat-file --literally -t"

Karthik Nayak [off-list ref] writes:

Looks suspiciously familiar...

+int sha1_object_info_literally(const unsigned char *sha1)
+{
+	enum object_type type;
+	struct strbuf sb = STRBUF_INIT;
+	struct object_info oi = {NULL};
+
+	oi.typename = &sb;
+	oi.typep = &type;
+	if (sha1_object_info_extended(sha1, &oi, LOOKUP_LITERALLY) < 0)
+		return -1;
+	if (*oi.typep > 0)
+		printf("%s\n", typename(*oi.typep));
+	else
+		printf("%s\n", oi.typename->buf);
+	strbuf_release(oi.typename);
+	return 0;
+}
+

NAK.

Please don't add end-user facing final output to sha1_file.c;
instead have the caller use a helper function like this one to
extract necessary information and perform the end-user interaction
there.

On 03/05/2015 02:28 AM, Junio C Hamano wrote:

Karthik Nayak [off-list ref] writes:

quoted

add "sha1_object_info_literally()" which is to be used when
the "literally" option is given to get the type of object
and print it, using "sha1_object_info_extended()".

add "unpack_sha1_header_literally()" to unpack sha headers
which may be greater than 32 bytes, which is the threshold
for a regular object header.

modify "sha1_loose_object_info()" to include a flag argument
and also include changes to call "unpack_sha1_header_literally()"
when the literally flag is passed. Also copies the obtained
type to the typename field of object_info.

modify "sha1_object_info_extended()" to call the function
"sha1_loose_object_info()" with flags.

Signed-off-by: Karthik Nayak <redacted>
---
  sha1_file.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++++------
  1 file changed, 77 insertions(+), 7 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 69a60ec..1068ca0 100644
--- a/sha1_file.c
+++ b/sha1_file.c

@@ -1564,6 +1564,36 @@ int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned long ma
      return git_inflate(stream, 0);
  }

+static int unpack_sha1_header_literally(git_zstream *stream, unsigned char *map,
+                    unsigned long mapsize,
+                    struct strbuf *header)
+{
+...
+}

Looks suspiciously familiar...

Yes, you suggested it.
It has a similar structure to unpack_sha1_header().
Anything wrong with it?

quoted

+int sha1_object_info_literally(const unsigned char *sha1)
+{
+    enum object_type type;
+    struct strbuf sb = STRBUF_INIT;
+    struct object_info oi = {NULL};
+
+    oi.typename = &sb;
+    oi.typep = &type;
+    if (sha1_object_info_extended(sha1, &oi, LOOKUP_LITERALLY) < 0)
+        return -1;
+    if (*oi.typep > 0)
+        printf("%s\n", typename(*oi.typep));
+    else
+        printf("%s\n", oi.typename->buf);
+    strbuf_release(oi.typename);
+    return 0;
+}
+

NAK.

Please don't add end-user facing final output to sha1_file.c;
instead have the caller use a helper function like this one to
extract necessary information and perform the end-user interaction
there.

Ok, will do that.