Re: [PATCH v2 10/10] tag: don't misreport type of tagged objects in errors
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 31, 2021, 18:05 UTC
- Message-ID
- <xmqqim578agu.fsf@gitster.g>
- In-Reply-To
- <YGRWqX+qF+Rtqr20@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 8 quoted lines
> I also think the manual handling of OBJ_NONE in each lookup_* function > is confusing. They all call object_as_type() because the point of that > function is both to type-check the struct and to convert it away from > OBJ_NONE. > > If we handled this error there, then I think it would be much more > natural, because we'd have already covered the OBJ_NONE case, and > because it's already the place we're emitting the existing error. E.g.:
This makes quite a lot of sense. If presented with this simple change and 10-patch series at the same time and are told that the goal of the changes were more or less the same, I'd pick this one 100% of the time.
Show 36 quoted lines
> diff --git a/object.c b/object.c
> index 2c32691dc4..e6345541f7 100644
> --- a/object.c
> +++ b/object.c
> @@ -157,7 +157,7 @@ void *create_object(struct repository *r, const struct object_id *oid, void *o)
> return obj;
> }
>
> -void *object_as_type(struct object *obj, enum object_type type, int quiet)
> +void *object_as_type(struct object *obj, enum object_type type, unsigned flags)
> {
> if (obj->type == type)
> return obj;
> @@ -169,10 +169,16 @@ void *object_as_type(struct object *obj, enum object_type type, int quiet)
> return obj;
> }
> else {
> - if (!quiet)
> - error(_("object %s is a %s, not a %s"),
> - oid_to_hex(&obj->oid),
> - type_name(obj->type), type_name(type));
> + if (!(flags & OBJECT_AS_TYPE_QUIET)) {
> + if (flags & OBJECT_AS_TYPE_EXPECT_PARSED)
> + error(_("object %s is a %s, but was referred to as a %s"),
> + oid_to_hex(&obj->oid), type_name(obj->type),
> + type_name(type));
> + else
> + error(_("object %s referred to as both a %s and a %s"),
> + oid_to_hex(&obj->oid),
> + type_name(obj->type), type_name(type));
> + }
> return NULL;
> }
> }
>
> -Peff