Re: [PATCH v2] tag: support configuring --sort via .gitconfig
From: "Keller, Jacob E" <jacob.e.keller@intel.com>
Date: 2016-06-15 23:01:52
On Thu, 2014-07-10 at 00:07 -0400, Jeff King wrote:
On Wed, Jul 09, 2014 at 03:36:51PM -0700, Jacob Keller wrote:quoted
+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).
I am fine with this being another patch or squashed in. I didn't even know that was available :) I also like putting the two equivalent conditionals into the same if block. Thanks, Jake
quoted hunk ↗ jump to hunk
-- >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;