-
Notifications
You must be signed in to change notification settings - Fork 202
Bug/support nquads #148
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
Bug/support nquads #148
Conversation
@@ -788,7 +788,7 @@ jsonld.normalize = function(input, options, callback) { | |||
} | |||
|
|||
if('inputFormat' in options) { | |||
if(options.inputFormat !== 'application/nquads') { | |||
if((options.inputFormat !== 'application/nquads') || (options.inputFormat !== 'application/n-quads')) { |
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.
Shouldn't this be an &&
? Otherwise it would seem that this compound conditional would always be true.
That being said, it would probably be cleaner to make a shallow copy of the options and if the inputFormat
is application/nquads
it should just be changed early to the correct application/n-quads
. Then we only need to check for that in the code later. So let's normalize to application/n-quads
.
@dlongley So, I have made the necessary changes. Yes, indeed you were right we should just convert |
And why these travis tests are failing? |
var opts = _clone(options); | ||
|
||
if('inputFormat' in opts) { | ||
if(opts.inputFormat.indexOf('application/nquads') != -1) { |
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.
Comparison should do:
opts.inputFormat === 'application/nquads'
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.
Oh God! What was I thinking 😡 👊. my bad
Due to significant code changes, I redid this patch. Perhaps not addressing the "change the format early" issue though. |
Fixed in 0.5.18: |
Hey,
As pointed out in #141, This PR intends to support both
application/nquads
andapplication/n-quads
in jsonld.js.There are a few things, that I wanted to know
application/nquads
?options.useStandardNQuadsType
and if the user passestrue
then onlyapplication/n-quads
formatting is done. Should I make this behaviour by default? And also should I change the variable name to something more generic?