Web lists-archives.com

[PATCH 3/3] rev-parse: simplify parsing of ref options




All of these options do the same thing "--foo" iterates over
the "foo" refs, and "--foo=<glob>" does the same with a
glob. We can factor this into its own function to avoid
repeating ourselves.

There are two subtleties to note:

  - the original called for_each_branch_ref(), etc, in the
    non-glob case. Now we will call for_each_ref_in("refs/heads/")
    which is exactly what for_each_branch_ref() did under
    the hood.

  - for --glob, we'll call for_each_glob_ref_in() with a
    NULL "prefix" argument. Which is exactly what
    for_each_glob_ref() was doing already.

So both cases should behave identically, and it seems
reasonable to assume that this will remain the same. The
functions we are calling now are the more-generic ones, and
the ones we are dropping are just convenience wrappers.

Signed-off-by: Jeff King <peff@xxxxxxxx>
---
This actually drops the last caller for for_each_branch_ref(). I'm not
sure if we shoulder consider cleaning up the proliferation of
for_each_ref() helpers.

 builtin/rev-parse.c | 45 ++++++++++++++++-----------------------------
 1 file changed, 16 insertions(+), 29 deletions(-)

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index db1417160..76be91350 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -554,6 +554,15 @@ static int opt_with_value(const char *arg, const char *opt, const char **value)
 	return 0;
 }
 
+static void handle_ref_opt(const char *pattern, const char *prefix)
+{
+	if (pattern)
+		for_each_glob_ref_in(show_reference, pattern, prefix, NULL);
+	else
+		for_each_ref_in(prefix, show_reference, NULL);
+	clear_ref_exclusion(&ref_excludes);
+}
+
 int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 {
 	int i, as_is = 0, verify = 0, quiet = 0, revs_count = 0, type = 0;
@@ -749,42 +758,20 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				for_each_ref_in("refs/bisect/good", anti_reference, NULL);
 				continue;
 			}
-			if (skip_prefix(arg, "--branches=", &arg)) {
-				for_each_glob_ref_in(show_reference, arg,
-					"refs/heads/", NULL);
-				clear_ref_exclusion(&ref_excludes);
-				continue;
-			}
-			if (!strcmp(arg, "--branches")) {
-				for_each_branch_ref(show_reference, NULL);
-				clear_ref_exclusion(&ref_excludes);
-				continue;
-			}
-			if (skip_prefix(arg, "--tags=", &arg)) {
-				for_each_glob_ref_in(show_reference, arg,
-					"refs/tags/", NULL);
-				clear_ref_exclusion(&ref_excludes);
+			if (opt_with_value(arg, "--branches", &arg)) {
+				handle_ref_opt(arg, "refs/heads/");
 				continue;
 			}
-			if (!strcmp(arg, "--tags")) {
-				for_each_tag_ref(show_reference, NULL);
-				clear_ref_exclusion(&ref_excludes);
+			if (opt_with_value(arg, "--tags", &arg)) {
+				handle_ref_opt(arg, "refs/tags/");
 				continue;
 			}
 			if (skip_prefix(arg, "--glob=", &arg)) {
-				for_each_glob_ref(show_reference, arg, NULL);
-				clear_ref_exclusion(&ref_excludes);
-				continue;
-			}
-			if (skip_prefix(arg, "--remotes=", &arg)) {
-				for_each_glob_ref_in(show_reference, arg,
-					"refs/remotes/", NULL);
-				clear_ref_exclusion(&ref_excludes);
+				handle_ref_opt(arg, NULL);
 				continue;
 			}
-			if (!strcmp(arg, "--remotes")) {
-				for_each_remote_ref(show_reference, NULL);
-				clear_ref_exclusion(&ref_excludes);
+			if (opt_with_value(arg, "--remotes", &arg)) {
+				handle_ref_opt(arg, "refs/remotes/");
 				continue;
 			}
 			if (skip_prefix(arg, "--exclude=", &arg)) {
-- 
2.12.0.613.g6e7c52a0d