Thread (103 messages) 103 messages, 6 authors, 2021-04-22

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help