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 <ps@xxxxxx>
## 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 <ps@xxxxxx>
## 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 <ps@xxxxxx>
## 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 <ps@xxxxxx>
## 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 <ps@xxxxxx>
## 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
Attachment:
signature.asc
Description: PGP signature
