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

Added path in url. #536

Merged
merged 3 commits into from Mar 10, 2013
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 5 additions & 4 deletions lib/jekyll/page.rb
Expand Up @@ -46,9 +46,9 @@ def permalink
# Returns the template String.
def template
if self.site.permalink_style == :pretty && !index? && html?
"/:basename/"
"/:path/:basename/"
else
"/:basename:output_ext"
"/:path/:basename:output_ext"
end
end

Expand All @@ -62,6 +62,7 @@ def url
permalink
else
{
"path" => @dir,
"basename" => self.basename,
"output_ext" => self.output_ext,
}.inject(template) { |result, token|
Expand Down Expand Up @@ -105,7 +106,7 @@ def render(layouts, site_payload)
# Returns the Hash representation of this Page.
def to_liquid
self.data.deep_merge({
"url" => File.join(@dir, self.url),
"url" => self.url,
"content" => self.content })
end

Expand All @@ -117,7 +118,7 @@ def to_liquid
def destination(dest)
# The url needs to be unescaped in order to preserve the correct
# filename.
path = File.join(dest, @dir, CGI.unescape(self.url))
path = File.join(dest, CGI.unescape(self.url))
path = File.join(path, "index.html") if self.url =~ /\/$/
path
end
Expand Down
5 changes: 5 additions & 0 deletions test/source/contacts/bar.html
@@ -0,0 +1,5 @@
---
title: Contact Information
---

Contact Information
5 changes: 5 additions & 0 deletions test/source/contacts/index.html
@@ -0,0 +1,5 @@
---
title: Contact Information
---

Contact Information
65 changes: 63 additions & 2 deletions test/test_page.rb
@@ -1,8 +1,10 @@
require 'helper'

class TestPage < Test::Unit::TestCase
def setup_page(file)
@page = Page.new(@site, source_dir, '', file)
def setup_page(*args)
dir, file = args
dir, file = ['', dir] if file.nil?
@page = Page.new(@site, source_dir, dir, file)
end

def do_render(page)
Expand All @@ -23,6 +25,18 @@ def do_render(page)
assert_equal "/contacts.html", @page.url
end

context "in a directory hierarchy" do
should "create url based on filename" do
@page = setup_page('/contacts', 'bar.html')
assert_equal "/contacts/bar.html", @page.url
end

should "create index url based on filename" do
@page = setup_page('/contacts', 'index.html')
assert_equal "/contacts/index.html", @page.url
end
end

should "deal properly with extensions" do
@page = setup_page('deal.with.dots.html')
assert_equal ".html", @page.ext
Expand All @@ -47,6 +61,23 @@ def do_render(page)
@page = setup_page('index.html')
assert_equal '/', @page.dir
end

context "in a directory hierarchy" do
should "create url based on filename" do
@page = setup_page('/contacts', 'index.html')
assert_equal "/contacts/index.html", @page.url
end

should "return dir correctly" do
@page = setup_page('/contacts', 'bar.html')
assert_equal '/contacts/bar/', @page.dir
end

should "return dir correctly for index page" do
@page = setup_page('/contacts', 'index.html')
assert_equal '/contacts', @page.dir
Copy link
Member

Choose a reason for hiding this comment

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

This test assertion doesn't have the trailing / while the previous one doesn't. What's up with that? Why wouldn't an index page also deserve this trailing slash?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@parkr well it's been almost a year (wow... 11 months ago) since I submitted this. 😄

OK, I dug through the code again. Nice catch! But it doesn't seem to matter since this is the directory where the file / page will be put into and not a (pretty) URL.

It appears it's not related to this commit but it just surfaced from the tests. The "issue" lies within #dir.

For example when the URL is /contacts/index.html, it doesn't end with a slash and File#basename returns /contacts. When the URL is /contacts/bar/ it ends with a URL and the dir is returned as is (same at the URL).

Honestly I'm more concerned with the "A Page processing pages with pretty url style in a directory hierarchy should create url based on filename" test which should return "/contacts/" as the pretty URL for the index file in a directory.

I will investigate this a bit more when I get home. Looks like that #dir and/or #template might have to be changed a bit.

Copy link
Member

Choose a reason for hiding this comment

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

Let me know what you find out.

In terms of the wait, I have only been a moderator for two months and I think I've done a pretty good job of dealing with crucial issues. I look at every notification from this repo. Once it's convincing enough, I'll accept it. You can expect more responsiveness from me.

end
end
end

context "with any other url style" do
Expand Down Expand Up @@ -111,6 +142,36 @@ def do_render(page)
assert File.directory?(dest_dir)
assert File.exists?(File.join(dest_dir, '.htaccess'))
end

context "in a directory hierarchy" do
should "write properly the index" do
page = setup_page('/contacts', 'index.html')
do_render(page)
page.write(dest_dir)

assert File.directory?(dest_dir)
assert File.exists?(File.join(dest_dir, 'contacts', 'index.html'))
end

should "write properly" do
page = setup_page('/contacts', 'bar.html')
do_render(page)
page.write(dest_dir)

assert File.directory?(dest_dir)
assert File.exists?(File.join(dest_dir, 'contacts', 'bar.html'))
end

should "write properly without html extension" do
page = setup_page('/contacts', 'bar.html')
page.site.permalink_style = :pretty
do_render(page)
page.write(dest_dir)

assert File.directory?(dest_dir)
assert File.exists?(File.join(dest_dir, 'contacts', 'bar', 'index.html'))
end
end
end

end
Expand Down