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

Order of Prefix Declarations #59

Closed
Edkamb opened this issue Apr 26, 2024 · 4 comments · Fixed by #79
Closed

Order of Prefix Declarations #59

Edkamb opened this issue Apr 26, 2024 · 4 comments · Fixed by #79
Labels
spec:editorial Minor issue or proposed change in the specification (markup, typo, informative text) wr:response-sent Wide review management

Comments

@Edkamb
Copy link

Edkamb commented Apr 26, 2024

As far as I can tell, the normative part does not sufficiently specify how the order of prefix declaration interacts with the triples that use the prefix, in particular what if a prefix is declared after its first use. For example:

pref:infiv pref:pred pref:obj .
@prefix pref: <www.smolang.org/test#> .

The TTL parsers I tested (rapper, OWL-API) reject the above, but I see nothing in the normative part that forbids it: It is certainly defined by grammar and the part about parser state (7.1) does not specify order for the namespaces, only that "Outside of a prefixID production" a declared prefix will be replaced (not "After"). I think this should be clarified explicitly, Example 9 also does not cover this case.

@afs
Copy link
Contributor

afs commented Apr 26, 2024

I agree it isn't as clear as it might be.

The grammar rules for triples work on IRIs,not prefixed names. e.g.

[10] subject ::= iri | BlankNode | collection

so any prefixed name must have been translated for the rule to match. A prefixed name does not match the "subject" rule.

The section Parser State defines how the map is maintained.

The second and third rule arguments (PNAME_NS and IRIREF) in the prefixID production assign a namespace name (IRIREF) for the prefix (PNAME_NS).

Combined with

When used in a PrefixedName production, the iri is the value in the namespaces map corresponding to the first argument of the rule.

the prefix declaration must be before its use.

How might the text be improved?

@Edkamb
Copy link
Author

Edkamb commented Apr 29, 2024

The IRI production refers to prefixed names, so subject would match:

[135s] iri ::= IRIREF | PrefixedName
[136s] PrefixedName ::= PNAME_LN | PNAME_NS

The parser state part is, independent of the above, not clear about order, it states

Outside of a prefixID production, any PNAME_NS is substituted with the namespace.

But it should be clarified that "Outside" refers to "Last prefixID before the PNAME_NS".

@gkellogg gkellogg added spec:editorial Minor issue or proposed change in the specification (markup, typo, informative text) wr:open Wide review management labels May 13, 2024
@gkellogg
Copy link
Member

The namespaces map is in the Parser State so it seems implicit to me that the state is updated during processing. The seen by a Term Constructor such as PNAME_LN it indicates that namespaces map MUST have a corresponding namespace, which is clearly based on the current state of that map.

I think what is missing is a similar requirement in PNAME_NS where the namespaces map MUST have an entry corresponding to the first argument of the rule.

The whole point of the Parsing section is to consider how the grammar is parsed to result in an RDF Graph, so state is key to this. There are, of course, other state variables that need to be evaluated in order as well.

Other than updating PNAME_NS, if there is some informative note you think would help clarify this, please suggest some wording.

Note that the test suite has some tests that specifically look for order of prefixes, although there may not be that check for the absence of a prefix causing an error.

gkellogg added a commit that referenced this issue Oct 29, 2024
@gkellogg gkellogg added wr:response-sent Wide review management and removed wr:open Wide review management labels Oct 29, 2024
@gkellogg
Copy link
Member

See #79 for an attempt to satisfy this issue. @Edkamb please respond if this satisfies your concerns or suggest further changes in the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec:editorial Minor issue or proposed change in the specification (markup, typo, informative text) wr:response-sent Wide review management
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants