| Message ID | patch-3.8-22e7d9a3db-20210420T133218Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | object.c: add and use "is expected" utility function + object_as_type() use | expand |
> +char* oid_is_type_or_die_msg(const struct object_id *oid, > + enum object_type want, > + enum object_type *type) > +{ > + struct strbuf sb = STRBUF_INIT; > + if (want == *type) > + BUG("call this just to get the message!"); > + strbuf_addf(&sb, _(object_type_mismatch_msg), oid_to_hex(oid), > + type_name(*type), type_name(want)); > + return strbuf_detach(&sb, NULL); > +} It would be more convenient for the caller if this function also checks want vs type and returns NULL if they match. That would also be more consistent with the other functions, and the caller wouldn't need to remember that this function only works if want and type are different.
On 2021.04.20 15:36, Ævar Arnfjörð Bjarmason wrote: > diff --git a/object.c b/object.c > index 9e06c0ee92..0f07f976fb 100644 > --- a/object.c > +++ b/object.c > @@ -176,6 +176,18 @@ int oid_is_type_or_error(const struct object_id *oid, > type_name(want)); > } > > +char* oid_is_type_or_die_msg(const struct object_id *oid, It's kind of a nitpick, but I found the function name to be confusing. It sounds like you're going to die with a custom message. Maybe something like "get_oid_type_mismatch_msg()" would be more straightforward.
Josh Steadmon <steadmon@google.com> writes: > On 2021.04.20 15:36, Ævar Arnfjörð Bjarmason wrote: >> diff --git a/object.c b/object.c >> index 9e06c0ee92..0f07f976fb 100644 >> --- a/object.c >> +++ b/object.c >> @@ -176,6 +176,18 @@ int oid_is_type_or_error(const struct object_id *oid, >> type_name(want)); >> } >> >> +char* oid_is_type_or_die_msg(const struct object_id *oid, > > It's kind of a nitpick, but I found the function name to be confusing. > It sounds like you're going to die with a custom message. Maybe > something like "get_oid_type_mismatch_msg()" would be more > straightforward. Yeah, in an older round I found this function's name was confusing, too. Also, there is a style (in our codebase, asterisk to signal the pointer-ness sticks to the identifier, not to the type name). Thanks.
diff --git a/merge-recursive.c b/merge-recursive.c index b952106203..c74239544f 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -2999,9 +2999,11 @@ static int read_oid_strbuf(struct merge_options *opt, if (!buf) return err(opt, _("cannot read object %s"), oid_to_hex(oid)); if (type != OBJ_BLOB) { - const char* msg = oid_is_type_or_die_msg(oid, OBJ_BLOB, &type); + char *msg = oid_is_type_or_die_msg(oid, OBJ_BLOB, &type); + int ret = err(opt, msg); free(buf); - return err(opt, _("object %s is not a blob"), oid_to_hex(oid)); + free(msg); + return ret; } strbuf_attach(dst, buf, size, size + 1); return 0; diff --git a/object.c b/object.c index 9e06c0ee92..0f07f976fb 100644 --- a/object.c +++ b/object.c @@ -176,6 +176,18 @@ int oid_is_type_or_error(const struct object_id *oid, type_name(want)); } +char* oid_is_type_or_die_msg(const struct object_id *oid, + enum object_type want, + enum object_type *type) +{ + struct strbuf sb = STRBUF_INIT; + if (want == *type) + BUG("call this just to get the message!"); + strbuf_addf(&sb, _(object_type_mismatch_msg), oid_to_hex(oid), + type_name(*type), type_name(want)); + return strbuf_detach(&sb, NULL); +} + void *object_as_type(struct object *obj, enum object_type type, int quiet) { if (obj->type == type) diff --git a/object.h b/object.h index f8609a8518..7ae6407598 100644 --- a/object.h +++ b/object.h @@ -127,6 +127,9 @@ void oid_is_type_or_die(const struct object_id *oid, enum object_type want, enum object_type *type); int oid_is_type_or_error(const struct object_id *oid, enum object_type want, enum object_type *type); +char* oid_is_type_or_die_msg(const struct object_id *oid, + enum object_type want, + enum object_type *type); /* * Returns the object, having parsed it to find out what it is.
Add a oid_is_type_or_die_msg() function to go with the "error" and "die" forms for emitting "expected type X, got Y" messages. This is useful for callers that want the message itself as a char *. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- merge-recursive.c | 6 ++++-- object.c | 12 ++++++++++++ object.h | 3 +++ 3 files changed, 19 insertions(+), 2 deletions(-)