Fixes handling of Turkish I
#153
10000
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
Fixes handling of Turkish I and introduces maximum length support in slug generation.
- Updates the TurkishEncodingOfI test expected output and removes the Skip attribute.
- Adds a new theory test for MaximumLength behavior.
- Switches to ToLowerInvariant for consistent lower casing and introduces MaximumLength configuration in production code.
- Updates README.md documentation to include the new MaximumLength option.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/Slugify.Core.Tests/SlugHelperTests.cs | Updated Turkish I test expected output and added MaximumLength tests |
| src/Slugify.Core/SlugHelper.cs | Changed to using ToLowerInvariant and implemented MaximumLength trimming logic |
| src/Slugify.Core/SlugHelperConfiguration.cs | Added MaximumLength property to configuration |
| README.md | Documented the new MaximumLength option |
Comments suppressed due to low confidence (1)
tests/Slugify.Core.Tests/SlugHelperTests.cs:713
- The expected output in the TurkishEncodingOfI test was changed from 'fıfa 18' to 'fifa-18'. Validate that this updated expected value properly reflects the intended Turkish lowercase behavior.
const string expected = "fifa-18";
There was a problem hiding this comment.
Pull Request Overview
This PR addresses the handling of the Turkish letter "I" by updating case conversion to use an invariant method, ensuring consistent slug generation.
- Updates case conversion in SlugHelper to use ToLowerInvariant
- Adjusts the Turkish encoding test by removing the skip and updating the expected output
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Slugify.Core/SlugHelper.cs | Changed case conversion to use ToLowerInvariant for culture-independent behavior |
| tests/Slugify.Core.Tests/SlugHelperTests.cs | Enabled and updated the Turkish encoding test to reflect invariant slug output changes |
Comments suppressed due to low confidence (1)
tests/Slugify.Core.Tests/SlugHelperTests.cs:711
- The test sets CultureInfo.CurrentCulture without restoring the original value, which may affect other tests. Consider saving the original culture and restoring it in a finally block.
CultureInfo.CurrentCulture = new CultureInfo("tr-TR");
CultureInfo.CurrentCulture = new CultureInfo("tr-TR");
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR builds on the changes in #151
Fixes #10