Typos, test coverage, and documentation#883
Typos, test coverage, and documentation#883vekovius wants to merge 4 commits intopermissionlesstech:mainfrom
Conversation
…lor+Peer - Add comprehensive documentation to String+DJB2 explaining the hash algorithm and its use case - Add enhanced documentation to String+Nickname with usage examples - Add StringUtilsTests with 24 test cases covering: * DJB2 hash consistency, case sensitivity, unicode handling * Nickname suffix parsing edge cases and validation - Add ColorPeerTests with 20 test cases covering: * Color consistency and caching behavior * Dark/light mode color differentiation * Orange hue avoidance for mention highlighting * Real-world scenarios with Nostr pubkeys and edge cases
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @Test func peerColor_sameSeedProducesSameColor() { | ||
| let color1 = Color(peerSeed: "alice", isDark: false) | ||
| let color2 = Color(peerSeed: "alice", isDark: false) | ||
|
|
||
| // Since the cache is static, both should produce the same color | ||
| // We can't directly compare Color objects, but we can verify they produce consistent hashes | ||
| #expect(color1.description == color2.description, "Same seed should produce same color") | ||
| } | ||
|
|
||
| @Test func peerColor_differentSeedsProduceDifferentColors() { | ||
| let color1 = Color(peerSeed: "alice", isDark: false) | ||
| let color2 = Color(peerSeed: "bob", isDark: false) |
There was a problem hiding this comment.
The new tests call Color(peerSeed:isDark:) but the production module does not define such an initializer anywhere in the repository. Because SwiftUI’s Color has no initializer with this signature, the test suite will not even compile once this file is included in the build. Either the initializer should be implemented in the app target or the tests need to use an existing API.
Useful? React with 👍 / 👎.
| func splitSuffix() -> (String, String) { | ||
| let name = self.replacingOccurrences(of: "@", with: "") | ||
| guard name.count >= 5 else { return (name, "") } | ||
| let suffix = String(name.suffix(5)) | ||
| if suffix.first == "#", suffix.dropFirst().allSatisfy({ c in | ||
| ("0"..."9").contains(String(c)) || ("a"..."f").contains(String(c)) || ("A"..."F").contains(String(c)) | ||
| }) { | ||
| let base = String(name.dropLast(5)) | ||
| return (base, suffix) |
There was a problem hiding this comment.
The new splitSuffix() treats any string whose last five characters look like # plus four hex digits as a valid suffix. This means names such as "test##1234" are parsed into base "test#" and suffix "#1234", even though the extra # indicates the string does not follow the expected <name>#<hex> form. The accompanying tests expect such inputs to be rejected, so the implementation should also verify that the character before the suffix is not '#' (or otherwise validate that only one delimiter is present) before splitting.
Useful? React with 👍 / 👎.
|
please resolve conflicts and remove the landscape mode |
3 participants
Uh oh!
There was an error while loading. Please reload this page.