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

Replaced the default slug-fn with a more robust version. #236

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

Conversation

wiseman
Copy link
Contributor

@wiseman wiseman commented May 23, 2019

The previous default slug-fn used by the slug task assumed
Jekyll-style naming, e.g., "2001-01-01-my-post.md", and if this
assumption was violated it would return non-sensical results.

Previous slug-fn:

Source filename Slug Final filename
"2001-01-01-my-post.md" "my-post" "my-post.html"
"index.md" "" ".html"
"my-great-new-post.md" "post" "post.html"

The new default slug-fn handles Jekyll-compliant names in the same
way, but if given a filename that doesn't fit the Jekyll format it
will just strip the extension.

New slug-fn:

Source filename Slug Final filename
"2001-01-01-my-post.md" "my-post" "my-post.html"
"index.md" "index" "index.html"
"my-great-new-post.md" "my-great-new-post" "my-great-new-post.html"

I think think it's worth changing the default to a function that gives useful results in more cases, especially since stock perun doesn't use the date info in the filename for anything, making the Jekyll convention unnecessary.

The previous default slug-fn used by the slug task assumed
Jekyll-style naming, e.g., "2001-01-01-my-post.md", and if this
assumption was violated it would return non-sensical results.

Previous slug-fn:

| Source filename         | Slug                | Final filename           |
|-------------------------|---------------------|--------------------------|
| "2001-01-01-my-post.md" | "my-post"           | "my-post.html"           |
| "index.md"              | ""                  | ".html"                  |
| "my-great-new-post.md"  | "post"              | "post.html"              |

The new default slug-fn handles Jekyll-compliant names in the same
way, but if given a filename that doesn't fit the Jekyll format it
will just strip the extension.

New slug-fn:

| Source filename         | Slug                | Final filename           |
|-------------------------|---------------------|--------------------------|
| "2001-01-01-my-post.md" | "my-post"           | "my-post.html"           |
| "index.md"              | "index"             | "index.html"             |
| "my-great-new-post.md"  | "my-great-new-post" | "my-great-new-post.html" |
@allentiak
Copy link
Contributor

allentiak commented May 23, 2019 via email

Copy link
Member

@podviaznikov podviaznikov left a comment

Choose a reason for hiding this comment

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

@wiseman this looks great!
Thank you for PR. Especially for giving example of old and new behavior.
I'll approve it. Also if you have time, it will be cool if you can implement this desired behavior in unit tests. We do have some unit tests, so this should be doable - just test slug function itself.

@allentiak
Copy link
Contributor

allentiak commented May 23, 2019 via email

@arichiardi
Copy link
Collaborator

arichiardi commented Mar 4, 2021

Should we bring this to completion? I can help with rebase very likely in the next couple of weeks

@allentiak
Copy link
Contributor

Not me, sorry.
#241 (comment)

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.

4 participants