Conversation
Though the separator can be overridden, this has been using the default
separator ('-') behind the scenes then replacing it with the chosen
separator as the last step. This means, among other things, that hyphens
would always be replaced with the separator, even if you overrode the
allowed characters regex to try to allow them.
This changes `slugify` to use the provided `separator` throughout, which
required some changes to the allowed-characters and multiple-separators
logic.
Since `slugify` is no longer using '-' as a separator behind the scenes and switching at the last minute, the "collapse multiple separators" and "truncate" steps are now operating on the text with the chosen separator rather than on one with hyphens. Which changes the expected output of a couple tests. - test_multi_character_separator is different because the multi-character separator now counts against the length calculation, so it had to drop one piece. - test_regex_pattern_keep_underscore_with_underscore_as_separator now strips leading and trailing underscores, rather than just collapsing them. To me the new behavior seems more correct.
Adds two tests, both with an overridden separator, to exercise the new behavior around hyphens. 'test_separator_remove_hyphens' will behave the same as it would have before, but for different reasons. 'test_regex_pattern_allow_hyphens' will now pass, when it would have failed (replaced hyphens even though they're supposed to be allowed) before.
2c47a53 to
0bff01f
Compare
|
This is to generate a human-readable set of words. Dash-separated words that is. Anything else is out of the scope of this package. Feel free to fork and adapt for your own edge-case. |
|
The It also seems like a bug to me that |
|
This is Dojo, posting a maintainer-authorized follow-up linking this PR to #191. Partially addressed: modern mode budgets actual emitted separator width. Literal-hyphen preservation is NOT implemented; both algorithms retain global dash mapping. The default algorithm remains legacy; improved output rules are opt-in. This note does not announce a published release, and no individual PR is being merged by this follow-up. Thank you for the contribution and discussion. 🚀 Generated with Dojo ⛩️ |
3 participants
The underlying motivation for this is that I wanted to be able to slugify text that contains hyphens and keep the hyphens. It seemed like that should work with
i.e. that that should produce 'hyphen-having_text_ftw'. In fact it produced 'hyphen_having_text_ftw' because, behind the scenes, this was always using hyphen as the separator, just switching it out and the end if the separator was overridden, which means hyphen couldn't be an allowed character.
This changes it to use the specified separator from the beginning.
One side effect of this is that the test with a multi-character separator and
max_length=20produces a different result, because the multi-character separator counts against the length. This seems like an improvement to me.