Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
	Felipe Contreras <felipe.contreras@gmail.com>,
	Glen Choo <chooglen@google.com>,
	Jonathan Tan <jonathantanmy@google.com>,
	Jacob Keller <jacob.e.keller@intel.com>
Subject: [PATCH v3 6/8] fetch: move display format parsing into main function
Date: Wed, 3 May 2023 13:34:40 +0200	[thread overview]
Message-ID: <cd23440128ea3057c2f5110d230417abc59bdc3c.1683113177.git.ps@pks.im> (raw)
In-Reply-To: <cover.1683113177.git.ps@pks.im>

[-- Attachment #1: Type: text/plain, Size: 5425 bytes --]

Parsing the display format happens inside of `display_state_init()`. As
we only need to check for a simple config entry, this is a natural
location to put this code as it means that display-state logic is neatly
contained in a single location.

We're about to introduce a output format though that is intended to be
parseable by machines, for example inside of a script. In that case it
becomes a bit awkward of an interface if you have to call git-fetch(1)
with the `fetch.output` config key set. We're thus going to introduce a
new `--output-format` switch for git-fetch(1) so that the output format
can be configured more directly.

This means we'll have to hook parsing of the display format into the
command line options parser. Having the code to determine the actual
output format scattered across two different sites is hard to reason
about though.

Refactor the code such that callers are expected to pass the display
format that is to be used into `display_state_init()`. This allows us to
lift up the code into the main function, where we can then hook it into
command line options parser in a follow-up commit.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/fetch.c | 41 ++++++++++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 9e7e45344d..e15d43dc1e 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -788,14 +788,13 @@ static int refcol_width(const struct ref *ref, int compact_format)
 }
 
 static void display_state_init(struct display_state *display_state, struct ref *ref_map,
-			       const char *raw_url)
+			       const char *raw_url, enum display_format format)
 {
-	const char *format = "full";
 	int i;
 
 	memset(display_state, 0, sizeof(*display_state));
-
 	strbuf_init(&display_state->buf, 0);
+	display_state->format = format;
 
 	if (raw_url)
 		display_state->url = transport_anonymize_url(raw_url);
@@ -812,15 +811,6 @@ static void display_state_init(struct display_state *display_state, struct ref *
 	if (verbosity < 0)
 		return;
 
-	git_config_get_string_tmp("fetch.output", &format);
-	if (!strcasecmp(format, "full"))
-		display_state->format = DISPLAY_FORMAT_FULL;
-	else if (!strcasecmp(format, "compact"))
-		display_state->format = DISPLAY_FORMAT_COMPACT;
-	else
-		die(_("invalid value for '%s': '%s'"),
-		    "fetch.output", format);
-
 	switch (display_state->format) {
 	case DISPLAY_FORMAT_FULL:
 	case DISPLAY_FORMAT_COMPACT: {
@@ -1614,7 +1604,8 @@ static int backfill_tags(struct display_state *display_state,
 }
 
 static int do_fetch(struct transport *transport,
-		    struct refspec *rs)
+		    struct refspec *rs,
+		    enum display_format display_format)
 {
 	struct ref_transaction *transaction = NULL;
 	struct ref *ref_map = NULL;
@@ -1700,7 +1691,7 @@ static int do_fetch(struct transport *transport,
 	if (retcode)
 		goto cleanup;
 
-	display_state_init(&display_state, ref_map, transport->url);
+	display_state_init(&display_state, ref_map, transport->url, display_format);
 
 	if (atomic_fetch) {
 		transaction = ref_transaction_begin(&err);
@@ -2078,7 +2069,8 @@ static inline void fetch_one_setup_partial(struct remote *remote)
 }
 
 static int fetch_one(struct remote *remote, int argc, const char **argv,
-		     int prune_tags_ok, int use_stdin_refspecs)
+		     int prune_tags_ok, int use_stdin_refspecs,
+		     enum display_format display_format)
 {
 	struct refspec rs = REFSPEC_INIT_FETCH;
 	int i;
@@ -2145,7 +2137,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv,
 	sigchain_push_common(unlock_pack_on_signal);
 	atexit(unlock_pack_atexit);
 	sigchain_push(SIGPIPE, SIG_IGN);
-	exit_code = do_fetch(gtransport, &rs);
+	exit_code = do_fetch(gtransport, &rs, display_format);
 	sigchain_pop(SIGPIPE);
 	refspec_clear(&rs);
 	transport_disconnect(gtransport);
@@ -2157,6 +2149,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 {
 	int i;
 	const char *bundle_uri;
+	enum display_format display_format = DISPLAY_FORMAT_UNKNOWN;
 	struct string_list list = STRING_LIST_INIT_DUP;
 	struct remote *remote = NULL;
 	int result = 0;
@@ -2183,6 +2176,19 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix,
 			     builtin_fetch_options, builtin_fetch_usage, 0);
 
+	if (display_format == DISPLAY_FORMAT_UNKNOWN) {
+		const char *format = "full";
+
+		git_config_get_string_tmp("fetch.output", &format);
+		if (!strcasecmp(format, "full"))
+			display_format = DISPLAY_FORMAT_FULL;
+		else if (!strcasecmp(format, "compact"))
+			display_format = DISPLAY_FORMAT_COMPACT;
+		else
+			die(_("invalid value for '%s': '%s'"),
+			    "fetch.output", format);
+	}
+
 	if (recurse_submodules_cli != RECURSE_SUBMODULES_DEFAULT)
 		recurse_submodules = recurse_submodules_cli;
 
@@ -2311,7 +2317,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	} else if (remote) {
 		if (filter_options.choice || has_promisor_remote())
 			fetch_one_setup_partial(remote);
-		result = fetch_one(remote, argc, argv, prune_tags_ok, stdin_refspecs);
+		result = fetch_one(remote, argc, argv, prune_tags_ok, stdin_refspecs,
+				   display_format);
 	} else {
 		int max_children = max_jobs;
 
-- 
2.40.1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  parent reply	other threads:[~2023-05-03 11:35 UTC|newest]

Thread overview: 120+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-19 12:31 [PATCH 0/8] fetch: introduce machine-parseable output Patrick Steinhardt
2023-04-19 12:31 ` [PATCH 1/8] fetch: split out tests for output format Patrick Steinhardt
2023-04-19 12:31 ` [PATCH 2/8] fetch: add a test to exercise invalid output formats Patrick Steinhardt
2023-04-19 12:31 ` [PATCH 3/8] fetch: fix missing from-reference when fetching HEAD:foo Patrick Steinhardt
2023-04-26 19:20   ` Jacob Keller
2023-04-27 10:58     ` Patrick Steinhardt
2023-04-26 19:21   ` Jacob Keller
2023-04-27 10:58     ` Patrick Steinhardt
2023-04-26 19:25   ` Glen Choo
2023-04-27 10:58     ` Patrick Steinhardt
2023-04-19 12:31 ` [PATCH 4/8] fetch: introduce `display_format` enum Patrick Steinhardt
2023-04-19 12:31 ` [PATCH 5/8] fetch: move display format parsing into main function Patrick Steinhardt
2023-04-19 12:31 ` [PATCH 6/8] fetch: move option related variables " Patrick Steinhardt
2023-04-19 12:31 ` [PATCH 7/8] fetch: introduce new `--output-format` option Patrick Steinhardt
2023-04-26 19:40   ` Glen Choo
2023-04-27 10:58     ` Patrick Steinhardt
2023-04-19 12:31 ` [PATCH 8/8] fetch: introduce machine-parseable "porcelain" output format Patrick Steinhardt
2023-04-26 19:52   ` Glen Choo
2023-04-27 10:58     ` Patrick Steinhardt
2023-04-27 23:20       ` Glen Choo
2023-04-28  8:51         ` Patrick Steinhardt
2023-04-28 17:20           ` Glen Choo
2023-05-02 20:55       ` Felipe Contreras
2023-04-24 20:17 ` [PATCH 0/8] fetch: introduce machine-parseable output Felipe Contreras
2023-04-25  9:58   ` Patrick Steinhardt
2023-04-26 19:14     ` Jacob Keller
2023-04-26 20:23       ` Junio C Hamano
2023-04-26 20:30         ` Jacob Keller
2023-04-27 10:58         ` Patrick Steinhardt
2023-04-27 19:46           ` Jacob Keller
2023-04-27 22:49         ` Glen Choo
2023-04-26 20:24       ` Junio C Hamano
2023-04-26 18:54 ` Glen Choo
2023-04-26 21:14   ` Glen Choo
2023-04-26 19:17 ` Jacob Keller
2023-04-27 11:13 ` [PATCH v2 " Patrick Steinhardt
2023-04-27 11:13   ` [PATCH v2 1/8] fetch: split out tests for output format Patrick Steinhardt
2023-04-29 17:34     ` SZEDER Gábor
2023-05-03 11:21       ` Patrick Steinhardt
2023-04-27 11:13   ` [PATCH v2 2/8] fetch: add a test to exercise invalid output formats Patrick Steinhardt
2023-04-27 11:13   ` [PATCH v2 3/8] fetch: fix missing from-reference when fetching HEAD:foo Patrick Steinhardt
2023-04-27 17:26     ` Glen Choo
2023-04-27 19:49     ` Jacob Keller
2023-04-27 11:13   ` [PATCH v2 4/8] fetch: introduce `display_format` enum Patrick Steinhardt
2023-04-27 11:13   ` [PATCH v2 5/8] fetch: move display format parsing into main function Patrick Steinhardt
2023-04-27 11:13   ` [PATCH v2 6/8] fetch: move option related variables " Patrick Steinhardt
2023-04-27 21:52     ` Junio C Hamano
2023-04-27 11:13   ` [PATCH v2 7/8] fetch: introduce new `--output-format` option Patrick Steinhardt
2023-04-27 22:01     ` Junio C Hamano
2023-04-28 22:03       ` Glen Choo
2023-05-03  9:12         ` Patrick Steinhardt
2023-04-28 22:31     ` Glen Choo
2023-05-03  9:43       ` Patrick Steinhardt
2023-05-03 11:36         ` Patrick Steinhardt
2023-04-27 11:13   ` [PATCH v2 8/8] fetch: introduce machine-parseable "porcelain" output format Patrick Steinhardt
2023-04-27 19:52     ` Jacob Keller
2023-04-28 22:42     ` Glen Choo
2023-05-03 11:34 ` [PATCH v3 0/8] fetch: introduce machine-parseable output Patrick Steinhardt
2023-05-03 11:34   ` [PATCH v3 1/8] fetch: fix `--no-recurse-submodules` with multi-remote fetches Patrick Steinhardt
2023-05-08 22:51     ` Glen Choo
2023-05-09 12:41       ` Patrick Steinhardt
2023-05-03 11:34   ` [PATCH v3 2/8] fetch: split out tests for output format Patrick Steinhardt
2023-05-03 11:34   ` [PATCH v3 3/8] fetch: add a test to exercise invalid output formats Patrick Steinhardt
2023-05-03 11:34   ` [PATCH v3 4/8] fetch: fix missing from-reference when fetching HEAD:foo Patrick Steinhardt
2023-05-03 11:34   ` [PATCH v3 5/8] fetch: introduce `display_format` enum Patrick Steinhardt
2023-05-03 11:34   ` Patrick Steinhardt [this message]
2023-05-03 11:34   ` [PATCH v3 7/8] fetch: move option related variables into main function Patrick Steinhardt
2023-05-03 11:34   ` [PATCH v3 8/8] fetch: introduce machine-parseable "porcelain" output format Patrick Steinhardt
2023-05-08 23:42     ` Glen Choo
2023-05-09 12:41       ` Patrick Steinhardt
2023-05-09  0:03     ` Glen Choo
2023-05-03 16:48   ` [PATCH v3 0/8] fetch: introduce machine-parseable output Junio C Hamano
2023-05-03 16:53     ` Junio C Hamano
2023-05-04  7:57       ` Patrick Steinhardt
2023-05-09  0:06   ` Glen Choo
2023-05-09 12:42     ` Patrick Steinhardt
2023-05-09 13:01 ` [PATCH v4 " Patrick Steinhardt
2023-05-09 13:02   ` [PATCH v4 1/8] fetch: fix `--no-recurse-submodules` with multi-remote fetches Patrick Steinhardt
2023-05-09 17:49     ` Junio C Hamano
2023-05-09 18:27       ` Glen Choo
2023-05-10 12:34         ` Patrick Steinhardt
2023-05-09 13:02   ` [PATCH v4 2/8] fetch: split out tests for output format Patrick Steinhardt
2023-05-09 13:02   ` [PATCH v4 3/8] fetch: add a test to exercise invalid output formats Patrick Steinhardt
2023-05-09 17:58     ` Junio C Hamano
2023-05-10 12:34       ` Patrick Steinhardt
2023-05-09 13:02   ` [PATCH v4 4/8] fetch: fix missing from-reference when fetching HEAD:foo Patrick Steinhardt
2023-05-09 19:28     ` Junio C Hamano
2023-05-10 12:34       ` Patrick Steinhardt
2023-05-09 13:02   ` [PATCH v4 5/8] fetch: introduce `display_format` enum Patrick Steinhardt
2023-05-09 20:19     ` Junio C Hamano
2023-05-10 12:35       ` Patrick Steinhardt
2023-05-09 13:02   ` [PATCH v4 6/8] fetch: move display format parsing into main function Patrick Steinhardt
2023-05-09 20:35     ` Junio C Hamano
2023-05-09 22:30     ` Glen Choo
2023-05-10 12:35       ` Patrick Steinhardt
2023-05-09 13:02   ` [PATCH v4 7/8] fetch: move option related variables " Patrick Steinhardt
2023-05-09 13:02   ` [PATCH v4 8/8] fetch: introduce machine-parseable "porcelain" output format Patrick Steinhardt
2023-05-09 20:43     ` Junio C Hamano
2023-05-10 12:35       ` Patrick Steinhardt
2023-05-10 12:33 ` [PATCH v5 0/9] fetch: introduce machine-parseable output Patrick Steinhardt
2023-05-10 12:34   ` [PATCH v5 1/9] fetch: fix `--no-recurse-submodules` with multi-remote fetches Patrick Steinhardt
2023-05-10 12:34   ` [PATCH v5 2/9] fetch: split out tests for output format Patrick Steinhardt
2023-05-10 12:34   ` [PATCH v5 3/9] fetch: add a test to exercise invalid output formats Patrick Steinhardt
2023-05-10 12:34   ` [PATCH v5 4/9] fetch: print left-hand side when fetching HEAD:foo Patrick Steinhardt
2023-05-12  0:16     ` Glen Choo
2023-05-13 16:59     ` Jeff King
2023-05-15  5:15       ` Patrick Steinhardt
2023-05-10 12:34   ` [PATCH v5 5/9] fetch: refactor calculation of the display table width Patrick Steinhardt
2023-05-12  0:49     ` Glen Choo
2023-05-10 12:34   ` [PATCH v5 6/9] fetch: introduce `display_format` enum Patrick Steinhardt
2023-05-10 12:34   ` [PATCH v5 7/9] fetch: lift up parsing of "fetch.output" config variable Patrick Steinhardt
2023-05-10 12:34   ` [PATCH v5 8/9] fetch: move option related variables into main function Patrick Steinhardt
2023-05-10 12:34   ` [PATCH v5 9/9] fetch: introduce machine-parseable "porcelain" output format Patrick Steinhardt
2023-05-12  1:02     ` Glen Choo
2023-05-10 18:05   ` [PATCH v5 0/9] fetch: introduce machine-parseable output Junio C Hamano
2023-05-11 11:05     ` Patrick Steinhardt
2023-05-11 16:53       ` Junio C Hamano
2023-05-11 17:24       ` Felipe Contreras
2023-05-12  1:09   ` Glen Choo
2023-05-12  7:16     ` Patrick Steinhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cd23440128ea3057c2f5110d230417abc59bdc3c.1683113177.git.ps@pks.im \
    --to=ps@pks.im \
    --cc=chooglen@google.com \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jacob.e.keller@intel.com \
    --cc=jonathantanmy@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).