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

A better Slug method #454

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

A better Slug method #454

wants to merge 2 commits into from

Conversation

bakanis
Copy link

@bakanis bakanis commented Jan 9, 2014

Latin-ish inputs should have very stable output. All inputs are passed through an NFKD transform, and anything still in the unicode Letter and Number categories are passed through intact. Anything in the Mark or Lm/Sk categories (modifiers) are skipped, and runs of characters from any other categories are collapsed to a single hyphen.

Taken from https://github.com/extemporalgenome/slug

Output e.g

Original strings of input text

My daughter's dog
Šešios žąsys su šešiais žąsyčiais
Дру́жба дру́жбой, а слу́жба слу́жбой
循序渐进 [循序漸進]


revel slug output:
my-daughters-dog
eios-sys-su-eiais-syiais


new slug output:
my-daughter-s-dog
sesios-zasys-su-sesiais-zasyciais
дружба-дружбои-а-служба-службои
循序渐进-循序漸進

/*
* credits go to https://github.com/extemporalgenome/slug
*/
func Slug(s string) string {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason for the short s variable name? I think it would be better if we tried to use descriptive variable names. Perhaps slug would be better?

@brendensoares
Copy link
Member

We have some questions that still need answering and I would also like to see some unit tests added before we merge this into the code base.

Because of that, I've pushed this PR back to the next release. When the above needs are met, I ask that you create a new pull request that targets the develop branch along with a dedicated feature branch on your forked repo per https://github.com/robfig/revel/blob/master/CONTRIBUTING.md#how-revel-is-developed

Thank you for your contribution thus far. Let's get this added soon!

@brendensoares brendensoares modified the milestones: v0.10, v0.9 Feb 16, 2014
@brendensoares brendensoares modified the milestones: Backlog, v0.10 Feb 19, 2014
@dmage
Copy link

dmage commented May 11, 2014

For Дру́жба дру́жбой, а слу́жба слу́жбой good versions are

дружба-дружбой-а-служба-службой
druzhba-druzhboy-a-sluzhba-sluzhboy

For URLs latest is preferred (because it wouldn't be skewed by url encoding).

But дружба-дружбои-а-служба-службои is awful. й != и

@@ -14,6 +14,8 @@ import (
"strconv"
"strings"
"time"
"code.google.com/p/go.text/unicode/norm"
Copy link
Contributor

@jeevatkm jeevatkm May 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently this package is not exists, its been changed to golang.org/x/text/unicode/norm

@brendensoares
Copy link
Member

@bakanis are you still interested in taking the lead on this PR or should we take it over?

@notzippy
Copy link
Collaborator

Suggest using this version instead https://github.com/gosimple/slug

@notzippy notzippy added the Revel 2 Possibility for Revel 2 label Apr 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Revel 2 Possibility for Revel 2 topic-routing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants