Strip slashes on nil url tokens #3325

Merged
merged 3 commits into from Jan 24, 2015

Conversation

Projects
None yet
4 participants
@imathis
Contributor

imathis commented Jan 18, 2015

This will make it cleaner to generate urls with optional keys in the permalink template.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jan 18, 2015

Member

Excellent, thank you! Would you mind adding a unit test? I believe they're in test/test_url.rb. If not, no problem. I'll add them at merge time! Thanks 😄

Member

parkr commented Jan 18, 2015

Excellent, thank you! Would you mind adding a unit test? I believe they're in test/test_url.rb. If not, no problem. I'll add them at merge time! Thanks 😄

@parkr parkr added the fix label Jan 18, 2015

@parkr parkr added this to the 3.0 milestone Jan 18, 2015

@parkr parkr self-assigned this Jan 18, 2015

@imathis

This comment has been minimized.

Show comment
Hide comment
@imathis

imathis Jan 18, 2015

Contributor

Request granted :)

Contributor

imathis commented Jan 18, 2015

Request granted :)

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jan 18, 2015

Should these values be switched around such that template is /:x/:y/:z/? The permalink will override the template so you can remove that key as well.

Should these values be switched around such that template is /:x/:y/:z/? The permalink will override the template so you can remove that key as well.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jan 18, 2015

Member

Thanks!

Member

parkr commented Jan 18, 2015

Thanks!

@imathis

This comment has been minimized.

Show comment
Hide comment
@imathis

imathis Jan 19, 2015

Contributor

@parkr. Done.

One other thing though. As I'm working with permalinks I'm finding that the default template names — pretty, date, etc — only work in the site's permalink config. Setting permalink: date in a post's front-matter puts the post at /date/index.html which is totally not what I was expecting.

Can you confirm that this is how it's supposed to work? That's not the impression I got from the docs at all.

Contributor

imathis commented Jan 19, 2015

@parkr. Done.

One other thing though. As I'm working with permalinks I'm finding that the default template names — pretty, date, etc — only work in the site's permalink config. Setting permalink: date in a post's front-matter puts the post at /date/index.html which is totally not what I was expecting.

Can you confirm that this is how it's supposed to work? That's not the impression I got from the docs at all.

@parkr parkr assigned alfredxing and unassigned parkr Jan 24, 2015

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jan 24, 2015

Member

LGTM! :shipit:

As I'm working with permalinks I'm finding that the default template names — pretty, date, etc — only work in the site's permalink config. Setting permalink: date in a post's front-matter puts the post at /date/index.html which is totally not what I was expecting.

Total parity would be nice. The only problem is that permalink in a post is considered as-is. It's an experiment but I don't want it to go awry! Haha. @benbalter, do you think allowing the special url template names (e.g. permalink: pretty) inside a post's YAML front matter is risky? Seems like a smart idea to me but I don't want to go being clever and inadvertently break things. 😉 Thanks!

Member

parkr commented Jan 24, 2015

LGTM! :shipit:

As I'm working with permalinks I'm finding that the default template names — pretty, date, etc — only work in the site's permalink config. Setting permalink: date in a post's front-matter puts the post at /date/index.html which is totally not what I was expecting.

Total parity would be nice. The only problem is that permalink in a post is considered as-is. It's an experiment but I don't want it to go awry! Haha. @benbalter, do you think allowing the special url template names (e.g. permalink: pretty) inside a post's YAML front matter is risky? Seems like a smart idea to me but I don't want to go being clever and inadvertently break things. 😉 Thanks!

@alfredxing alfredxing merged commit 31b03e9 into jekyll:master Jan 24, 2015

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jan 24, 2015

Member

🎉 🎊

Member

parkr commented Jan 24, 2015

🎉 🎊

@alfredxing

This comment has been minimized.

Show comment
Hide comment
@alfredxing

alfredxing Jan 24, 2015

Member

🙏

Member

alfredxing commented Jan 24, 2015

🙏

@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.