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

Use unicode escapes for ASCII control characters. #5

Closed
wants to merge 1 commit into from

Conversation

brightbyte
Copy link

N3 does not support many c-style escape sequences like \a.

Bug: https://phabricator.wikimedia.org/T145757

N3 does not support many c-style escape sequences like \a.

Bug: https://phabricator.wikimedia.org/T145757
@brightbyte
Copy link
Author

@smalyshev please have a look

"\x07" => '\u0007',
"\x08" => '\u0008',
"\x09" => '\u0009',
"\x0A" => '\u000A',
Copy link
Contributor

Choose a reason for hiding this comment

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

Why we have \x0A here if we already escaped it with addcslases?

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we shouldn't use one function here instead of two. Escaping is called tens of millions of times when generating dump.

// String escapes. Note that the N3 spec is more restrictive than the Turtle and TR
// specifications, see <https://www.w3.org/TeamSubmission/n3/#escaping>
// and <https://www.w3.org/TR/turtle/#string>
// and <https://www.w3.org/TR/n-triples/#grammar-production-literal>.
Copy link
Contributor

Choose a reason for hiding this comment

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

This says: ECHAR ::= '' [tbnrf"']

So there are more acceptable escapes than r. n and t - at least b and f also supported.

@wmfgerrit wmfgerrit closed this Aug 5, 2019
@wmfgerrit wmfgerrit deleted the unicode-escapes branch August 5, 2019 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants