Use case-insensitive locale-aware sorting for Acknowledgements#41
Conversation
Replace inconsistent sorting: Comparable used case-sensitive comparison while all() used .lowercased(). Now both use localizedStandardCompare via the Comparable conformance, which handles locale-specific ordering. Closes #38
There was a problem hiding this comment.
Pull request overview
This PR unifies the Acknowledgement sorting behavior to be case-insensitive and locale-aware by switching the Comparable implementation from plain string < to localizedStandardCompare(_:), and simplifying all() to delegate to .sorted(). It also updates tests to validate the new case-insensitive ordering.
Changes:
Acknowledgement.Comparablenow useslocalizedStandardCompare(_:)instead of raw<Acknowledgement.all()simplified toacks.sorted(), delegating toComparable- Tests renamed and updated to reflect case-insensitive expectations
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
Sources/AckGenCore/Acknowledgement.swift |
Switches < to localizedStandardCompare; simplifies all() to .sorted() |
Tests/AckGenTests/AcknowledgementTests.swift |
Renames test and updates expected sort order to case-insensitive |
Tests/AckGenTests/AckGenTests.swift |
Updates testAllReturnsSorted to use .sorted() instead of the old lowercased closure |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let sorted = items.sorted() | ||
| XCTAssertEqual(sorted.map(\.title), ["apple", "Mango", "Zebra"]) |
There was a problem hiding this comment.
In testAllReturnsSorted, the test creates a plist file on disk and initializes bundle, but neither is actually used — Acknowledgement.all(fromPlist:in:) is never called in this test. The plist is written to a temp directory and then deleted, but the sorting is verified by calling items.sorted() directly on the local array, bypassing the all() method entirely.
This means the test does not actually validate that Acknowledgement.all(fromPlist:in:) returns a correctly sorted result. The bundle setup and file I/O are dead code. The test should either be updated to call Acknowledgement.all(fromPlist:in:) with a properly resolved bundle (or a custom URL-based loader), or the dead code should be removed and a comment added explaining why all() is not directly tested here.
Summary
Comparablenow useslocalizedStandardCompareinstead of plain<all()delegates to.sorted()instead of using a separate.lowercased()closureSupersedes #38 (which targeted the already-merged
feature/bug-fixes-and-testsbranch).Test plan
testSortingIsCaseInsensitivevalidates ["apple", "Banana", "Zebra"] ordering