#375831 - Sort empty file extensions correctly
ClosedPublic

Authored by palant on Feb 2 2017, 8:35 PM.

Details

Reviewers
martinkostolny
abika
asensi
Group Reviewers
Krusader
Summary

This change makes sure that empty strings are sorted before non-empty strings. Note that it is treating the case where both strings are equal specially, this is for consistency with the logic at the bottom of compareTextsAlphabetical() and compareTextsCharacterCode(). I don't see a difference if I leave out this case but if it is important then personally I would have written this differently:

if (aS2.length() == 0) {
    return false;
} else if (aS1.length() == 0) {
    return true;
}

The checks in compareTextsAlphabetical() can be simplified similarly:

if (lPositionS2 == aS2.length()) return false;
else if (lPositionS1 == aS1.length()) return true;

These two comparisons produce the same result as the six comparisons currently there.

Test Plan

See STR in bug.

Diff Detail

Repository
R167 Krusader
Lint
Lint Skipped
Unit
Unit Tests Skipped
palant retitled this revision from to #375831 - Sort empty file extensions correctly.Feb 2 2017, 8:35 PM
palant updated this object.
palant edited the test plan for this revision. (Show Details)
palant added a reviewer: Krusader.
palant set the repository for this revision to R167 Krusader.
palant added a project: Krusader.
martinkostolny accepted this revision.Feb 4 2017, 1:52 PM
martinkostolny added a reviewer: martinkostolny.
martinkostolny added a subscriber: martinkostolny.

Agreed. Thanks a lot for your work! :)

This revision is now accepted and ready to land.Feb 4 2017, 1:52 PM
abika accepted this revision.Feb 4 2017, 2:08 PM
abika added a reviewer: abika.
asensi accepted this revision.Feb 5 2017, 7:50 AM
asensi added a reviewer: asensi.
asensi added a subscriber: asensi.

Nice! Thanks, Wladimir!

abika closed this revision.Feb 13 2017, 8:53 PM

Merged with master (and Phabricator fails again to autoclose this)