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

URIs don't begin with < or end with > #124

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jpmccu
Copy link

@jpmccu jpmccu commented Jul 6, 2012

Especially ones embedded in RDFa. You're probably thinking of N3-based representations, but the <>s aren't actually part of the URI. I may have missed some edge cases where you were relying on the <>s for something, but at least now your URIs aren't broken, and you can accept real ones.

@travisbot
Copy link

This pull request fails (merged 7502d46 into 0289577).

@bergie
Copy link
Owner

bergie commented Jul 6, 2012

Thanks for the contribution! URIs were wrapped in < and > in older JSON-LD specifications, which is why we still do it.

There has been some discussion (and a ticket, #110) on this, and the most important consideration is how to do this in a way that it doesn't break existing implementations. For this we need especially isReference to be rock-solid. And probably we need a version of toJSONLD that is able to return the old format.

@jpmccu
Copy link
Author

jpmccu commented Jul 6, 2012

That's reasonable. I think that the library should focus on keeping the <>s in JSON-LD without pulling them into the library itself. I'll look again at toJSONLD and isReference and the test failures. I was trying to read in some RDFa from an HTML page which failed originally because I didn't include <>s around my URIs, as the RDFa specification says. I couldn't find a way to execute the unit tests from my local git repo. Is there documentation on that?

Thanks,
Jim

@bergie
Copy link
Owner

bergie commented Jul 6, 2012

@jimmccusker you need to build your local VIE version by running ant.

Then just open test/index.html file in a browser.

Optionally you can also do:

$ npm install
$ npm test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants