On Mon, Dec 12, 2016 at 10:27 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> SZEDER Gábor <szeder.dev@xxxxxxxxx> writes:
>
>> diff --git a/versioncmp.c b/versioncmp.c
>> index a55c23ad5..f86ac562e 100644
>> --- a/versioncmp.c
>> +++ b/versioncmp.c
>> @@ -26,12 +26,15 @@ static int initialized;
>>
>> /*
>> * 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.
>> + * s1 and s2. If either s1 or s2 contains a prerelease suffix containing
>> + * that offset, then that string will be forced to be on top.
>> *
>> - * If both s1 and s2 contain a (different) suffix at that position,
>> + * If both s1 and s2 contain a (different) suffix around that position,
>> * their order is determined by the order of those two suffixes in the
>> * configuration.
>> + * If any of the strings contains more than one different suffixes around
>> + * that position, then that string is sorted according to the contained
>> + * suffix which comes first in the configuration.
>> *
>> * Return non-zero if *diff contains the return value for versioncmp()
>> */
>> @@ -44,10 +47,21 @@ static int swap_prereleases(const char *s1,
>>
>> for (i = 0; i < prereleases->nr; i++) {
>> const char *suffix = prereleases->items[i].string;
>> - if (i1 == -1 && starts_with(s1 + off, suffix))
>> - i1 = i;
>> - if (i2 == -1 && starts_with(s2 + off, suffix))
>> - i2 = i;
>> + int j, start, suffix_len = strlen(suffix);
>> + if (suffix_len < off)
>> + start = off - suffix_len + 1;
>> + else
>> + start = 0;
>
> Now that this function has to rewind the beginning of the comparison
> earlier than the given 'off', it makes me wonder if it still makes
> sense for the caller to compute it in the first place.
The caller has to compute it anyway, because it must deal with all the
cases when the two compared tagnames are not reordered based on their
(prerelease)suffix.