Skip to content

Conversation

jastax
Copy link
Contributor

@jastax jastax commented Apr 4, 2017

Implements #343

@jastax
Copy link
Contributor Author

jastax commented Apr 4, 2017

Not sure, if this is what was requested though ;)

public String slug(@Name("text") String text, @Name(value = "delim", defaultValue = "-") String delim) {
if (text == null) return null;
if (delim == null) return null;
return text.replace(" ", delim).replace(delim+delim, delim);
Copy link
Member

Choose a reason for hiding this comment

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

probably use replaceAll("\s+",delim) for all whitespace
or \W+ for non-word-characters

@jexp
Copy link
Member

jexp commented Apr 5, 2017

Looks good, please have a look at my comment. Perhpas also test for subsequent space?

@jastax
Copy link
Contributor Author

jastax commented Apr 5, 2017

Better like this? :) Anything else to improve?

@jexp
Copy link
Member

jexp commented Apr 6, 2017

something is wrong now, there is a lot of other changes, can you rebase your work onto the latest master state? and then push to the PR branch again

@jastax
Copy link
Contributor Author

jastax commented Apr 6, 2017

sure, sorry :)

public String slug(@Name("text") String text, @Name(value = "delim", defaultValue = "-") String delim) {
if (text == null) return null;
if (delim == null) return null;
return text.trim()
Copy link
Member

Choose a reason for hiding this comment

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

it's actually much easier `.replaceAll("[\W\s]+","delim")

@jexp jexp merged commit 81f60c7 into neo4j-contrib:3.1 Apr 8, 2017
@jexp
Copy link
Member

jexp commented Apr 8, 2017

Thanks a lot !!

jexp pushed a commit that referenced this pull request Apr 12, 2017
albertodelazzari pushed a commit to albertodelazzari/neo4j-apoc-procedures that referenced this pull request Jun 28, 2017
@jastax jastax deleted the text-slug branch March 28, 2018 18:56
@tomswinkels
Copy link

tomswinkels commented Jul 3, 2018

I found a problem, the slug is not set to lowercase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants