Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add slugify function to std #5627

Closed
marvinhagemeister opened this issue Aug 5, 2024 · 8 comments
Closed

Add slugify function to std #5627

marvinhagemeister opened this issue Aug 5, 2024 · 8 comments
Labels
good first issue Good for newcomers PR welcome A pull request for this issue would be welcome text

Comments

@marvinhagemeister
Copy link
Contributor

Is your feature request related to a problem? Please describe.

A common part of building web apps includes generating pretty URLs based of a random string like a page title.

  • Hello World -> hello-world
  • déjà vu -> deja-vu
  • Foo & Bar -> foo-and-bar
  • ...etc

Describe the solution you'd like

Describe alternatives you've considered

@kt3k
Copy link
Member

kt3k commented Aug 5, 2024

reference: https://www.npmjs.com/package/slugify

@kt3k
Copy link
Member

kt3k commented Aug 5, 2024

Sounds like a good addition to std/text

@iuioiua iuioiua added good first issue Good for newcomers text PR welcome A pull request for this issue would be welcome labels Aug 5, 2024
@timreichen
Copy link
Contributor

I can take that one. Looking over the implementation of https://www.npmjs.com/package/slugify, there are many options I think would probably not be necessary:

{
  replacement: '-',  // replace spaces with replacement character, defaults to `-`
  remove: undefined, // remove characters that match regex, defaults to `undefined`
  lower: false,      // convert to lower case, defaults to `false`
  strict: false,     // strip special characters except replacement, defaults to `false`
  locale: 'vi',      // language code of the locale to use
  trim: true         // trim leading and trailing replacement chars, defaults to `true`
}
  • I think strict, remove, lower, trim should be done by default or that should be done manually.
  • replacement seems like it should be separator. Does this vary with slugification -? Else instead of slugify() we could have a "remove special chars" function in combination with @std/to*Case() functions.

WDYT?

@kt3k
Copy link
Member

kt3k commented Aug 6, 2024

I'm in favor of reducing number of options in first pass.

I think strict, remove, lower, trim should be done by default

Do you mean we would implement it like all these options are true, or these set to the same as npm:slugify?

replacement seems like it should be separator. Does this vary with slugification -

I don't think we need to make separator optional in first pass.

@kt3k
Copy link
Member

kt3k commented Aug 6, 2024

I'd also suggest we would use str.normalize("NFD").replace(/\p{Diacritic}/gu, "") for removing diacritic characters instead of mapping with manually maintained list.

@timreichen
Copy link
Contributor

I think strict, remove, lower, trim should be done by default

Do you mean we would implement it like all these options are true, or these set to the same as npm:slugify?

I am not too familiar with slugify(), I thought it should always be trimmed and lowercased. If so I propose to do it without an option. If not, it should be done like slugify("....").trim().toLowerCase(). Same with remove and strict.

@timreichen
Copy link
Contributor

There seems to be differences between online slugification for example for ©: https://slugify.online returns ©, https://commentpicker.com/slugify.php returns c, npm:slugify returns (c). @ on the other hand is removed by all except npm:slugify which returns unmodified @.

@kt3k
Copy link
Member

kt3k commented Aug 16, 2024

closed by #5646

@kt3k kt3k closed this as completed Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers PR welcome A pull request for this issue would be welcome text
Projects
None yet
Development

No branches or pull requests

4 participants