No spaces #171
No spaces #171
Conversation
| @@ -516,7 +516,8 @@ var Fetcher = function Fetcher (store, timeout, async) { | |||
| } else { | |||
| // See https://www.iana.org/assignments/link-relations/link-relations.xml | |||
| // Alas not yet in RDF yet for each predicate | |||
| predicate = kb.sym(Uri.join(rel, 'http://www.iana.org/assignments/link-relations/')) | |||
| /// encode space in e.g. rel="shortcut icon" | |||
| predicate = kb.sym(Uri.join(encodeURIComponent(rel), 'http://www.iana.org/assignments/link-relations/')) | |||
There was a problem hiding this comment.
I took a look at http://www.iana.org/assignments/link-relations/ and didn't see any relations with spaces in them. Why is this necessary?
There was a problem hiding this comment.
a) when theer could be and you don't want your code to crash and
b) See the comment ... e.g. rel="shortcut icon" was found in real life
There was a problem hiding this comment.
I'm for a), but isn't b) misusing the iana link relations vocabulary? Furthermore http://www.iana.org/assignments/link-relations/shortcut%20icon dereferences to a 404.
There was a problem hiding this comment.
I agree that NamedNodes should url encode spaces in their values before serialization. But code that constructs named nodes should be providing URIs which resolve to real data.
Separately, I think we should have NamedNode.toString do the url encoding rather than have callers pass in already-encoded urls.
There was a problem hiding this comment.
- The way things evolve in things like HTML tags and link relations and HTTP headers is always a mixture of standards first and code first.
- yes, and IANA should by linked data principles should provide RDF at the end of the link. This is a longstanding issue over years. (That's why the classes derived from mime types I code to be in W3c space.
A to-do is to populate that space in fact with triples the triples to go int it, they need t e put in files ) - Another thing we cna do is make a mirror of what we sould tlike they cto have, and have a list of things whih are not maintained well with pointers to where pothers mainatin a better copy -- just for core things like tis
There was a problem hiding this comment.
Thanks for the background 😄
What do you think about having the url encoding done during NamedNode serialization rather than during the construction of NamedNodes?
There was a problem hiding this comment.
Because any string which is not a URL is neither here nor there, not a well-defined thing. You can only compare URLs. Things which random code in places converts into URLs, like fileames, people names, titles of documents, HRRP headers, link relations, whatever.... there is no standard for them, they only have well-defined properties in their own defined spaces like file system etc etc.
When they all come together as URIs they are part of a single universal namespace.
src/serializer.js
Outdated
| @@ -34,20 +34,20 @@ var Serializer = function () { | |||
| /* pass */ | |||
| } | |||
|
|
|||
| __Serializer.prototype.setBase = function (base) { this.base = base ; return this} | |||
| __Serializer.prototype.setBase = function (base) { this.base = base; return this } | |||
There was a problem hiding this comment.
Was this code generated by a transpiler?
There was a problem hiding this comment.
I was trying to remember whether it was python or coffee - -maybe this was when coffee was the thing... .. as I don't see any pyjslib calls.
src/serializer.js
Outdated
| p = 'n' // Otherwise the loop below may never termimnate | ||
| } | ||
| for (var i = 0;; i++) if (canUse(p.slice(0, 3) + i)) return pok | ||
| for (var j = 0; ; j++) if (canUse(p.slice(0, 3) + j)) return pok |
There was a problem hiding this comment.
Good point. Yes, I re-coded it, as (canUse(p.slice(0, 3) + i)) return p.slice(0, 3) + i) etc
src/serializer.js
Outdated
| @@ -197,8 +197,8 @@ var Serializer = function () { | |||
| // @param force, "we know this is a root, do it anyway. It isn't a loop." | |||
| function dummyObjectTree(obj, subjects, rootsHash, force) { | |||
| // dump('dummyObjectTree('+obj+'...)\n') | |||
| if (obj.termType == 'BlankNode' && (subjects[sz.toStr(obj)] && | |||
| (force || (rootsHash[obj.toNT()] == undefined )))) {// and there are statements | |||
| if (obj.termType === 'BlankNode' && (subjects[sz.toStr(obj)] && | |||
There was a problem hiding this comment.
Is this just dead code? If so, it should be deleted. If not do we have a ticket to do something with it?
|
👍 Nice work on the serializer! |
…ndard processing). Add a test t10 with more than a few triples in one file! many more tests needed
|
👍 Thanks @timbl for reminding me that it shouldn't be possible to create a |
URIs cannot contain spaces (unless they are encoded as %20)
This has always been the case, but recently we have made it a run-time error to try to generate a named node with them, so various bugs have been surfaced. This PR
fixes two places where spaces were ending in URIs, eg from link headers
upgrade the code in serializer.js to pass standard #technicalcredit