Fix base path for RDF parsing#496
Uh oh!
There was an error while loading. Please reload this page.
Conversation
dmitrizagidulin commented Mar 13, 2017
cc @csarven for testing -- let me know if this passes the LDN test. |
csarven commented Mar 13, 2017
The URIs in the output looks good to me. |
csarven commented Mar 13, 2017
Passes LDN tests as well! Note that it only tests JSON-LD. However, I'm noticing that if a resource with JSON-LD uses |
dmitrizagidulin commented Mar 13, 2017
@csarven - can you open an issue for that (the turtle serialization thing you mention above), with some more details (an example file so I can reproduce it, etc)? |
dmitrizagidulin commented Mar 13, 2017
Is |
csarven commented Mar 13, 2017
Created issue #497 |
csarven commented Mar 13, 2017
Yes, |
dmitrizagidulinforce-pushed the fix-serialize-base-path branch from a62958f to 6c74c7dCompare
RubenVerborgh left a comment
RubenVerborgh left a comment There was a problem hiding this comment.
Functionally correct; maybe the names can be clarified.
lib/utils.js Outdated
| */ | ||
| function uriBase (req){ | ||
| return uriAbs(req) + (req.baseUrl || '') | ||
| return uriAbs(req) + url.resolve(req.baseUrl, req.path) |
There was a problem hiding this comment.
Took me some figuring out, but it seems correct indeed.
I just wonder whether uriBase is the right name; something like fullUri seems to better convey the meaning.
There was a problem hiding this comment.
Similarly, uriAbs doesn't seem intuitive; I actually think this one should be called uriBase.
RubenVerborgh commented Aug 18, 2017
Had a go at the renaming in 8aea09e. |
dmitrizagidulin commented Aug 18, 2017
@RubenVerborgh nice! :) 👍 |
Fixes issue #476.