Re: [PATCH v7 4/6] verify-tag: prepare verify_tag for libification
Eric Sunshinewrites: > I'd have probably called this "display_name", but then I suppose it > suffers the same issue Junio mentioned previously about it sounding > like a boolean. Anyhow, as long as Junio is happy with it, that's what > matters. No ;-) I am just trying to help people come up with patches in a better shape. Somehow "display name" does not bother me as much as "report name" did. name_to_report may be a mouthful, but it conveys what it is clearly, and I think it is good enough. > > This version of the patch is nicely improved. One nit below. > >> Signed-off-by: Santiago Torres >> --- >> diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c >> @@ -80,6 +83,8 @@ int cmd_verify_tag(int argc, const char **argv, const char >> *prefix) >> { >> int i = 1, verbose = 0, had_error = 0; >> unsigned flags = 0; >> + unsigned char sha1[20]; >> + const char *name; > > Mentioned previously[1]: These two declarations could be moved inside > the while-loop scope (below). > > [1]: http://article.gmane.org/gmane.comp.version-control.git/291813 Yup. > >> const struct option verify_tag_options[] = { >> OPT__VERBOSE(, N_("print tag contents")), >> OPT_BIT(0, "raw", , N_("print raw gpg status output"), >> GPG_VERIFY_RAW), >> @@ -96,8 +101,12 @@ int cmd_verify_tag(int argc, const char **argv, const >> char *prefix) >> if (verbose) >> flags |= GPG_VERIFY_VERBOSE; >> >> - while (i < argc) >> - if (verify_tag(argv[i++], flags)) >> + while (i < argc) { >> + name = argv[i++]; >> + if (get_sha1(name, sha1)) >> + had_error = !!error("tag '%s' not found.", name); >> + else if (verify_tag(sha1, name, flags)) >> had_error = 1; >> + } >> return had_error; >> } >> -- >> 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 4/6] verify-tag: prepare verify_tag for libification
On Tue, Apr 19, 2016 at 1:47 PM,wrote: > The current interface of verify_tag() resolves reference names to SHA1, > however, the plan is to make this functionality public and the current > interface is cumbersome for callers: they are expected to supply the > textual representation of a sha1/refname. In many cases, this requires > them to turn the sha1 to hex representation, just to be converted back > inside verify_tag. > > Add a SHA1 parameter to use instead of the name parameter, and rename > the name parameter to "name_to_report" for reporting purposes only. I'd have probably called this "display_name", but then I suppose it suffers the same issue Junio mentioned previously about it sounding like a boolean. Anyhow, as long as Junio is happy with it, that's what matters. This version of the patch is nicely improved. One nit below. > Signed-off-by: Santiago Torres > --- > diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c > @@ -80,6 +83,8 @@ int cmd_verify_tag(int argc, const char **argv, const char > *prefix) > { > int i = 1, verbose = 0, had_error = 0; > unsigned flags = 0; > + unsigned char sha1[20]; > + const char *name; Mentioned previously[1]: These two declarations could be moved inside the while-loop scope (below). [1]: http://article.gmane.org/gmane.comp.version-control.git/291813 > const struct option verify_tag_options[] = { > OPT__VERBOSE(, N_("print tag contents")), > OPT_BIT(0, "raw", , N_("print raw gpg status output"), > GPG_VERIFY_RAW), > @@ -96,8 +101,12 @@ int cmd_verify_tag(int argc, const char **argv, const > char *prefix) > if (verbose) > flags |= GPG_VERIFY_VERBOSE; > > - while (i < argc) > - if (verify_tag(argv[i++], flags)) > + while (i < argc) { > + name = argv[i++]; > + if (get_sha1(name, sha1)) > + had_error = !!error("tag '%s' not found.", name); > + else if (verify_tag(sha1, name, flags)) > had_error = 1; > + } > return had_error; > } > -- > 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 4/6] verify-tag: prepare verify_tag for libification
From: Santiago TorresThe current interface of verify_tag() resolves reference names to SHA1, however, the plan is to make this functionality public and the current interface is cumbersome for callers: they are expected to supply the textual representation of a sha1/refname. In many cases, this requires them to turn the sha1 to hex representation, just to be converted back inside verify_tag. Add a SHA1 parameter to use instead of the name parameter, and rename the name parameter to "name_to_report" for reporting purposes only. Helped-by: Junio C Hamano Signed-off-by: Santiago Torres --- builtin/verify-tag.c | 27 ++- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index fa26e40..01e956f 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -42,25 +42,28 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags) return ret; } -static int verify_tag(const char *name, unsigned flags) +static int verify_tag(const unsigned char *sha1, const char *name_to_report, + unsigned flags) { enum object_type type; - unsigned char sha1[20]; char *buf; unsigned long size; int ret; - if (get_sha1(name, sha1)) - return error("tag '%s' not found.", name); - type = sha1_object_info(sha1, NULL); if (type != OBJ_TAG) return error("%s: cannot verify a non-tag object of type %s.", - name, typename(type)); + name_to_report ? + name_to_report : + find_unique_abbrev(sha1, DEFAULT_ABBREV), + typename(type)); buf = read_sha1_file(sha1, , ); if (!buf) - return error("%s: unable to read file.", name); + return error("%s: unable to read file.", + name_to_report ? + name_to_report : + find_unique_abbrev(sha1, DEFAULT_ABBREV)); ret = run_gpg_verify(buf, size, flags); @@ -80,6 +83,8 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) { int i = 1, verbose = 0, had_error = 0; unsigned flags = 0; + unsigned char sha1[20]; + const char *name; const struct option verify_tag_options[] = { OPT__VERBOSE(, N_("print tag contents")), OPT_BIT(0, "raw", , N_("print raw gpg status output"), GPG_VERIFY_RAW), @@ -96,8 +101,12 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) if (verbose) flags |= GPG_VERIFY_VERBOSE; - while (i < argc) - if (verify_tag(argv[i++], flags)) + while (i < argc) { + name = argv[i++]; + if (get_sha1(name, sha1)) + had_error = !!error("tag '%s' not found.", name); + else if (verify_tag(sha1, name, flags)) had_error = 1; + } return had_error; } -- 2.8.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html