git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
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 4/7] versioncmp: pass full tagnames to swap_prereleases()
Date: Thu,  8 Dec 2016 15:23:58 +0100	[thread overview]
Message-ID: <20161208142401.1329-5-szeder.dev@gmail.com> (raw)
In-Reply-To: <20161208142401.1329-1-szeder.dev@gmail.com>

The swap_prereleases() helper function is responsible for finding
configured prerelease suffixes in a pair of tagnames to be compared,
but this function currently gets to see only the parts of those two
tagnames starting at the first different character.  To fix some
issues related to the common part of two tagnames overlapping with
leading part of a prerelease suffix, this helper function must see
both full tagnames.

In preparation for the fix in the following patch, refactor
swap_prereleases() and its caller to pass two full tagnames and an
additional offset indicating the position of the first different
character.

While updating the comment describing that function, remove the
sentence about not dealing with both tagnames having the same suffix.
Currently it doesn't add much value (we know that there is a different
character, so it's obvious that it can't possibly be the same suffix
in both), and at the end of this patch series it won't even be true
anymore.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 versioncmp.c | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/versioncmp.c b/versioncmp.c
index 80bfd109f..a55c23ad5 100644
--- a/versioncmp.c
+++ b/versioncmp.c
@@ -25,32 +25,28 @@ static const struct string_list *prereleases;
 static int initialized;
 
 /*
- * p1 and p2 point to the first different character in two strings. If
- * either p1 or p2 starts with a prerelease suffix, it will be forced
- * to be on top.
+ * 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 starting
+ * at that offset, it will be forced to be on top.
  *
- * If both p1 and p2 start with (different) suffix, the order is
- * determined by config file.
- *
- * Note that we don't have to deal with the situation when both p1 and
- * p2 start with the same suffix because the common part is already
- * consumed by the caller.
+ * If both s1 and s2 contain a (different) suffix at that position,
+ * their order is determined by the order of those two suffixes in the
+ * configuration.
  *
  * Return non-zero if *diff contains the return value for versioncmp()
  */
-static int swap_prereleases(const void *p1_,
-			    const void *p2_,
+static int swap_prereleases(const char *s1,
+			    const char *s2,
+			    int off,
 			    int *diff)
 {
-	const char *p1 = p1_;
-	const char *p2 = p2_;
 	int i, i1 = -1, i2 = -1;
 
 	for (i = 0; i < prereleases->nr; i++) {
 		const char *suffix = prereleases->items[i].string;
-		if (i1 == -1 && starts_with(p1, suffix))
+		if (i1 == -1 && starts_with(s1 + off, suffix))
 			i1 = i;
-		if (i2 == -1 && starts_with(p2, suffix))
+		if (i2 == -1 && starts_with(s2 + off, suffix))
 			i2 = i;
 	}
 	if (i1 == -1 && i2 == -1)
@@ -121,7 +117,8 @@ int versioncmp(const char *s1, const char *s2)
 		initialized = 1;
 		prereleases = git_config_get_value_multi("versionsort.prereleasesuffix");
 	}
-	if (prereleases && swap_prereleases(p1 - 1, p2 - 1, &diff))
+	if (prereleases && swap_prereleases(s1, s2, (const char *) p1 - s1 - 1,
+					    &diff))
 		return diff;
 
 	state = result_type[state * 3 + (((c2 == '0') + (isdigit (c2) != 0)))];
-- 
2.11.0.78.g5a2d011


  parent reply	other threads:[~2016-12-08 14:24 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                   ` SZEDER Gábor [this message]
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                     ` [PATCHv2 6.5/7] squash! " SZEDER Gábor
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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161208142401.1329-5-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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).