Re: [PATCH 5/7] object.c: add a utility function for "expected type X, got Y"
flat view
From: Taylor Blau <hidden>
Date: 2021-03-08 21:00:56
On Mon, Mar 08, 2021 at 09:04:24PM +0100, Ævar Arnfjörð Bjarmason wrote:
quoted hunk ↗ jump to hunk
Refactor various "Object X is not Y" error messages so that they use the same message as the long-standing object_as_type() error message. Now we'll consistently report e.g. that we got a commit when we expected a tag, not just that the object is not a tag. Signed-off-by: Ævar Arnfjörð Bjarmason <redacted> --- builtin/index-pack.c | 9 +++------ commit.c | 10 ++++------ object.c | 34 +++++++++++++++++++++++++++++++++- object.h | 5 +++++ tree.c | 7 ++++--- 5 files changed, 49 insertions(+), 16 deletions(-)diff --git a/builtin/index-pack.c b/builtin/index-pack.c index 253cfb07fbd..d9082831bb2 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c@@ -217,8 +217,8 @@ static int mark_link(struct object *obj, int type, void *data, struct fsck_optio if (!obj) return -1; - if (type != OBJ_ANY && obj->type != type) - die(_("object type mismatch at %s"), oid_to_hex(&obj->oid)); + if (type != OBJ_ANY) + oid_is_type_or_die(&obj->oid, obj->type, &type);
Nice. This is definitely an improvement over the existing code.
+static const char *oid_is_a_X_not_a_Y = N_("object %s is a %s, not a %s");
+This name is a little verbose. I'm nitpicking, of course, but would you consider "object_type_mismatch_msg" instead?
quoted hunk ↗ jump to hunk
const char *type_name(unsigned int type) { if (type >= ARRAY_SIZE(object_type_strings))@@ -159,6 +161,36 @@ void *create_object(struct repository *r, const struct object_id *oid, void *o) return obj; } +static int oid_is_type_or(const struct object_id *oid, + enum object_type want, + enum object_type type, + int err) +{ + if (want == type) + return 0; + if (err) + return error(_(oid_is_a_X_not_a_Y), + oid_to_hex(oid), type_name(type), + type_name(want)); + else + die(_(oid_is_a_X_not_a_Y), oid_to_hex(oid), + type_name(type), type_name(want)); +} + +void oid_is_type_or_die(const struct object_id *oid, + enum object_type want, + enum object_type *type) +{ + oid_is_type_or(oid, want, *type, 0); +} + +int oid_is_type_or_error(const struct object_id *oid, + enum object_type want, + enum object_type *type) +{ + return oid_is_type_or(oid, want, *type, 1); +} +
I'm not sure that this oid_is_type_or() is really doing all that much. It allows you to share the 'if (want == type)' conditional between the other two functions, but the rest of the function is a conditional itself that behaves differently depending on whether you called the _die() or _error() variant. Why not duplicate the 'if (want == type)' between the two functions, and then remove 'oid_is_type_or()' altogether? Thanks, Taylor