-
Notifications
You must be signed in to change notification settings - Fork 10
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
Make uri mapping neat #99
Conversation
4e31ca4
to
6c20443
Compare
lib/jekyll/drops/rdf_resource.rb
Outdated
# opaque jekyll produces static pages, so we do not dereferencing | ||
# query queries do not address resources | ||
# file_name << "#/#{uri[:fragment]}" unless uri[:fragment].nil? fragments are not evaluated by browsers, only by clients | ||
rescue Addressable::URI::InvalidURIError => x #unclean coding: blanknodes are recognized through errors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this comment, if this is not true anymore
lib/jekyll/drops/rdf_resource.rb
Outdated
file_name += '/' | ||
end | ||
rescue URI::InvalidURIError => x #unclean coding: blanknodes are recognized through errors | ||
#key_field = [:scheme, :userinfo, :host, :port, :registry, :path, :opaque, :query, :fragment] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make a more verbose note, that these are the parameters of uri
6c20443
to
bc32cc8
Compare
bc32cc8
to
d7c16bd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also add dedicated tests for the new behavior please use the example given in https://github.com/white-gecko/jekyll-rdf/issues/94 for this.
d7c16bd
to
14652dd
Compare
14652dd
to
088b02d
Compare
lib/jekyll/drops/rdf_resource.rb
Outdated
file_name = "invalids/#{term.to_s}" | ||
Jekyll.logger.error("Invalid resource found: #{term.to_s} is not a proper uri") | ||
Jekyll.logger.error("URI parser exited with message: #{x.message}") | ||
end | ||
end | ||
# windows compatibility | ||
file_name = file_name.gsub('_','_u') | ||
file_name = file_name.gsub('//','/') # needs a better regex to include /// ////... | ||
file_name = file_name.gsub(':','_D') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should have been removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also check if the // -> / replacement is still needed.
test/test_rdf_page_data.rb
Outdated
assert_equal "/b/x", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/b/x"), sparql, @site).page_url | ||
assert_equal "/b/y/index.html", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/b/y/"), sparql, @site).render_path | ||
assert_equal "/b/y/", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/b/y/"), sparql, @site).page_url | ||
assert_equal "/a.html", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/a.html"), sparql, @site).render_path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line should be duplicated for page_url = "/a.html"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because the .html is already contained in the URI
test/test_rdf_page_data.rb
Outdated
|
||
should "let fragment-identifier default to super resource" do | ||
assert_equal "/c.html", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/c#alpha"), sparql, @site).render_path | ||
assert_equal "/c.html", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/c#beta"), sparql, @site).render_path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the page_url here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the page_url should be "/c#alpha" resp. "/c#beta"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
considering #94 we should only have another value for page_url if our subresource does not have a super resource.
The problem is that, checking if a superresource exist might be to complex for what we want to achieve
test/test_rdf_page_data.rb
Outdated
|
||
context "Jekyll::Drops::RdfResource.render_path"do | ||
setup do | ||
@config = Jekyll.configuration({'url' => "http://ex.org", 'baseurl' => "" }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please duplicate this test for baseurl => "/", where the page_url for "http://ex.org/a" would be "a" and the render path would be "a.html"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe also do the same for baseurl => "/blog" with http://ex.org/blog/a …
for http://ex.org/a in this case the page_url would be "/rdfsites/http/ex.org/a" (or "/rdfsites/http/ex/org/a") and the render_path: "/rdfsites/http/ex.org/a.html"
088b02d
to
368d73a
Compare
5969ee9
to
8a9771e
Compare
8a9771e
to
d260859
Compare
test/test_rdf_page_data.rb
Outdated
assert_equal "/b/y/", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/b/y/"), @site).page_url | ||
assert_equal "/a.html", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/a.html"), @site).render_path | ||
assert_equal "/a.html", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/a.html"), @site).page_url |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tab
test/test_rdf_page_data.rb
Outdated
should "let fragment-identifier default to super resource" do | ||
assert_equal "/c.html", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/c#alpha"), @site).render_path | ||
assert_equal "/c", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/c#alpha"), @site).page_url |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tab
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The page_url
here should be /c#alpha
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also have a look at: https://github.com/white-gecko/jekyll-rdf/issues/82#issuecomment-338967632
test/test_rdf_page_data.rb
Outdated
assert_equal "/rdfsites/http/ex.org/a", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/a"), @site).page_url | ||
assert_equal "/bla/blog/a.html", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/bla/blog/a"), @site).render_path | ||
assert_equal "/a.html", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/a"), @site).render_path | ||
assert_equal "/a", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/a"), @site).page_url |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If 'url' => "http://ex.org", 'baseurl' => "/blog"
is configured the correct renderpath for "http://ex.org/bla/blog/a"
should be "/rdfsites/http/ex.org/bla/blog/a.html"
not "/bla/blog/a.html"
. My test were already testing the right things as we have discussed.
test/test_rdf_page_data.rb
Outdated
assert_equal "rdfsites/http/ex.org/a", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/a"), @site).page_url | ||
assert_equal "/bla/blog/a.html", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/bla/blog/a"), @site).render_path | ||
assert_equal "/a.html", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/a"), @site).render_path | ||
assert_equal "/a", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/a"), @site).page_url |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
test/test_rdf_page_data.rb
Outdated
assert_equal "/bla/blog/a.html", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/bla/blog/a"), @site).render_path | ||
assert_equal "/a.html", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/a"), @site).render_path | ||
assert_equal "/a", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/a"), @site).page_url |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The last three lines are not correct.
If 'url' => "http://ex.org", 'baseurl' => "/blog" is configured the correct renderpath for "http://ex.org/bla/blog/a" should be "/rdfsites/http/ex.org/bla/blog/a.html" not "/bla/blog/a.html". My test were already testing the right things as we have discussed before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can also have a look at white-gecko@a700943#r146799188
test/test_rdf_page_data.rb
Outdated
assert_equal "/bla/blog/a.html", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/bla/blog/a"), @site).render_path | ||
assert_equal "/a.html", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/a"), @site).render_path | ||
assert_equal "/a", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/a"), @site).page_url |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same problem as above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad, thought we said baseurl
and url
have to be considered separately.
a700943
to
9b68f2f
Compare
test/test_rdf_page_data.rb
Outdated
assert_equal "rdfsites/http/ex.org/bla/blog/a.html", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/bla/blog/a"), @site).render_path | ||
assert_equal "rdfsites/http/ex.org/a.html", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/a"), @site).render_path | ||
assert_equal "rdfsites/http/ex.org/a", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/a"), @site).page_url |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rdfsites never has a leading /
because it describes the same position independent of url
and baseurl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, there has to be a leading /
as well.
9b68f2f
to
c72b3e2
Compare
@classResources["http://xmlns.com/foaf/0.1/Person"].subClasses.any?{|class_res| class_res.to_s.eql? "http://pcai042.informatik.uni-leipzig.de/~dtp16/#AnotherSpecialPerson"}), "http://xmlns.com/foaf/0.1/Person should have http://pcai042.informatik.uni-leipzig.de/~dtp16/#SpecialPerson and http://pcai042.informatik.uni-leipzig.de/~dtp16/#AnotherSpecialPerson as subclass" | ||
assert @classResources["http://pcai042.informatik.uni-leipzig.de/~dtp16/#AnotherSpecialPerson"].subClasses.any?{|class_res| class_res.to_s.eql? "http://pcai042.informatik.uni-leipzig.de/~dtp16/#SpecialPerson"},"http://pcai042.informatik.uni-leipzig.de/~dtp16/#AnotherSpecialPerson should have http://pcai042.informatik.uni-leipzig.de/~dtp16/#SpecialPerson as subclass" | ||
assert (@classResources["http://xmlns.com/foaf/0.1/Person"].subClasses.any?{|class_res| class_res.to_s.eql? "http://pcai042.informatik.uni-leipzig.de/~dtp16#SpecialPerson"}&& | ||
@classResources["http://xmlns.com/foaf/0.1/Person"].subClasses.any?{|class_res| class_res.to_s.eql? "http://pcai042.informatik.uni-leipzig.de/~dtp16#AnotherSpecialPerson"}), "http://xmlns.com/foaf/0.1/Person should have http://pcai042.informatik.uni-leipzig.de/~dtp16#SpecialPerson and http://pcai042.informatik.uni-leipzig.de/~dtp16#AnotherSpecialPerson as subclass" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there an assert
missing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok the previous line ends with &&
test/test_rdf_page_data.rb
Outdated
assert_equal "rdfsites/http/ex.org/bla/blog/a.html", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/bla/blog/a"), @site).render_path | ||
assert_equal "rdfsites/http/ex.org/a.html", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/a"), @site).render_path | ||
assert_equal "rdfsites/http/ex.org/a", Jekyll::Drops::RdfResource.new(RDF::URI("http://ex.org/a"), @site).page_url |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, there has to be a leading /
as well.
c72b3e2
to
eb25a72
Compare
- Fix testcases for fragemnt identifier - Fix testcases for baseurl ending with slash - Add testcases for different baseurls (ending with slash and not) - Replace tabs by spaces
eb25a72
to
20c0327
Compare
Fix #94, #78, #82
it does not throw the error described at the end of #94