search

simov / slugify

Slugifies a string
MIT License
1.47k stars 126 forks source link

fix: remove() should work with multichar strings #165

Closed Trott closed 1 year ago

Trott commented 1 year ago

Fixes: https://github.com/simov/slugify/issues/164

Trott commented 1 year ago

This preserves the current behavior but at the expense of a duck-type check for a regular expression combined with a new RegExp(options.remove, 'g'). I'd want to think hard about edge cases where this might be a security issue. Rather than do that thinking, I prefer the straightforward (but small breaking change) approach in #166.

Trott commented 1 year ago

Another argument for the breaking change instead of this fix: This preserves the "replace all matches of a regular expression even if it doesn't have a 'g' flag" behavior...but only for regular expressions. Strings will now only replace the first instance. (This is still a bug fix for some strings, as previously the strings didn't work at all if there were multicharacter strings specified, as in the linked issue. Those now work, ableit only for the first match.)