Correct behaviour of compute_qname for URNs #1274
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Mar 8, 2021
|
Just a note: removing the colon from ALLOWED_NAME_CHARS doesn't break any existing tests, and passes the 2 new tests so it should be an easy fix:) The colon was added originally to modify the behavior of is_cname, and split_uri but was subsequently filtered out in the is_cname function (#663 (comment)) as it caused issues there too. Seems like it causes issues for split_uri as well, it just wasn't noticed due to no testing for URNs. |
…and relatedly for correct turtle serialization of URNs
… identifiers not getting split at all and thus leading to compute_qname being unable to compute the qnames of URNs. This change did not break any current tests, and allowed the previously added 2 tests to pass (note: as a quick fix is_cname already excludes colon from ALLOWED_NAME_CHARS as it causes issues there too #663 , so as far as I can tell there really is no reason for colon to be included)
nicholascar
approved these changes
Mar 9, 2021
white-gecko
approved these changes
Mar 10, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: Allow URN qnames to be computed correcly by compute_qname
Removed colon from ALLOWED_NAME_CHARS, as having it included results in URN identifiers never being split by split_uri and thus no qname being computed by compute_qname for URNs.
I've also added some tests for correct behaviour of compute_qname for URNs and turtle serialization (where this issue surfaced initally, see the end of #977)