git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 05/12] builtin/show-ref: refactor `--exclude-existing` options

From
Patrick Steinhardt <ps@pks.im>
Date
Oct 31, 2023, 08:16 UTC
Message-ID
<b47440089b6701061786b5729501bbbbda8387be.1698739941.git.ps@pks.im>
In-Reply-To
<cover.1698739941.git.ps@pks.im>

It's not immediately obvious options which options are applicable to what subcommand in git-show-ref(1) because all options exist as global state. This can easily cause confusion for the reader.

Refactor options for the `--exclude-existing` subcommand to be contained in a separate structure. This structure is stored on the stack and passed down as required. Consequently, it clearly delimits the scope of those options and requires the reader to worry less about global state.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/show-ref.c | 78 ++++++++++++++++++++++++++--------------------
 1 file changed, 44 insertions(+), 34 deletions(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index f95418d3d16..5aa6016376a 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -19,8 +19,7 @@ static const char * const show_ref_usage[] = {
 };
 
 static int deref_tags, show_head, tags_only, heads_only, found_match, verify,
-	   quiet, hash_only, abbrev, exclude_arg;
-static const char *exclude_existing_arg;
+	   quiet, hash_only, abbrev;
 
 static void show_one(const char *refname, const struct object_id *oid)
 {
@@ -95,6 +94,15 @@ static int add_existing(const char *refname,
 	return 0;
 }
 
+struct exclude_existing_options {
+	/*
+	 * We need an explicit `enabled` field because it is perfectly valid
+	 * for `pattern` to be `NULL` even if `--exclude-existing` was given.
+	 */
+	int enabled;
+	const char *pattern;
+};
+
 /*
  * read "^(?:<anything>\s)?<refname>(?:\^\{\})?$" from the standard input,
  * and
@@ -104,11 +112,11 @@ static int add_existing(const char *refname,
  * (4) ignore if refname is a ref that exists in the local repository;
  * (5) otherwise output the line.
  */
-static int cmd_show_ref__exclude_existing(const char *match)
+static int cmd_show_ref__exclude_existing(const struct exclude_existing_options *opts)
 {
 	struct string_list existing_refs = STRING_LIST_INIT_DUP;
 	char buf[1024];
-	int matchlen = match ? strlen(match) : 0;
+	int patternlen = opts->pattern ? strlen(opts->pattern) : 0;
 
 	for_each_ref(add_existing, &existing_refs);
 	while (fgets(buf, sizeof(buf), stdin)) {
@@ -124,11 +132,11 @@ static int cmd_show_ref__exclude_existing(const char *match)
 		for (ref = buf + len; buf < ref; ref--)
 			if (isspace(ref[-1]))
 				break;
-		if (match) {
+		if (opts->pattern) {
 			int reflen = buf + len - ref;
-			if (reflen < matchlen)
+			if (reflen < patternlen)
 				continue;
-			if (strncmp(ref, match, matchlen))
+			if (strncmp(ref, opts->pattern, patternlen))
 				continue;
 		}
 		if (check_refname_format(ref, 0)) {
@@ -201,44 +209,46 @@ static int hash_callback(const struct option *opt, const char *arg, int unset)
 static int exclude_existing_callback(const struct option *opt, const char *arg,
 				     int unset)
 {
+	struct exclude_existing_options *opts = opt->value;
 	BUG_ON_OPT_NEG(unset);
-	exclude_arg = 1;
-	*(const char **)opt->value = arg;
+	opts->enabled = 1;
+	opts->pattern = arg;
 	return 0;
 }
 
-static const struct option show_ref_options[] = {
-	OPT_BOOL(0, "tags", &tags_only, N_("only show tags (can be combined with heads)")),
-	OPT_BOOL(0, "heads", &heads_only, N_("only show heads (can be combined with tags)")),
-	OPT_BOOL(0, "verify", &verify, N_("stricter reference checking, "
-		    "requires exact ref path")),
-	OPT_HIDDEN_BOOL('h', NULL, &show_head,
-			N_("show the HEAD reference, even if it would be filtered out")),
-	OPT_BOOL(0, "head", &show_head,
-	  N_("show the HEAD reference, even if it would be filtered out")),
-	OPT_BOOL('d', "dereference", &deref_tags,
-		    N_("dereference tags into object IDs")),
-	OPT_CALLBACK_F('s', "hash", &abbrev, N_("n"),
-		       N_("only show SHA1 hash using <n> digits"),
-		       PARSE_OPT_OPTARG, &hash_callback),
-	OPT__ABBREV(&abbrev),
-	OPT__QUIET(&quiet,
-		   N_("do not print results to stdout (useful with --verify)")),
-	OPT_CALLBACK_F(0, "exclude-existing", &exclude_existing_arg,
-		       N_("pattern"), N_("show refs from stdin that aren't in local repository"),
-		       PARSE_OPT_OPTARG | PARSE_OPT_NONEG, exclude_existing_callback),
-	OPT_END()
-};
-
 int cmd_show_ref(int argc, const char **argv, const char *prefix)
 {
+	struct exclude_existing_options exclude_existing_opts = {0};
+	const struct option show_ref_options[] = {
+		OPT_BOOL(0, "tags", &tags_only, N_("only show tags (can be combined with heads)")),
+		OPT_BOOL(0, "heads", &heads_only, N_("only show heads (can be combined with tags)")),
+		OPT_BOOL(0, "verify", &verify, N_("stricter reference checking, "
+			    "requires exact ref path")),
+		OPT_HIDDEN_BOOL('h', NULL, &show_head,
+				N_("show the HEAD reference, even if it would be filtered out")),
+		OPT_BOOL(0, "head", &show_head,
+		  N_("show the HEAD reference, even if it would be filtered out")),
+		OPT_BOOL('d', "dereference", &deref_tags,
+			    N_("dereference tags into object IDs")),
+		OPT_CALLBACK_F('s', "hash", &abbrev, N_("n"),
+			       N_("only show SHA1 hash using <n> digits"),
+			       PARSE_OPT_OPTARG, &hash_callback),
+		OPT__ABBREV(&abbrev),
+		OPT__QUIET(&quiet,
+			   N_("do not print results to stdout (useful with --verify)")),
+		OPT_CALLBACK_F(0, "exclude-existing", &exclude_existing_opts,
+			       N_("pattern"), N_("show refs from stdin that aren't in local repository"),
+			       PARSE_OPT_OPTARG | PARSE_OPT_NONEG, exclude_existing_callback),
+		OPT_END()
+	};
+
 	git_config(git_default_config, NULL);
 
 	argc = parse_options(argc, argv, prefix, show_ref_options,
 			     show_ref_usage, 0);
 
-	if (exclude_arg)
-		return cmd_show_ref__exclude_existing(exclude_existing_arg);
+	if (exclude_existing_opts.enabled)
+		return cmd_show_ref__exclude_existing(&exclude_existing_opts);
 	else if (verify)
 		return cmd_show_ref__verify(argv);
 	else
-- 
2.42.0
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 58 of 66 in “show-ref: introduce mode to check for ref existence”
  1. 00/12 show-ref: introduce mode to check for ref existencePatrick Steinhardt, Oct 24, 2023
  2. 01/12 builtin/show-ref: convert pattern to a local variablePatrick Steinhardt, Oct 24, 2023
  3. 02/12 builtin/show-ref: split up different subcommandsPatrick Steinhardt, Oct 24, 2023
  4. Eric SunshineOct 24, 2023
  5. 03/12 builtin/show-ref: fix leaking string bufferPatrick Steinhardt, Oct 24, 2023
  6. 04/12 builtin/show-ref: fix dead code when passing patternsPatrick Steinhardt, Oct 24, 2023
  7. Eric SunshineOct 24, 2023
  8. 05/12 builtin/show-ref: refactor `--exclude-existing` optionsPatrick Steinhardt, Oct 24, 2023
  9. Eric SunshineOct 24, 2023
  10. Patrick SteinhardtOct 25, 2023
  11. 06/12 builtin/show-ref: stop using global variable to count matchesPatrick Steinhardt, Oct 24, 2023
  12. 07/12 builtin/show-ref: stop using global vars for `show_one()`Patrick Steinhardt, Oct 24, 2023
  13. 08/12 builtin/show-ref: refactor options for patterns subcommandPatrick Steinhardt, Oct 24, 2023
  14. 09/12 builtin/show-ref: ensure mutual exclusiveness of subcommandsPatrick Steinhardt, Oct 24, 2023
  15. Eric SunshineOct 24, 2023
  16. 10/12 builtin/show-ref: explicitly spell out different modes in synopsisPatrick Steinhardt, Oct 24, 2023
  17. Eric SunshineOct 24, 2023
  18. Patrick SteinhardtOct 25, 2023
  19. 11/12 builtin/show-ref: add new mode to check for reference existencePatrick Steinhardt, Oct 24, 2023
  20. Eric SunshineOct 24, 2023
  21. Patrick SteinhardtOct 25, 2023
  22. 12/12 t: use git-show-ref(1) to check for ref existencePatrick Steinhardt, Oct 24, 2023
  23. Junio C HamanoOct 24, 2023
  24. Han-Wen NienhuysOct 25, 2023
  25. Phillip WoodOct 25, 2023
  26. Patrick SteinhardtOct 26, 2023
  27. Phillip WoodOct 27, 2023
  28. Patrick SteinhardtOct 26, 2023
  29. 00/12 show-ref: introduce mode to check for ref existencePatrick Steinhardt, Oct 26, 2023
  30. 01/12 builtin/show-ref: convert pattern to a local variablePatrick Steinhardt, Oct 26, 2023
  31. 02/12 builtin/show-ref: split up different subcommandsPatrick Steinhardt, Oct 26, 2023
  32. 03/12 builtin/show-ref: fix leaking string bufferPatrick Steinhardt, Oct 26, 2023
  33. Taylor BlauOct 30, 2023
  34. 04/12 builtin/show-ref: fix dead code when passing patternsPatrick Steinhardt, Oct 26, 2023
  35. Taylor BlauOct 30, 2023
  36. 05/12 builtin/show-ref: refactor `--exclude-existing` optionsPatrick Steinhardt, Oct 26, 2023
  37. Taylor BlauOct 30, 2023
  38. Patrick SteinhardtOct 31, 2023
  39. Taylor BlauOct 30, 2023
  40. Patrick SteinhardtOct 31, 2023
  41. 06/12 builtin/show-ref: stop using global variable to count matchesPatrick Steinhardt, Oct 26, 2023
  42. Taylor BlauOct 30, 2023
  43. 07/12 builtin/show-ref: stop using global vars for `show_one()`Patrick Steinhardt, Oct 26, 2023
  44. 08/12 builtin/show-ref: refactor options for patterns subcommandPatrick Steinhardt, Oct 26, 2023
  45. 09/12 builtin/show-ref: ensure mutual exclusiveness of subcommandsPatrick Steinhardt, Oct 26, 2023
  46. Taylor BlauOct 30, 2023
  47. Patrick SteinhardtOct 31, 2023
  48. 10/12 builtin/show-ref: explicitly spell out different modes in synopsisPatrick Steinhardt, Oct 26, 2023
  49. 11/12 builtin/show-ref: add new mode to check for reference existencePatrick Steinhardt, Oct 26, 2023
  50. 12/12 t: use git-show-ref(1) to check for ref existencePatrick Steinhardt, Oct 26, 2023
  51. Taylor BlauOct 30, 2023
  52. Junio C HamanoOct 31, 2023
  53. 00/12 builtin/show-ref: introduce mode to check for ref existencePatrick Steinhardt, Oct 31, 2023
  54. 01/12 builtin/show-ref: convert pattern to a local variablePatrick Steinhardt, Oct 31, 2023
  55. 02/12 builtin/show-ref: split up different subcommandsPatrick Steinhardt, Oct 31, 2023
  56. 03/12 builtin/show-ref: fix leaking string bufferPatrick Steinhardt, Oct 31, 2023
  57. 04/12 builtin/show-ref: fix dead code when passing patternsPatrick Steinhardt, Oct 31, 2023
  58. 05/12 builtin/show-ref: refactor `--exclude-existing` optionsPatrick Steinhardt, Oct 31, 2023
  59. 06/12 builtin/show-ref: stop using global variable to count matchesPatrick Steinhardt, Oct 31, 2023
  60. 07/12 builtin/show-ref: stop using global vars for `show_one()`Patrick Steinhardt, Oct 31, 2023
  61. 08/12 builtin/show-ref: refactor options for patterns subcommandPatrick Steinhardt, Oct 31, 2023
  62. 09/12 builtin/show-ref: ensure mutual exclusiveness of subcommandsPatrick Steinhardt, Oct 31, 2023
  63. 10/12 builtin/show-ref: explicitly spell out different modes in synopsisPatrick Steinhardt, Oct 31, 2023
  64. 11/12 builtin/show-ref: add new mode to check for reference existencePatrick Steinhardt, Oct 31, 2023
  65. 12/12 t: use git-show-ref(1) to check for ref existencePatrick Steinhardt, Oct 31, 2023
  66. Taylor BlauOct 31, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.