On Thu, Oct 07, 2021 at 01:56:26PM -0700, Junio C Hamano wrote: > Jeff King <peff@xxxxxxxx> writes: > > > diff --git a/builtin/cat-file.c b/builtin/cat-file.c > > index b533935d5c..219ff5628d 100644 > > --- a/builtin/cat-file.c > > +++ b/builtin/cat-file.c > > @@ -358,15 +358,26 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d > > The two new parameters might deserve a comment in front of the > function as to the calling convention (namely, offset can be any > garbage when the caller signals "unknown" with pack==NULL). Yes, though I'd hope everyone would just pass NULL/0. :) I wasn't too worried about it as this is a static-local function, but perhaps it's worth squashing the patch below into the final patch. The "we may also look at the oid" thing is perhaps a bit subtle. It happens because we may still print %(objectname), of course, but also the current code will use read_object_file(oid) when printing the actual contents. -Peff --- diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 219ff5628d..86fc03242b 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -355,6 +355,11 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d } } +/* + * If "pack" is non-NULL, then "offset" is the byte offset within the pack from + * which the object may be accessed (though note that we may also rely on + * data->oid, too). If "pack" is NULL, then offset is ignored. + */ static void batch_object_write(const char *obj_name, struct strbuf *scratch, struct batch_options *opt,
