From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 23 Oct 2025 06:00:03 GMT
Subject: Re: [PATCH v3 14/14] ref-filter: parse objects on demand
Message-ID: <aPnEYzHvY9u8tfw6@pks.im>
In-Reply-To: <xmqqtszryn10.fsf@gitster.g>

On Wed, Oct 22, 2025 at 08:27:39AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > +static int get_or_parse_object(struct expand_data *data, const char *refname,
> > +			       struct object **object, struct strbuf *err, int *eaten)
> > +{
> > +	if (!data->maybe_object) {
> > +		data->maybe_object = parse_object_buffer(the_repository, &data->oid, data->type,
> > +							 data->size, data->content, eaten);
> > +		if (!data->maybe_object)
> > +			return strbuf_addf_ret(err, -1, _("parse_object_buffer failed on %s for %s"),
> > +					       oid_to_hex(&data->oid), refname);
> > +	}
> > +
> > +	*object = data->maybe_object;
> > +	return 0;
> > +}
> 
> I wonder if the calling convention for this function can use further
> ergonomic improvements, after seeing that ...
> 
> > +		if (!tag) {
> > +			struct object *object;
> > +
> > +			ret = get_or_parse_object(data, refname, &object, err, eaten);
> > +			if (ret < 0)
> > +				return ret;
> > +
> > +			tag = (struct tag *) object;
> > +		}
> 
> ... this pattern is repeated three times in the patch.  If the
> helper cannot legitimately return 0 (success) while stuffing NULL to
> the object, then something like this
> 
> 		if (!tag) {
> 			tag = (struct tag *) get_or_parse(data,	refname, err, eaten);
> 			if (!tag)
> 				return -1;
> 		}
> 
> ought to be sufficient for this caller and the other two.  I also
> wonder if this can further be simplified, but stopping at the above
> is fine to me.

Good suggestion indeed. Let me use this style and send another version.
Thanks!

Patrick