-
Notifications
You must be signed in to change notification settings - Fork 234
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
Implementation of validate for Model Statement and Scalar. #2100
Conversation
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://cadlwebsite.z1.web.core.windows.net/prs/2100/ |
}); | ||
|
||
if (id.sv === "validate" && token() === Token.Identifier) { | ||
const vid = parseIdentifier({ |
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 we should make the id
optional. It is super annoying to have to add an id, but will be needed depending on how you want to use it. Linter can of course enforce that you have id
.
allowStringLiteral: true, | ||
}); | ||
|
||
if (id.sv === "validate" && token() === Token.Identifier) { |
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 feel like validates
reads better but I don't feel super strongly. Anyone else have feelings?
I've taken a look at the parser and related bits, looks great, you handled a lot of the fiddly bits. I think the only thing missing is an update to the grammar file and keyword colorization - see src/server/tmlanguage.ts. May also need updates to getSemanticTokens since I will check into the checker bits tomorrow! |
@marron-at-work I think this is superceded by #2141? |
Adds support for validate on Model Statements and Scalars (issue #2041):
Tests for basic functionality.
TODO: prettier formatting.