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.

Sign up for GitHub

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

Jump to bottom

feat/rdfstar support #311

Closed
wants to merge 29 commits into from
Closed

feat/rdfstar support #311

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
2f7d6e8
chore: add nested list test
jeswr Nov 23, 2022
37ab09c
fix: support quoted triples in list
jeswr Nov 23, 2022
f837b8d
breaking: drop support for quads in quoted triples as they are forbid…
jeswr Nov 23, 2022
da945e9
feat: support annotated triples
jeswr Nov 23, 2022
624e3e1
chore: error on quoted compound bnodes
jeswr Nov 23, 2022
d27b920
feat: turtle-star spec tests are passing
jeswr Nov 23, 2022
2af5e05
chore: fix lint and coverage errors
jeswr Nov 23, 2022
0ac4c46
chore: remove commented code
jeswr Nov 23, 2022
0337ed8
chore: rename RDF* -> RDF-star
jeswr Nov 23, 2022
99d6b72
chore: update RDF-star reference in readme
jeswr Nov 24, 2022
d258f22
chore: fix round trip on deeply nested rdfstar triples
jeswr Nov 24, 2022
56cc184
chore: add tests from https://github.com/rdfjs/N3.js/pull/303
jeswr Nov 26, 2022
2f0f57d
chore: describe quoted triple predicate parsing
jeswr Jan 4, 2023
0539be9
chore: clarify use of graph term in quoted quads
jeswr Jan 4, 2023
e7646d9
fix: allow a split between '|' and '}' (see https://github.com/rdfjs/…
jeswr Jan 4, 2023
6140e86
chore: remove doubling comment
jeswr Jan 4, 2023
bec8395
chore: add comment about nested parameter
jeswr Jan 4, 2023
8ce8428
fix: use describe for all shouldParse test suites
jeswr Jan 4, 2023
211bf07
fix: dont interpret }| as {|
jeswr Jan 4, 2023
4489772
Update test/N3Parser-test.js
jeswr Jan 5, 2023
c76f3fd
Update src/N3Parser.js
jeswr Jan 5, 2023
8a6dcbb
Update src/N3Parser.js
jeswr Jan 5, 2023
7df44bc
chore: document writing rdf-star
jeswr Feb 25, 2023
545e646
BREAKING CHANGE: enable rdfStar support by default
jeswr Feb 25, 2023
90e15b9
chore: update docs to not rdfStar default
jeswr Feb 25, 2023
6907296
chore: refactor lexer
jeswr Feb 25, 2023
73791d8
fix: re-enable line mode check
jeswr Feb 26, 2023
8e3b997
chore: refactor lexer
jeswr Feb 26, 2023
2e655fb
fix: make rdf-star work with n3 paths
jeswr Feb 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/N3DataFactory.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,10 @@ export class DefaultGraph extends Term {
DEFAULTGRAPH = new DefaultGraph();

// ### Constructs a term from the given internal string ID
// The third 'nested' parameter of this function is to aid
// with recursion over nested terms. It should not be used
// by consumers of this library.
// See https://github.com/rdfjs/N3.js/pull/311#discussion_r1061042725
export function termFromId(id, factory, nested) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jeswr In what cases should nested be true? Is it the same as rdfStar basically?

Copy link
Collaborator Author

@jeswr jeswr Jan 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is really just an internal parameter that is used when recursively calling termFromId on quad terms within other quads.

The only place that parameter should ever be used is

N3.js/src/N3DataFactory.js

Lines 224 to 229 in 56cc184

return factory.quad(
termFromId(id[0], factory, true),
termFromId(id[1], factory, true),
termFromId(id[2], factory, true),
id[3] && termFromId(id[3], factory, true)
);

So no this is not an rdfStar parameter because this should still be false/undefined on the asserted triple.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a comment clarifying this

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So only by itself; gotcha.

factory = factory || DataFactory;

Expand Down Expand Up @@ -230,6 +234,10 @@ export function termFromId(id, factory, nested) {
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Questions for the future:

  • Should N3.js Term internally still rely on id?
  • (For what cases) do we still need termFromId, or is termToId enough—and can it just be .hash on the thing itself then?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(For what cases) do we still need termFromId, or is termToId enough—and can it just be .hash on the thing itself then?

Any use of the Store will still require termFromId this since terms can't be recovered from a hash. Using a hash is really only an optimisation for term equality (in which case a hash for the id is fine).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh actually - I think I see more what you're saying is that quads could just use a set of ids/hashes in the internal representation that are then looked up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any use of the Store will still require termFromId this since terms can't be recovered from a hash.

But that could be an internal function then indeed!


// ### Constructs an internal string ID from the given term or ID string
// The third 'nested' parameter of this function is to aid
// with recursion over nested terms. It should not be used
// by consumers of this library.
// See https://github.com/rdfjs/N3.js/pull/311#discussion_r1061042725
export function termToId(term, nested) {
if (typeof term === 'string')
return term;
Expand All @@ -248,8 +256,6 @@ export function termToId(term, nested) {
term.language ? `@${term.language}` :
(term.datatype && term.datatype.value !== xsd.string ? `^^${term.datatype.value}` : '')}`;
case 'Quad':
// To identify RDF-star quad components, we escape quotes by doubling them.
// This avoids the overhead of backslash parsing of Turtle-like syntaxes.
const res = [
termToId(term.subject, true),
termToId(term.predicate, true),
Expand Down
17 changes: 11 additions & 6 deletions src/N3Lexer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,19 +294,24 @@ export default class N3Lexer {
case '!':
if (!this._n3Mode)
break;
case '{':
// Note the input[0] === '{' is required as this could be a fall-through from the above case
if (input.length > 1 && input[0] === '{' && input[1] === '|') {
jeswr marked this conversation as resolved.
Show resolved Hide resolved
type = '{|', matchLength = 2;
break;
}
case ',':
case ';':
case '[':
case ']':
case '(':
case ')':
case '{':
case '}':
if (input.length > 1 && input[1] === '|') {
type = '{|', matchLength = 2;
break;
}
if (!this._lineMode) {
if (
!this._lineMode &&
// The token might actually be {| and we just have not encountered the pipe yet
(input !== '{' || input.length > 1)
) {
jeswr marked this conversation as resolved.
Show resolved Hide resolved
matchLength = 1;
type = firstChar;
}
Expand Down
7 changes: 5 additions & 2 deletions src/N3Parser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,8 @@ export default class N3Parser {

this._saveContext('{|', this._graph, this._subject, this._predicate, this._object);

// Note - we always use the default graph for the quoted triple component
// As a convention we use set the graph term as the Default Graph in quads representing quoted triples
jeswr marked this conversation as resolved.
Show resolved Hide resolved
// see https://github.com/rdfjs/N3.js/pull/311#discussion_r1061039556 for details
this._subject = this._quad(this._subject, this._predicate, this._object, this.DEFAULTGRAPH);
this._predicate = null;
this._object = null;
Expand Down Expand Up @@ -899,7 +900,8 @@ export default class N3Parser {
if (token.type === '{|') {
this._saveContext('{|', this._graph, this._subject, this._predicate, this._object);

// Note - we always use the default graph for the quoted triple component
// As a convention we use set the graph term as the Default Graph in quads representing quoted triples
jeswr marked this conversation as resolved.
Show resolved Hide resolved
// see https://github.com/rdfjs/N3.js/pull/311#discussion_r1061039556 for details
this._subject = this._quad(this._subject, this._predicate, this._object, this.DEFAULTGRAPH);
this._predicate = null;
this._object = null;
Expand All @@ -909,6 +911,7 @@ export default class N3Parser {
this._emit(this._subject, this._predicate, this._object, this._graph);
}

// If the quoted triple is not finished, the next token must be a predicate
if (token.type !== '|}')
jeswr marked this conversation as resolved.
Show resolved Hide resolved
return this._readPredicate;
this._restoreContext('{|', token);
Expand Down
Loading