Re: [PATCH] add --summary option to git-push and git-fetch
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:47:01
Possibly related (same subject, not in this thread)
- 2016-06-15 · [PATCH] add --summary option to git-push and git-fetch · Larry D'Anna <hidden>
Larry D'Anna [off-list ref] writes:
--summary will cause git-push to output a one-line of each commit pushed.
--summary=n will display at most n commits for each ref pushed.
$ git push --dry-run --summary origin :
To /home/larry/gitsandbox/a
80f0e50..5593a38 master -> master
> 5593a38 foo
> 81c03f8 bar
Fetch works the same way.
Signed-off-by: Larry D'Anna <redacted>
---With this rewrite not to call cmd_log() directly, it looks much better than the previous round. It allows us more freedom to do things slightly differently than the stock cmd_log() lets us, such as giving "..." after "n" commits by default, much like fmt-merge-msg does.
quoted hunk
diff --git a/builtin-fetch.c b/builtin-fetch.c index cd5eb9a..b79d870 100644 --- a/builtin-fetch.c +++ b/builtin-fetch.c@@ -29,6 +29,7 @@ static const char *depth; static const char *upload_pack; static struct strbuf default_rla = STRBUF_INIT; static struct transport *transport; +static int summary = 0;
Don't initialize statics with "= 0;". BSS will take care of it.
quoted hunk
static struct option builtin_fetch_options[] = { OPT__VERBOSITY(&verbosity),@@ -47,6 +48,9 @@ static struct option builtin_fetch_options[] = { "allow updating of HEAD ref"), OPT_STRING(0, "depth", &depth, "DEPTH", "deepen history of shallow clone"), + { OPTION_INTEGER, 0, "summary", &summary, "n", "print a summary of [at most n] fetched commits", + PARSE_OPT_OPTARG, NULL, -1 + }, OPT_END() };
I think I'd prefer some reasonable default instead of making it unlimited, much like how fmt-merge-msg does. We might want to make this configurable (I think fmt-merge-msg uses a hardcoded 20), and perhaps even use them the same configuration variable (summary.length or something).
quoted hunk
@@ -260,11 +265,12 @@ static int update_local_ref(struct ref *ref, sprintf(display, "%c %-*s %-*s -> %s%s", r ? '!' : '*', SUMMARY_WIDTH, what, REFCOL_WIDTH, remote, pretty_ref, r ? " (unable to update local ref)" : ""); + if (!r) + strcpy (quickref, find_unique_abbrev(ref->new_sha1, DEFAULT_ABBREV));
We do not leave extra whitespace between function name and open parenthesis (you have other instances of this style violation); on the other hand, we do keep one whitespace after keywords like if, while, and for (this is just fyi; I do not think I saw any violation of the latter in the patch).
quoted hunk
diff --git a/builtin-log.c b/builtin-log.c index 44f9a27..cc4dc0a 100644 --- a/builtin-log.c +++ b/builtin-log.c@@ -1293,3 +1293,38 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)... +void print_summary_for_push_or_fetch (const char *quickref, int limit) +{ +... + temp = stdout; + stdout = stderr;
Is this even a valid C? stdout and friends are described in POSIX.1 as "Normally, there are three open streams with constant pointers declared in the <stdio.h> header and associated with the standard open files." At least Solaris 11 Clib headers does not seem to like it. I do not know about Windows.