From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Jeff King" <peff@peff.net>,
"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
"Leho Kraav" <leho@conversionready.com>,
git@vger.kernel.org, "SZEDER Gábor" <szeder.dev@gmail.com>
Subject: [PATCHv2 6.5/7] squash! versioncmp: use earliest-longest contained suffix to determine sorting order
Date: Thu, 8 Dec 2016 15:48:11 +0100 [thread overview]
Message-ID: <20161208144811.4372-1-szeder.dev@gmail.com> (raw)
In-Reply-To: <20161208142401.1329-7-szeder.dev@gmail.com>
As the number of identical steps to be done for both tagnames grows,
extract them into a helper function, with the additional benefit that
the conditionals near the end of swap_prereleases() will use more
meaningful variable names.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
I was not particularly happy with the amount of code duplication this
series added to swap_prereleases() in patches 5 and 6, hence this bit
of refactoring. Which I'm not particularly happy with either,
because, well, look at that diffstat.
I don't have strong feelings either way, but here it is, take it or
leave it.
versioncmp.c | 62 ++++++++++++++++++++++++++++++++++++------------------------
1 file changed, 37 insertions(+), 25 deletions(-)
diff --git a/versioncmp.c b/versioncmp.c
index 62c8d69dc..601ceddef 100644
--- a/versioncmp.c
+++ b/versioncmp.c
@@ -24,6 +24,29 @@
static const struct string_list *prereleases;
static int initialized;
+struct suffix_match {
+ int conf_pos;
+ int start;
+ int len;
+};
+
+static void find_better_matching_suffix(const char *tagname, const char *suffix,
+ int suffix_len, int start, int conf_pos,
+ struct suffix_match *match)
+{
+ /* A better match either starts earlier or starts at the same offset
+ * but is longer. */
+ int end = match->len < suffix_len ? match->start : match->start-1;
+ int i;
+ for (i = start; i <= end; i++)
+ if (starts_with(tagname + i, suffix)) {
+ match->conf_pos = conf_pos;
+ match->start = i;
+ match->len = suffix_len;
+ break;
+ }
+}
+
/*
* off is the offset of the first different character in the two strings
* s1 and s2. If either s1 or s2 contains a prerelease suffix containing
@@ -47,46 +70,35 @@ static int swap_prereleases(const char *s1,
int off,
int *diff)
{
- int i, i1 = -1, i2 = -1;
- int start_at1 = off, start_at2 = off, match_len1 = -1, match_len2 = -1;
+ int i;
+ struct suffix_match match1 = { -1, off, -1 };
+ struct suffix_match match2 = { -1, off, -1 };
for (i = 0; i < prereleases->nr; i++) {
const char *suffix = prereleases->items[i].string;
- int j, start, end, suffix_len = strlen(suffix);
+ int start, suffix_len = strlen(suffix);
if (suffix_len < off)
start = off - suffix_len;
else
start = 0;
- end = match_len1 < suffix_len ? start_at1 : start_at1-1;
- for (j = start; j <= end; j++)
- if (starts_with(s1 + j, suffix)) {
- i1 = i;
- start_at1 = j;
- match_len1 = suffix_len;
- break;
- }
- end = match_len2 < suffix_len ? start_at2 : start_at2-1;
- for (j = start; j <= end; j++)
- if (starts_with(s2 + j, suffix)) {
- i2 = i;
- start_at2 = j;
- match_len2 = suffix_len;
- break;
- }
+ find_better_matching_suffix(s1, suffix, suffix_len, start,
+ i, &match1);
+ find_better_matching_suffix(s2, suffix, suffix_len, start,
+ i, &match2);
}
- if (i1 == -1 && i2 == -1)
+ if (match1.conf_pos == -1 && match2.conf_pos == -1)
return 0;
- if (i1 == i2)
+ if (match1.conf_pos == match2.conf_pos)
/* Found the same suffix in both, e.g. "-rc" in "v1.0-rcX"
* and "v1.0-rcY": the caller should decide based on "X"
* and "Y". */
return 0;
- if (i1 >= 0 && i2 >= 0)
- *diff = i1 - i2;
- else if (i1 >= 0)
+ if (match1.conf_pos >= 0 && match2.conf_pos >= 0)
+ *diff = match1.conf_pos - match2.conf_pos;
+ else if (match1.conf_pos >= 0)
*diff = -1;
- else /* if (i2 >= 0) */
+ else /* if (match2.conf_pos >= 0) */
*diff = 1;
return 1;
}
--
2.11.0.78.g5a2d011
next prev parent reply other threads:[~2016-12-08 14:48 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-05 22:42 2.10.0: multiple versionsort.prereleasesuffix buggy? Leho Kraav (Conversion Ready)
2016-09-05 23:21 ` Jeff King
2016-09-06 1:07 ` SZEDER Gábor
2016-09-06 4:07 ` Jeff King
2016-09-06 19:45 ` SZEDER Gábor
2016-09-07 15:12 ` [PATCH 0/5] Fix version sort prerelease reordering bug SZEDER Gábor
2016-09-07 15:12 ` [PATCH 1/5] t7004-tag: delete unnecessary tags with test_when_finished SZEDER Gábor
2016-09-07 15:12 ` [PATCH 2/5] t7004-tag: use test_config helper SZEDER Gábor
2016-09-07 15:12 ` [PATCH 3/5] t7004-tag: add version sort tests to show prerelease reordering issues SZEDER Gábor
2016-09-07 15:12 ` [PATCH 4/5] versioncmp: pass full tagnames to swap_prereleases() SZEDER Gábor
2016-09-08 17:49 ` Junio C Hamano
2016-09-08 20:37 ` SZEDER Gábor
2016-09-08 21:31 ` Junio C Hamano
2016-09-07 15:12 ` [PATCH 5/5] versioncmp: cope with common leading parts in versionsort.prereleaseSuffix SZEDER Gábor
2016-09-07 15:48 ` SZEDER Gábor
2016-09-09 10:43 ` Duy Nguyen
2016-10-05 1:33 ` SZEDER Gábor
2016-10-05 17:01 ` Junio C Hamano
2016-10-05 21:26 ` SZEDER Gábor
2016-10-05 22:15 ` Junio C Hamano
2016-10-06 0:40 ` Jacob Keller
2016-10-06 5:48 ` Duy Nguyen
2016-12-08 14:23 ` [PATCHv2 0/7] Fix and generalize version sort reordering SZEDER Gábor
2016-12-08 14:23 ` [PATCHv2 1/7] t7004-tag: delete unnecessary tags with test_when_finished SZEDER Gábor
2016-12-08 14:23 ` [PATCHv2 2/7] t7004-tag: use test_config helper SZEDER Gábor
2016-12-08 14:23 ` [PATCHv2 3/7] t7004-tag: add version sort tests to show prerelease reordering issues SZEDER Gábor
2016-12-08 14:23 ` [PATCHv2 4/7] versioncmp: pass full tagnames to swap_prereleases() SZEDER Gábor
2016-12-08 14:23 ` [PATCHv2 5/7] versioncmp: cope with common part overlapping with prerelease suffix SZEDER Gábor
2016-12-12 21:27 ` Junio C Hamano
2016-12-13 0:27 ` SZEDER Gábor
2016-12-13 6:39 ` Junio C Hamano
2016-12-08 14:24 ` [PATCHv2 6/7] versioncmp: use earliest-longest contained suffix to determine sorting order SZEDER Gábor
2016-12-08 14:48 ` SZEDER Gábor [this message]
2016-12-08 14:24 ` [PATCHv2 7/7] versioncmp: generalize version sort suffix reordering SZEDER Gábor
2016-12-08 19:36 ` Junio C Hamano
2016-12-14 17:08 ` [PATCHv2 0/7] Fix and generalize version sort reordering Jeff King
2016-12-14 17:36 ` Junio C Hamano
2016-12-20 8:50 ` SZEDER Gábor
2016-12-20 16:49 ` Jeff King
2016-09-06 7:12 ` 2.10.0: multiple versionsort.prereleasesuffix buggy? Leho Kraav (Conversion Ready)
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=20161208144811.4372-1-szeder.dev@gmail.com \
--to=szeder.dev@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=leho@conversionready.com \
--cc=pclouds@gmail.com \
--cc=peff@peff.net \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.