-
Notifications
You must be signed in to change notification settings - Fork 458
Implement verbatim string literals #265
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
Conversation
a36fe10
to
54a987e
Compare
54a987e
to
8a6b970
Compare
:D Reviewing this is #1 priority when I'm back to work on Monday |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job, just minor changes needed
} | ||
} | ||
o << "\""; | ||
} else if (ast->tokenKind == LiteralString::VERBATIM_SINGLE) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's one other case to handle these, and that's in the FixIndentation pass. It's exactly the same, except you increment the column counter rather than writing to the the stream. So you'll still have to loop over ast->value and either add 2 or 1 depending no whether it's a quote.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, I think.
throw StaticError(filename, begin, "Unterminated verbatim string"); | ||
} | ||
if (*c == quot) { | ||
if (*(c+1) == quot) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a comment explaining that the '' and "" quoting is interpreted here, unlike the non-verbatim strings where it is done later by jsonnet_string_unescape. This is OK in this case because no information is lost by resolving the repeated quote into a single quote, so we can go back to the original form in the formatter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done; pretty much used your verbiage verbatim. (ha! verbatim! ... I'll show myself out)
|
||
<ul> | ||
<li>Double-quoted, beginning with <code>"</code> and ending with the first subsequent non-quoted | ||
<code>"</code> </li> | ||
<li>Single-quoted, beginning with <code>'</code> and ending with the first subsequent non-quoted | ||
<code>'</code> </li> | ||
<li>Double-quoted verbatim, beginning with <code>@"</code> and ending with the first subsequent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should probably say "non-quoted" again here, although the quoting scheme is different, it's still a quoting scheme.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
I haven't forgotten about this - just haven't had time to return to it. I'll try to finish it up soon. |
No problem, thanks for the update |
Awesome... I already have a mental workitem to update the tutorial with some of the new features so I'll add this at the same time. |
Fixes #246