Hi, This is the second version of my patch series to refactor the code that prints reference updates during git-fetch(1). Its aim is it to make the code more self-contained so that it can be easily amended for a new machine-parseable format. Changes compared to v1: - I've dropped the first patch to rename the preexisting `display` buffer variable. Instead, I'm now just picking the longer `display_state` variable name for the newly introduced structure in order to avoid naming conflicts. - I've touched up the commit messages of patches 4-6 to hopefully clarify their intent a little bit better. - I've dropped patch 7/8 that unified the logic to calculate the summary width. Even though it fixes a bug in one case, Jonathan correctly pointed out that it's weird in the case where there are only reference deletions without any reference updates. I may have another go in a separate patch series after the dust has settled. Thanks for the feedback so far! Patrick Patrick Steinhardt (6): fetch: move reference width calculation into `display_state` fetch: move output format into `display_state` fetch: pass the full local reference name to `format_display` fetch: centralize handling of per-reference format fetch: centralize logic to print remote URL fetch: centralize printing of reference updates builtin/fetch.c | 267 +++++++++++++++++++++++++----------------------- 1 file changed, 138 insertions(+), 129 deletions(-) Range-diff against v1: 1: 692206f7ff < -: ---------- fetch: rename `display` buffer to avoid name conflict 2: aa792b12a4 ! 1: ce2c4b61ae fetch: move reference width calculation into `display_state` @@ builtin/fetch.c: static void adjust_refcol_width(const struct ref *ref) } -static void prepare_format_display(struct ref *ref_map) -+static void display_state_init(struct display_state *display, struct ref *ref_map) ++static void display_state_init(struct display_state *display_state, struct ref *ref_map) { struct ref *rm; const char *format = "full"; -+ memset(display, 0, sizeof(*display)); ++ memset(display_state, 0, sizeof(*display_state)); + if (verbosity < 0) return; @@ builtin/fetch.c: static void prepare_format_display(struct ref *ref_map) die(_("invalid value for '%s': '%s'"), "fetch.output", format); -+ display->refcol_width = 10; ++ display_state->refcol_width = 10; for (rm = ref_map; rm; rm = rm->next) { + int width; + @@ builtin/fetch.c: static void prepare_format_display(struct ref *ref_map) + * appear on the left hand side of '->' and shrink the column + * back. + */ -+ if (display->refcol_width < width) -+ display->refcol_width = width; ++ if (display_state->refcol_width < width) ++ display_state->refcol_width = width; } } --static void print_remote_to_local(struct strbuf *display_buffer, -+static void print_remote_to_local(struct display_state *display, +-static void print_remote_to_local(struct strbuf *display, ++static void print_remote_to_local(struct display_state *display_state, + struct strbuf *display_buffer, const char *remote, const char *local) { -- strbuf_addf(display_buffer, "%-*s -> %s", refcol_width, remote, local); -+ strbuf_addf(display_buffer, "%-*s -> %s", display->refcol_width, remote, local); +- strbuf_addf(display, "%-*s -> %s", refcol_width, remote, local); ++ strbuf_addf(display_buffer, "%-*s -> %s", display_state->refcol_width, remote, local); } static int find_and_replace(struct strbuf *haystack, @@ builtin/fetch.c: static int find_and_replace(struct strbuf *haystack, return 1; } --static void print_compact(struct strbuf *display_buffer, -+static void print_compact(struct display_state *display, struct strbuf *display_buffer, +-static void print_compact(struct strbuf *display, ++static void print_compact(struct display_state *display_state, struct strbuf *display_buffer, const char *remote, const char *local) { struct strbuf r = STRBUF_INIT; struct strbuf l = STRBUF_INIT; if (!strcmp(remote, local)) { -- strbuf_addf(display_buffer, "%-*s -> *", refcol_width, remote); -+ strbuf_addf(display_buffer, "%-*s -> *", display->refcol_width, remote); +- strbuf_addf(display, "%-*s -> *", refcol_width, remote); ++ strbuf_addf(display_buffer, "%-*s -> *", display_state->refcol_width, remote); return; } -@@ builtin/fetch.c: static void print_compact(struct strbuf *display_buffer, +@@ builtin/fetch.c: static void print_compact(struct strbuf *display, if (!find_and_replace(&r, local, "*")) find_and_replace(&l, remote, "*"); -- print_remote_to_local(display_buffer, r.buf, l.buf); -+ print_remote_to_local(display, display_buffer, r.buf, l.buf); +- print_remote_to_local(display, r.buf, l.buf); ++ print_remote_to_local(display_state, display_buffer, r.buf, l.buf); strbuf_release(&r); strbuf_release(&l); } --static void format_display(struct strbuf *display_buffer, char code, -+static void format_display(struct display_state *display, +-static void format_display(struct strbuf *display, char code, ++static void format_display(struct display_state *display_state, + struct strbuf *display_buffer, char code, const char *summary, const char *error, const char *remote, const char *local, int summary_width) -@@ builtin/fetch.c: static void format_display(struct strbuf *display_buffer, char code, +@@ builtin/fetch.c: static void format_display(struct strbuf *display, char code, - strbuf_addf(display_buffer, "%c %-*s ", code, width, summary); + width = (summary_width + strlen(summary) - gettext_width(summary)); + +- strbuf_addf(display, "%c %-*s ", code, width, summary); ++ strbuf_addf(display_buffer, "%c %-*s ", code, width, summary); if (!compact_format) -- print_remote_to_local(display_buffer, remote, local); -+ print_remote_to_local(display, display_buffer, remote, local); +- print_remote_to_local(display, remote, local); ++ print_remote_to_local(display_state, display_buffer, remote, local); else -- print_compact(display_buffer, remote, local); -+ print_compact(display, display_buffer, remote, local); +- print_compact(display, remote, local); ++ print_compact(display_state, display_buffer, remote, local); if (error) - strbuf_addf(display_buffer, " (%s)", error); +- strbuf_addf(display, " (%s)", error); ++ strbuf_addf(display_buffer, " (%s)", error); } static int update_local_ref(struct ref *ref, struct ref_transaction *transaction, -+ struct display_state *display, ++ struct display_state *display_state, const char *remote, const struct ref *remote_ref, - struct strbuf *display_buffer, int summary_width) + struct strbuf *display, int summary_width) { @@ builtin/fetch.c: static int update_local_ref(struct ref *ref, if (oideq(&ref->old_oid, &ref->new_oid)) { if (verbosity > 0) -- format_display(display_buffer, '=', _("[up to date]"), NULL, -+ format_display(display, display_buffer, '=', _("[up to date]"), NULL, +- format_display(display, '=', _("[up to date]"), NULL, ++ format_display(display_state, display, '=', _("[up to date]"), NULL, remote, pretty_ref, summary_width); return 0; } @@ builtin/fetch.c: static int update_local_ref(struct ref *ref, * If this is the head, and it's not okay to update * the head, and the old value of the head isn't empty... */ -- format_display(display_buffer, '!', _("[rejected]"), -+ format_display(display, display_buffer, '!', _("[rejected]"), +- format_display(display, '!', _("[rejected]"), ++ format_display(display_state, display, '!', _("[rejected]"), _("can't fetch into checked-out branch"), remote, pretty_ref, summary_width); return 1; @@ builtin/fetch.c: static int update_local_ref(struct ref *ref, if (force || ref->force) { int r; r = s_update_ref("updating tag", ref, transaction, 0); -- format_display(display_buffer, r ? '!' : 't', _("[tag update]"), -+ format_display(display, display_buffer, r ? '!' : 't', _("[tag update]"), +- format_display(display, r ? '!' : 't', _("[tag update]"), ++ format_display(display_state, display, r ? '!' : 't', _("[tag update]"), r ? _("unable to update local ref") : NULL, remote, pretty_ref, summary_width); return r; } else { -- format_display(display_buffer, '!', _("[rejected]"), _("would clobber existing tag"), -+ format_display(display, display_buffer, '!', _("[rejected]"), +- format_display(display, '!', _("[rejected]"), _("would clobber existing tag"), ++ format_display(display_state, display, '!', _("[rejected]"), + _("would clobber existing tag"), remote, pretty_ref, summary_width); return 1; @@ builtin/fetch.c: static int update_local_ref(struct ref *ref, } r = s_update_ref(msg, ref, transaction, 0); -- format_display(display_buffer, r ? '!' : '*', what, -+ format_display(display, display_buffer, r ? '!' : '*', what, +- format_display(display, r ? '!' : '*', what, ++ format_display(display_state, display, r ? '!' : '*', what, r ? _("unable to update local ref") : NULL, remote, pretty_ref, summary_width); return r; @@ builtin/fetch.c: static int update_local_ref(struct ref *ref, strbuf_addstr(&quickref, ".."); strbuf_add_unique_abbrev(&quickref, &ref->new_oid, DEFAULT_ABBREV); r = s_update_ref("fast-forward", ref, transaction, 1); -- format_display(display_buffer, r ? '!' : ' ', quickref.buf, -+ format_display(display, display_buffer, r ? '!' : ' ', quickref.buf, +- format_display(display, r ? '!' : ' ', quickref.buf, ++ format_display(display_state, display, r ? '!' : ' ', quickref.buf, r ? _("unable to update local ref") : NULL, remote, pretty_ref, summary_width); strbuf_release(&quickref); @@ builtin/fetch.c: static int update_local_ref(struct ref *ref, strbuf_addstr(&quickref, "..."); strbuf_add_unique_abbrev(&quickref, &ref->new_oid, DEFAULT_ABBREV); r = s_update_ref("forced-update", ref, transaction, 1); -- format_display(display_buffer, r ? '!' : '+', quickref.buf, -+ format_display(display, display_buffer, r ? '!' : '+', quickref.buf, +- format_display(display, r ? '!' : '+', quickref.buf, ++ format_display(display_state, display, r ? '!' : '+', quickref.buf, r ? _("unable to update local ref") : _("forced update"), remote, pretty_ref, summary_width); strbuf_release(&quickref); return r; } else { -- format_display(display_buffer, '!', _("[rejected]"), _("non-fast-forward"), -+ format_display(display, display_buffer, '!', _("[rejected]"), _("non-fast-forward"), +- format_display(display, '!', _("[rejected]"), _("non-fast-forward"), ++ format_display(display_state, display, '!', _("[rejected]"), _("non-fast-forward"), remote, pretty_ref, summary_width); return 1; } @@ builtin/fetch.c: N_("it took %.2f seconds to check forced updates; you can use\n "to avoid this check\n"); -static int store_updated_refs(const char *raw_url, const char *remote_name, -+static int store_updated_refs(struct display_state *display, ++static int store_updated_refs(struct display_state *display_state, + const char *raw_url, const char *remote_name, int connectivity_checked, struct ref_transaction *transaction, struct ref *ref_map, @@ builtin/fetch.c: static int store_updated_refs(const char *raw_url, const char * strbuf_reset(¬e); if (ref) { - rc |= update_local_ref(ref, transaction, what, -+ rc |= update_local_ref(ref, transaction, display, what, ++ rc |= update_local_ref(ref, transaction, display_state, what, rm, ¬e, summary_width); free(ref); } else if (write_fetch_head || dry_run) { @@ builtin/fetch.c: static int store_updated_refs(const char *raw_url, const char * * is set). */ - format_display(¬e, '*', -+ format_display(display, ¬e, '*', ++ format_display(display_state, ¬e, '*', *kind ? kind : "branch", NULL, *what ? what : "HEAD", "FETCH_HEAD", summary_width); @@ builtin/fetch.c: static int check_exist_and_connected(struct ref *ref_map) } -static int fetch_and_consume_refs(struct transport *transport, -+static int fetch_and_consume_refs(struct display_state *display, ++static int fetch_and_consume_refs(struct display_state *display_state, + struct transport *transport, struct ref_transaction *transaction, struct ref *ref_map, @@ builtin/fetch.c: static int fetch_and_consume_refs(struct transport *transport, trace2_region_enter("fetch", "consume_refs", the_repository); - ret = store_updated_refs(transport->url, transport->remote->name, -+ ret = store_updated_refs(display, transport->url, transport->remote->name, ++ ret = store_updated_refs(display_state, transport->url, transport->remote->name, connectivity_checked, transaction, ref_map, fetch_head); trace2_region_leave("fetch", "consume_refs", the_repository); @@ builtin/fetch.c: static int fetch_and_consume_refs(struct transport *transport, } -static int prune_refs(struct refspec *rs, -+static int prune_refs(struct display_state *display, ++static int prune_refs(struct display_state *display_state, + struct refspec *rs, struct ref_transaction *transaction, struct ref *ref_map, @@ builtin/fetch.c: static int prune_refs(struct refspec *rs, shown_url = 1; } - format_display(&sb, '-', _("[deleted]"), NULL, -+ format_display(display, &sb, '-', _("[deleted]"), NULL, ++ format_display(display_state, &sb, '-', _("[deleted]"), NULL, _("(none)"), prettify_refname(ref->name), summary_width); fprintf(stderr, " %s\n",sb.buf); @@ builtin/fetch.c: static struct transport *prepare_transport(struct remote *remot } -static int backfill_tags(struct transport *transport, -+static int backfill_tags(struct display_state *display, ++static int backfill_tags(struct display_state *display_state, + struct transport *transport, struct ref_transaction *transaction, struct ref *ref_map, @@ builtin/fetch.c: static int backfill_tags(struct transport *transport, transport_set_option(transport, TRANS_OPT_DEPTH, "0"); transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL); - retcode = fetch_and_consume_refs(transport, transaction, ref_map, fetch_head); -+ retcode = fetch_and_consume_refs(display, transport, transaction, ref_map, fetch_head); ++ retcode = fetch_and_consume_refs(display_state, transport, transaction, ref_map, fetch_head); if (gsecondary) { transport_disconnect(gsecondary); @@ builtin/fetch.c: static int do_fetch(struct transport *transport, { struct ref_transaction *transaction = NULL; struct ref *ref_map = NULL; -+ struct display_state display; ++ struct display_state display_state; int autotags = (transport->remote->fetch_tags == 1); int retcode = 0; const struct ref *remote_refs; @@ builtin/fetch.c: static int do_fetch(struct transport *transport, if (retcode) goto cleanup; -+ display_state_init(&display, ref_map); ++ display_state_init(&display_state, ref_map); + if (atomic_fetch) { transaction = ref_transaction_begin(&err); @@ builtin/fetch.c: static int do_fetch(struct transport *transport, */ if (rs->nr) { - retcode = prune_refs(rs, transaction, ref_map, transport->url); -+ retcode = prune_refs(&display, rs, transaction, ref_map, transport->url); ++ retcode = prune_refs(&display_state, rs, transaction, ref_map, transport->url); } else { - retcode = prune_refs(&transport->remote->fetch, -+ retcode = prune_refs(&display, &transport->remote->fetch, ++ retcode = prune_refs(&display_state, &transport->remote->fetch, transaction, ref_map, transport->url); } @@ builtin/fetch.c: static int do_fetch(struct transport *transport, } - if (fetch_and_consume_refs(transport, transaction, ref_map, &fetch_head)) { -+ if (fetch_and_consume_refs(&display, transport, transaction, ref_map, &fetch_head)) { ++ if (fetch_and_consume_refs(&display_state, transport, transaction, ref_map, &fetch_head)) { retcode = 1; goto cleanup; } @@ builtin/fetch.c: static int do_fetch(struct transport *transport, * the transaction and don't commit anything. */ - if (backfill_tags(transport, transaction, tags_ref_map, -+ if (backfill_tags(&display, transport, transaction, tags_ref_map, ++ if (backfill_tags(&display_state, transport, transaction, tags_ref_map, &fetch_head)) retcode = 1; } 3: a4fd935c40 ! 2: 34eedb882c fetch: move output format into `display_state` @@ builtin/fetch.c: static int s_update_ref(const char *action, { int max, rlen, llen, len; -@@ builtin/fetch.c: static void display_state_init(struct display_state *display, struct ref *ref_ma +@@ builtin/fetch.c: static void display_state_init(struct display_state *display_state, struct ref * git_config_get_string_tmp("fetch.output", &format); if (!strcasecmp(format, "full")) - compact_format = 0; -+ display->compact_format = 0; ++ display_state->compact_format = 0; else if (!strcasecmp(format, "compact")) - compact_format = 1; -+ display->compact_format = 1; ++ display_state->compact_format = 1; else die(_("invalid value for '%s': '%s'"), "fetch.output", format); -@@ builtin/fetch.c: static void display_state_init(struct display_state *display, struct ref *ref_ma +@@ builtin/fetch.c: static void display_state_init(struct display_state *display_state, struct ref * !strcmp(rm->name, "HEAD")) continue; - width = refcol_width(rm); -+ width = refcol_width(rm, display->compact_format); ++ width = refcol_width(rm, display_state->compact_format); /* * Not precise calculation for compact mode because '*' can -@@ builtin/fetch.c: static void format_display(struct display_state *display, +@@ builtin/fetch.c: static void format_display(struct display_state *display_state, width = (summary_width + strlen(summary) - gettext_width(summary)); strbuf_addf(display_buffer, "%c %-*s ", code, width, summary); - if (!compact_format) -+ if (!display->compact_format) - print_remote_to_local(display, display_buffer, remote, local); ++ if (!display_state->compact_format) + print_remote_to_local(display_state, display_buffer, remote, local); else - print_compact(display, display_buffer, remote, local); + print_compact(display_state, display_buffer, remote, local); 4: 0998173b57 ! 3: ec355b8b8d fetch: pass the full local reference name to `format_display` @@ Commit message Signed-off-by: Patrick Steinhardt ## builtin/fetch.c ## -@@ builtin/fetch.c: static void format_display(struct display_state *display, +@@ builtin/fetch.c: static void format_display(struct display_state *display_state, strbuf_addf(display_buffer, "%c %-*s ", code, width, summary); - if (!display->compact_format) -- print_remote_to_local(display, display_buffer, remote, local); -+ print_remote_to_local(display, display_buffer, remote, prettify_refname(local)); + if (!display_state->compact_format) +- print_remote_to_local(display_state, display_buffer, remote, local); ++ print_remote_to_local(display_state, display_buffer, remote, prettify_refname(local)); else -- print_compact(display, display_buffer, remote, local); -+ print_compact(display, display_buffer, remote, prettify_refname(local)); +- print_compact(display_state, display_buffer, remote, local); ++ print_compact(display_state, display_buffer, remote, prettify_refname(local)); if (error) strbuf_addf(display_buffer, " (%s)", error); } @@ builtin/fetch.c: static int update_local_ref(struct ref *ref, - struct strbuf *display_buffer, int summary_width) + struct strbuf *display, int summary_width) { struct commit *current = NULL, *updated; - const char *pretty_ref = prettify_refname(ref->name); @@ builtin/fetch.c: static int update_local_ref(struct ref *ref, @@ builtin/fetch.c: static int update_local_ref(struct ref *ref, if (oideq(&ref->old_oid, &ref->new_oid)) { if (verbosity > 0) - format_display(display, display_buffer, '=', _("[up to date]"), NULL, + format_display(display_state, display, '=', _("[up to date]"), NULL, - remote, pretty_ref, summary_width); + remote, ref->name, summary_width); return 0; @@ builtin/fetch.c: static int update_local_ref(struct ref *ref, @@ builtin/fetch.c: static int update_local_ref(struct ref *ref, */ - format_display(display, display_buffer, '!', _("[rejected]"), + format_display(display_state, display, '!', _("[rejected]"), _("can't fetch into checked-out branch"), - remote, pretty_ref, summary_width); + remote, ref->name, summary_width); @@ builtin/fetch.c: static int update_local_ref(struct ref *ref, @@ builtin/fetch.c: static int update_local_ref(struct ref *ref, r = s_update_ref("updating tag", ref, transaction, 0); - format_display(display, display_buffer, r ? '!' : 't', _("[tag update]"), + format_display(display_state, display, r ? '!' : 't', _("[tag update]"), r ? _("unable to update local ref") : NULL, - remote, pretty_ref, summary_width); + remote, ref->name, summary_width); return r; } else { - format_display(display, display_buffer, '!', _("[rejected]"), + format_display(display_state, display, '!', _("[rejected]"), _("would clobber existing tag"), - remote, pretty_ref, summary_width); + remote, ref->name, summary_width); @@ builtin/fetch.c: static int update_local_ref(struct ref *ref, } @@ builtin/fetch.c: static int update_local_ref(struct ref *ref, r = s_update_ref(msg, ref, transaction, 0); - format_display(display, display_buffer, r ? '!' : '*', what, + format_display(display_state, display, r ? '!' : '*', what, r ? _("unable to update local ref") : NULL, - remote, pretty_ref, summary_width); + remote, ref->name, summary_width); @@ builtin/fetch.c: static int update_local_ref(struct ref *ref, @@ builtin/fetch.c: static int update_local_ref(struct ref *ref, r = s_update_ref("fast-forward", ref, transaction, 1); - format_display(display, display_buffer, r ? '!' : ' ', quickref.buf, + format_display(display_state, display, r ? '!' : ' ', quickref.buf, r ? _("unable to update local ref") : NULL, - remote, pretty_ref, summary_width); + remote, ref->name, summary_width); @@ builtin/fetch.c: static int update_local_ref(struct ref *ref, } else if (force || ref->force) { @@ builtin/fetch.c: static int update_local_ref(struct ref *ref, r = s_update_ref("forced-update", ref, transaction, 1); - format_display(display, display_buffer, r ? '!' : '+', quickref.buf, + format_display(display_state, display, r ? '!' : '+', quickref.buf, r ? _("unable to update local ref") : _("forced update"), - remote, pretty_ref, summary_width); + remote, ref->name, summary_width); strbuf_release(&quickref); return r; } else { - format_display(display, display_buffer, '!', _("[rejected]"), _("non-fast-forward"), + format_display(display_state, display, '!', _("[rejected]"), _("non-fast-forward"), - remote, pretty_ref, summary_width); + remote, ref->name, summary_width); return 1; } } -@@ builtin/fetch.c: static int prune_refs(struct display_state *display, +@@ builtin/fetch.c: static int prune_refs(struct display_state *display_state, shown_url = 1; } - format_display(display, &sb, '-', _("[deleted]"), NULL, + format_display(display_state, &sb, '-', _("[deleted]"), NULL, - _("(none)"), prettify_refname(ref->name), + _("(none)"), ref->name, summary_width); 5: d45ec31eea ! 4: e4c1ed4ad5 fetch: deduplicate handling of per-reference format @@ Metadata Author: Patrick Steinhardt ## Commit message ## - fetch: deduplicate handling of per-reference format + fetch: centralize handling of per-reference format - Both callsites that call `format_display()` and then print the result to - standard error use the same formatting directive " %s\n" to print the - reference to disk, thus duplicating a small part of the logic. + The function `format_display()` is used to print a single reference + update to a buffer which will then ultimately be printed by the caller. + This architecture causes us to duplicate some logic across the different + callsites of this function. This makes it hard to follow the code as + some parts of the logic are located in one place, while other parts of + the logic are located in a different place. Furthermore, by having the + logic scattered around it becomes quite hard to implement a new output + format for the reference updates. - Refactor the code to handle this in `format_display()` itself. This - paves the way for handling the printing logic in that function - completely. + We can make the logic a whole lot easier to understand by making the + `format_display()` function self-contained so that it handles formatting + and printing of the references. This will eventually allow us to easily + implement a completely different output format, but also opens the door + to conditionally print to either stdout or stderr depending on the + output format. + + As a first step towards that goal we move the formatting directive used + by both callers to print a single reference update into this function. Signed-off-by: Patrick Steinhardt ## builtin/fetch.c ## -@@ builtin/fetch.c: static void format_display(struct display_state *display, +@@ builtin/fetch.c: static void format_display(struct display_state *display_state, width = (summary_width + strlen(summary) - gettext_width(summary)); - strbuf_addf(display_buffer, "%c %-*s ", code, width, summary); + strbuf_addf(display_buffer, " %c %-*s ", code, width, summary); - if (!display->compact_format) - print_remote_to_local(display, display_buffer, remote, prettify_refname(local)); + if (!display_state->compact_format) + print_remote_to_local(display_state, display_buffer, remote, prettify_refname(local)); else - print_compact(display, display_buffer, remote, prettify_refname(local)); + print_compact(display_state, display_buffer, remote, prettify_refname(local)); if (error) strbuf_addf(display_buffer, " (%s)", error); + strbuf_addch(display_buffer, '\n'); } static int update_local_ref(struct ref *ref, -@@ builtin/fetch.c: static int store_updated_refs(struct display_state *display, +@@ builtin/fetch.c: static int store_updated_refs(struct display_state *display_state, url_len, url); shown_url = 1; } @@ builtin/fetch.c: static int store_updated_refs(struct display_state *display, } } } -@@ builtin/fetch.c: static int prune_refs(struct display_state *display, - format_display(display, &sb, '-', _("[deleted]"), NULL, +@@ builtin/fetch.c: static int prune_refs(struct display_state *display_state, + format_display(display_state, &sb, '-', _("[deleted]"), NULL, _("(none)"), ref->name, summary_width); - fprintf(stderr, " %s\n",sb.buf); 6: 2ea3a4e308 ! 5: 98b799af71 fetch: deduplicate logic to print remote URL @@ Metadata Author: Patrick Steinhardt ## Commit message ## - fetch: deduplicate logic to print remote URL + fetch: centralize logic to print remote URL When fetching from a remote, we not only print the actual references that have changed, but will also print the URL from which we have @@ Commit message can convert the global variable into a member of `display_state`. And second, we can deduplicate the logic to compute the anonymized URL. + This also works as expected when fetching from multiple remotes, for + example via a group of remotes, as we do this by forking a standalone + git-fetch(1) process per remote that is to be fetched. + Signed-off-by: Patrick Steinhardt ## builtin/fetch.c ## @@ builtin/fetch.c: static int refcol_width(const struct ref *ref, int compact_form return rlen; } --static void display_state_init(struct display_state *display, struct ref *ref_map) -+static void display_state_init(struct display_state *display, struct ref *ref_map, +-static void display_state_init(struct display_state *display_state, struct ref *ref_map) ++static void display_state_init(struct display_state *display_state, struct ref *ref_map, + const char *raw_url) { struct ref *rm; const char *format = "full"; + int i; - memset(display, 0, sizeof(*display)); + memset(display_state, 0, sizeof(*display_state)); + if (raw_url) -+ display->url = transport_anonymize_url(raw_url); ++ display_state->url = transport_anonymize_url(raw_url); + else -+ display->url = xstrdup("foreign"); ++ display_state->url = xstrdup("foreign"); + -+ display->url_len = strlen(display->url); -+ for (i = display->url_len - 1; display->url[i] == '/' && 0 <= i; i--) ++ display_state->url_len = strlen(display_state->url); ++ for (i = display_state->url_len - 1; display_state->url[i] == '/' && 0 <= i; i--) + ; -+ display->url_len = i + 1; -+ if (4 < i && !strncmp(".git", display->url + i - 3, 4)) -+ display->url_len = i - 3; ++ display_state->url_len = i + 1; ++ if (4 < i && !strncmp(".git", display_state->url + i - 3, 4)) ++ display_state->url_len = i - 3; + if (verbosity < 0) return; -@@ builtin/fetch.c: static void display_state_init(struct display_state *display, struct ref *ref_ma +@@ builtin/fetch.c: static void display_state_init(struct display_state *display_state, struct ref * } } -+static void display_state_release(struct display_state *display) ++static void display_state_release(struct display_state *display_state) +{ -+ free(display->url); ++ free(display_state->url); +} + - static void print_remote_to_local(struct display_state *display, + static void print_remote_to_local(struct display_state *display_state, struct strbuf *display_buffer, const char *remote, const char *local) -@@ builtin/fetch.c: static void format_display(struct display_state *display, +@@ builtin/fetch.c: static void format_display(struct display_state *display_state, if (verbosity < 0) return; -+ if (!display->shown_url) { -+ strbuf_addf(display_buffer, _("From %.*s\n"), display->url_len, display->url); -+ display->shown_url = 1; ++ if (!display_state->shown_url) { ++ strbuf_addf(display_buffer, _("From %.*s\n"), ++ display_state->url_len, display_state->url); ++ display_state->shown_url = 1; + } + width = (summary_width + strlen(summary) - gettext_width(summary)); @@ builtin/fetch.c: static void format_display(struct display_state *display, @@ builtin/fetch.c: N_("it took %.2f seconds to check forced updates; you can use\n" "to avoid this check\n"); - static int store_updated_refs(struct display_state *display, + static int store_updated_refs(struct display_state *display_state, - const char *raw_url, const char *remote_name, + const char *remote_name, int connectivity_checked, @@ builtin/fetch.c: N_("it took %.2f seconds to check forced updates; you can use\n rm = ref_map; if (check_connected(iterate_ref_map, &rm, &opt)) { - rc = error(_("%s did not send all necessary objects\n"), url); -+ rc = error(_("%s did not send all necessary objects\n"), display->url); ++ rc = error(_("%s did not send all necessary objects\n"), ++ display_state->url); goto abort; } } -@@ builtin/fetch.c: static int store_updated_refs(struct display_state *display, +@@ builtin/fetch.c: static int store_updated_refs(struct display_state *display_state, what = rm->name; } @@ builtin/fetch.c: static int store_updated_refs(struct display_state *display, strbuf_reset(¬e); if (*what) { if (*kind) -@@ builtin/fetch.c: static int store_updated_refs(struct display_state *display, +@@ builtin/fetch.c: static int store_updated_refs(struct display_state *display_state, append_fetch_head(fetch_head, &rm->old_oid, rm->fetch_head_status, - note.buf, url, url_len); -+ note.buf, display->url, display->url_len); ++ note.buf, display_state->url, ++ display_state->url_len); strbuf_reset(¬e); if (ref) { -@@ builtin/fetch.c: static int store_updated_refs(struct display_state *display, +@@ builtin/fetch.c: static int store_updated_refs(struct display_state *display_state, *what ? what : "HEAD", "FETCH_HEAD", summary_width); } @@ builtin/fetch.c: static int store_updated_refs(struct display_state *display, } } -@@ builtin/fetch.c: static int store_updated_refs(struct display_state *display, +@@ builtin/fetch.c: static int store_updated_refs(struct display_state *display_state, abort: strbuf_release(¬e); @@ builtin/fetch.c: static int store_updated_refs(struct display_state *display, return rc; } -@@ builtin/fetch.c: static int fetch_and_consume_refs(struct display_state *display, +@@ builtin/fetch.c: static int fetch_and_consume_refs(struct display_state *display_state, } trace2_region_enter("fetch", "consume_refs", the_repository); -- ret = store_updated_refs(display, transport->url, transport->remote->name, -+ ret = store_updated_refs(display, transport->remote->name, +- ret = store_updated_refs(display_state, transport->url, transport->remote->name, ++ ret = store_updated_refs(display_state, transport->remote->name, connectivity_checked, transaction, ref_map, fetch_head); trace2_region_leave("fetch", "consume_refs", the_repository); -@@ builtin/fetch.c: static int fetch_and_consume_refs(struct display_state *display, - static int prune_refs(struct display_state *display, +@@ builtin/fetch.c: static int fetch_and_consume_refs(struct display_state *display_state, + static int prune_refs(struct display_state *display_state, struct refspec *rs, struct ref_transaction *transaction, - struct ref *ref_map, @@ builtin/fetch.c: static int fetch_and_consume_refs(struct display_state *display if (!dry_run) { if (transaction) { for (ref = stale_refs; ref; ref = ref->next) { -@@ builtin/fetch.c: static int prune_refs(struct display_state *display, +@@ builtin/fetch.c: static int prune_refs(struct display_state *display_state, for (ref = stale_refs; ref; ref = ref->next) { struct strbuf sb = STRBUF_INIT; @@ builtin/fetch.c: static int prune_refs(struct display_state *display, - fprintf(stderr, _("From %.*s\n"), url_len, url); - shown_url = 1; - } - format_display(display, &sb, '-', _("[deleted]"), NULL, + format_display(display_state, &sb, '-', _("[deleted]"), NULL, _("(none)"), ref->name, summary_width); -@@ builtin/fetch.c: static int prune_refs(struct display_state *display, +@@ builtin/fetch.c: static int prune_refs(struct display_state *display_state, cleanup: strbuf_release(&err); @@ builtin/fetch.c: static int do_fetch(struct transport *transport, { struct ref_transaction *transaction = NULL; struct ref *ref_map = NULL; -- struct display_state display; -+ struct display_state display = { 0 }; +- struct display_state display_state; ++ struct display_state display_state = { 0 }; int autotags = (transport->remote->fetch_tags == 1); int retcode = 0; const struct ref *remote_refs; @@ builtin/fetch.c: static int do_fetch(struct transport *transport, if (retcode) goto cleanup; -- display_state_init(&display, ref_map); -+ display_state_init(&display, ref_map, transport->url); +- display_state_init(&display_state, ref_map); ++ display_state_init(&display_state, ref_map, transport->url); if (atomic_fetch) { transaction = ref_transaction_begin(&err); @@ builtin/fetch.c: static int do_fetch(struct transport *transport, * don't care whether --tags was specified. */ if (rs->nr) { -- retcode = prune_refs(&display, rs, transaction, ref_map, transport->url); -+ retcode = prune_refs(&display, rs, transaction, ref_map); +- retcode = prune_refs(&display_state, rs, transaction, ref_map, transport->url); ++ retcode = prune_refs(&display_state, rs, transaction, ref_map); } else { - retcode = prune_refs(&display, &transport->remote->fetch, + retcode = prune_refs(&display_state, &transport->remote->fetch, - transaction, ref_map, - transport->url); + transaction, ref_map); @@ builtin/fetch.c: static int do_fetch(struct transport *transport, error("%s", err.buf); } -+ display_state_release(&display); ++ display_state_release(&display_state); close_fetch_head(&fetch_head); strbuf_release(&err); free_refs(ref_map); 7: f67f9640a8 < -: ---------- fetch: fix inconsistent summary width for pruned and updated refs 8: 9667301711 < -: ---------- fetch: centralize printing of reference updates -: ---------- > 6: fe7e2e85eb fetch: centralize printing of reference updates -- 2.40.0