gosimple / slug

URL-friendly slugify with multiple languages support.
Mozilla Public License 2.0
1.15k stars 108 forks source link

fix edge case in truncate function allowing too long slugs #74

Closed Redlinkk closed 2 years ago

Redlinkk commented 2 years ago

Fixes https://github.com/gosimple/slug/issues/72

I'm not using operations on strings to avoid unnecessary allocations. I've added a test case for the fixed issue.

codecov[bot] commented 2 years ago

Codecov Report

Merging #74 (c05c02d) into master (452645f) will not change coverage. The diff coverage is 100.00%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #74 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 2 2 Lines 150 142 -8 ========================================= - Hits 150 142 -8 ``` | [Impacted Files](https://codecov.io/gh/gosimple/slug/pull/74?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gosimple) | Coverage Δ | | |---|---|---| | [slug.go](https://codecov.io/gh/gosimple/slug/pull/74/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gosimple#diff-c2x1Zy5nbw==) | `100.00% <100.00%> (ø)` | |
matrixik commented 2 years ago

Hello, sorry it took me so long to review it (burn out issues).

I improved tests to have more edge cases like this and rebased with master branch. Looks like when fixing this problem you introduced another one: now it's cutting word that should not be cut.

Thank you for catching this. I will wait with releasing new version until this is merged.

matrixik commented 2 years ago
--- FAIL: TestSlugMakeSmartTruncate (0.00s)
    slug_test.go:327: 4. MaxLength = 16; Make("Dobroslaw Zybort") = "dobroslaw"; want "dobroslaw-zybort"
    slug_test.go:327: 13. MaxLength = 6; Make("abc-de") = "abc"; want "abc-de"
    slug_test.go:327: 15. MaxLength = 6; Make("abc-de-fg") = "abc"; want "abc-de"
    slug_test.go:327: 18. MaxLength = 9; Make("abc-de-fg") = "abc-de"; want "abc-de-fg"
FAIL