On Sat, 17 Nov 2007, Jeff King wrote:
> diff --git a/builtin-send-pack.c b/builtin-send-pack.c
> index 418925e..90ca2d3 100644
> --- a/builtin-send-pack.c
> +++ b/builtin-send-pack.c
> @@ -218,15 +219,105 @@ static const char *prettify_ref(const char *name)
>
> #define SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)
>
> +static void print_ref_status(char flag, const char *summary, struct ref *to, struct ref *from, const char *msg)
Isn't "from" always "to->peer_ref"? It'd be nice to make this function
unable to print something different from what we actually did. (Actually
it might be "to->deletion ? NULL : to->peer_ref", but that would also be
better to have as an explicit feature of how you display "to", rather than
implicit in the set of callers.
> +static const char *status_abbrev(unsigned char sha1[20])
> +{
> + const char *abbrev;
> + abbrev = find_unique_abbrev(sha1, DEFAULT_ABBREV);
> + return abbrev ? abbrev : sha1_to_hex(sha1);
> +}
Maybe we should have a find_unique_abbrev()-like function that doesn't
mind if the requested object doesn't exist?
> +
> +static void print_ok_ref_status(struct ref *ref)
> +{
> + if (ref->deletion)
> + print_ref_status('-', "[deleted]", ref, NULL, NULL);
> + else if (is_null_sha1(ref->old_sha1))
> + print_ref_status('*',
> + (!prefixcmp(ref->name, "refs/tags/") ? "[new tag]" :
> + "[new branch]"),
> + ref, ref->peer_ref, NULL);
> + else {
> + char quickref[83];
Shouldn't this be 40 + 3 + 40 + 1?
> + char type;
> + const char *msg;
> +
> + strcpy(quickref, status_abbrev(ref->old_sha1));
> + if (ref->nonfastforward) {
> + strcat(quickref, "...");
> + type = '+';
> + msg = " (forced update)";
> + }
> + else {
Coding style, IIRC.
Regardless of these nits, it all looks good to me.
Acked-by: Daniel Barkalow <barkalow@xxxxxxxxxxxx>
-Daniel
*This .sig left intentionally blank*
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
