Re: [PATCH v2] tag: support configuring --sort via .gitconfig

On Wed, Jul 09, 2014 at 03:36:51PM -0700, Jacob Keller wrote:

+static int parse_sort_string(const char *arg)
+{
+	int sort = 0;
+	int flags = 0;
+
+	if (*arg == '-') {
+		flags |= REVERSE_SORT;
+		arg++;
+	}
+	if (starts_with(arg, "version:")) {
+		sort = VERCMP_SORT;
+		arg += 8;
+	} else if (starts_with(arg, "v:")) {
+		sort = VERCMP_SORT;
+		arg += 2;
+	} else
+		sort = STRCMP_SORT;
+	if (strcmp(arg, "refname"))
+		die(_("unsupported sort specification %s"), arg);
+	sort |= flags;
+
+	return sort;
+}

I know this is existing code you are moving, but I noticed it looks ripe
for using skip_prefix. Perhaps while we are in the area we should do the
following on top of your patch (I'd also be happy for it be squashed
in, but that may be too much in one patch).

-- >8 --
Subject: [PATCH] tag: use skip_prefix instead of magic numbers

We can make the parsing of the --sort parameter a bit more
readable by having skip_prefix keep our pointer up to date.

Signed-off-by: Jeff King <redacted>
---
 builtin/tag.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index a679e32..016df98 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c

@@ -353,18 +353,14 @@ static int parse_sort_string(const char *arg)
 	int sort = 0;
 	int flags = 0;
 
-	if (*arg == '-') {
+	if (skip_prefix(arg, "-", &arg))
 		flags |= REVERSE_SORT;
-		arg++;
-	}
-	if (starts_with(arg, "version:")) {
-		sort = VERCMP_SORT;
-		arg += 8;
-	} else if (starts_with(arg, "v:")) {
+
+	if (skip_prefix(arg, "version:", &arg) || skip_prefix(arg, "v:", &arg))
 		sort = VERCMP_SORT;
-		arg += 2;
-	} else
+	else
 		sort = STRCMP_SORT;
+
 	if (strcmp(arg, "refname"))
 		die(_("unsupported sort specification %s"), arg);
 	sort |= flags;

-- 
2.0.0.566.gfe3e6b2