-
Notifications
You must be signed in to change notification settings - Fork 226
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
Feature: String templates #2630
Feature: String templates #2630
Conversation
…ture/string-template
Changes in this PR will be published to the following url to try(check status of TypeSpec Pull Request Try It pipeline for publish status): Website: https://tspwebsitepr.z22.web.core.windows.net/prs/2630/ |
@@ -102,6 +102,14 @@ import { | |||
StdTypes, | |||
StringLiteral, | |||
StringLiteralNode, | |||
StringTemplate, |
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.
Do the names look good? Alternatives could have been
TemplateString
TemplateStringLiteral
TemplateLiteral
StringTemplateLiteral
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.
StringTemplateLiteral
is probably more accurate
EDIT: Ahh, I didn't see the rest of the names. Probably don't want all of them to be StringTemplateLiteralX
so StringTemplate
is fine IMO
@@ -102,6 +107,14 @@ import { | |||
StdTypes, | |||
StringLiteral, | |||
StringLiteralNode, | |||
StringTemplate, | |||
StringTemplateExpressionNode, | |||
StringTemplateHeadNode, |
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.
Do you split this into head/middle/end so that you only capture the part that needs to be computed as the middle?
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.
Those are technically slightly different nodes and that's how typescript also structure their template so thought be good to keep inline as they must have had a reason to do that. But right now the data inside is basically the same
common/changes/@typespec/compiler/feature-string-template_2023-11-14-00-00.json
Outdated
Show resolved
Hide resolved
packages/spec/src/spec.emu.html
Outdated
@@ -154,14 +154,28 @@ <h1>Lexical Grammar</h1> | |||
StringLiteral : | |||
`"` StringCharacters? `"` | |||
`"""` TripleQuotedStringCharacters? `"""` | |||
StringTemplateHead | |||
|
|||
StringTemplateHead :: |
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.
I think this grammar is incomplete because we cannot reach StringTemplateMiddle/StringTemplateTail from the top-level goal symbol InputElement. We will need a separate goal symbol (say, InputElementTemplateTail
). I also think it should be a parse error to have an empty substitution, which would require separating out right brace from the list of tokens and only allowing it with the InputElementTemplateTail
.
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.
updated, let me know if that looks better
…-11-14-00-00.json Co-authored-by: Brian Terlson <[email protected]>
…guerin/typespec into feature/string-template
fix #1591
String templates
Playground examples
Other fixes
Also fixes https://github.com/Azure/typespec-azure/issues/3399(Show invalid escape sequence char instead of the whole string)