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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved permalinks for pages and collections #3538

Merged
merged 1 commit into from Mar 4, 2015
Merged

Improved permalinks for pages and collections #3538

merged 1 commit into from Mar 4, 2015

Conversation

willnorris
Copy link
Contributor

This updates the default permalink style for pages and collections to match the site-wide 'permalink' setting (more detailed notes in the commit message).

I'm not wild about the method name Utils.append_permalink_ending, but at least it's accurate. I'm certainly open to suggestions for a better name there.

I also have not yet written the user-facing docs describing how this works. I'll certainly do that as part of this PR, but wanted to go ahead and get some 馃憖 on the implementation before then. I also wasn't quite sure how we handle docs for features that aren't yet in the current stable release. Do we just hold off pushing those live? Or do we just say that the docs on jekyllrb.com correspond to the latest master branch, not necessarily any particular released version?

@@ -166,4 +166,18 @@ class TestUtils < JekyllUnitTest
end
end

context "The \`Utils.append_permalink_ending\` method" do
should "handle built-in permalink styles" do
assert_equal "/:basename/", Utils.append_permalink_ending("/:basename", :pretty)
Copy link
Member

Choose a reason for hiding this comment

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

What if I have "/:basename/", :pretty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you wouldn't. The value of template is not user-supplied, it's hardcoded in the method calls in document.rb and page.rb so we always control what the value is, and the docs for the method explicitly say that template should not have a trailing slash or extension.

Copy link
Contributor

Choose a reason for hiding this comment

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

馃憥 to hardcoding anything that can be user configurable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

huh? This particular value isn't user configurable... it's the default template used for page/collection permalinks. Look at the code it is replacing... it's already hardcoded. Now we're just making the suffix match what the user has in their permalink setting in _config.yaml. Users can always provide their own per-page permalink value in the front matter, or the per-collection permalink setting in _config.yaml

@parkr
Copy link
Member

parkr commented Mar 4, 2015

Woot! Looking pretty good, just a couple questions above.

@parkr parkr modified the milestone: 3.0 Mar 4, 2015
This updates the default permalink style for pages and collections to
match the site-wide 'permalink' setting.  If the permalink setting
contains a trailing slash, either explicitly or by being set to
':pretty', then pages and collections permalinks will contain trailing
slashes by default as well.  Similarly, if the permalink setting
contains a trailing ':output_ext', so will pages and collections.  If
the permalink setting contains neither a trailing slash or extension,
neither will pages or collections.

This impacts only the default permalink structure for pages and
collections.  Permalinks set in the frontmatter of an individual page
take precedence, as does the permalink setting for a specific
collection.

Fixes #2691
@parkr
Copy link
Member

parkr commented Mar 4, 2015

I'm satisfied with this. Let's get it into the next beta so it can be thoroughly tested before we release it. :shipit:

We'll also want to keep an eye on how it affects performance, if at all.

@willnorris
Copy link
Contributor Author

@parkr: what about user-facing docs (e.g. http://jekyllrb.com/docs/permalinks/)? How do we handle that for features like this that are not in a stable release yet?

@envygeeks
Copy link
Contributor

@parkr @willnorris Do we not have edge docs?

@willnorris
Copy link
Contributor Author

@envygeeks we very well might, I'm just not completely up to speed on how all of that is setup. I can write the docs as part of this PR, but just want to make sure they don't accidentally get published somewhere they shouldn't.

@willnorris
Copy link
Contributor Author

I'm satisfied with this. Let's get it into the next beta so it can be thoroughly tested before we release it. :shipit:

I'm going to go ahead and check this in so we can start testing it. I'll do the user docs in a separate PR, and can figure out then what to do about edge docs.

@parkr
Copy link
Member

parkr commented Mar 4, 2015

We don't have edge docs. I like tip.golang.org, but it's hard to keep fixes and changes due to upgrading features separate. We'd have to argue that once we release something, the docs are as-is and you just go to like jekyllrb.com/2.5.3/ and it's the docs as they were at the v2.5.3 tag.

We usually add a note, check out the bottom of this: http://jekyllrb.com/docs/home/

willnorris added a commit that referenced this pull request Mar 4, 2015
Improved permalinks for pages and collections
@willnorris willnorris merged commit f6f2626 into jekyll:master Mar 4, 2015
willnorris added a commit that referenced this pull request Mar 4, 2015
@willnorris
Copy link
Contributor Author

ah, started merging this pr just before seeing your comment :). I'll add an unreleased note for the relevant changes to the permalink docs.

@parkr
Copy link
Member

parkr commented Mar 5, 2015

Thank you! Please ref this pr in that commit.

@willnorris willnorris deleted the permalink branch March 5, 2015 04:12
kevinoid added a commit to kevinoid/kevinlocke.name that referenced this pull request Jul 23, 2016
Due to the changes in jekyll/jekyll@90865d (Jekyll 3.1.0+) .xhtml pages
are not treated like .html pages which are placed in subdirectories
based on the sitewide permalink setting (from jekyll/jekyll#3538).

To avoid this, set the sitewide permalink setting to "none" and set a
permalink default value for drafts and pages with the desired permalink
for those types.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
@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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants