-
Notifications
You must be signed in to change notification settings - Fork 373
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
Add snake_case ValueForm support #8563
base: main
Are you sure you want to change the base?
Add snake_case ValueForm support #8563
Conversation
Introduce snake_case transformation in template engine. This includes both implementation and unit tests for snake_case, as well as updates to documentation and sample configurations.
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.
This is great! Thanks for the initiative.
@phenning if this is merged and backported to 9.0.2xx, would this be an affirmative reason for VS to bump the template engine dependency? And if so, we'd need to fix the NuGet package dependency regressions to unblock that, right?
``` | ||
|
||
**`snakeCase`** - Converts the value to snake case using the casing rules of the invariant culture. Available since .NET 9.0.100. |
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.
This PR is targeting main, which right now is for .NET 10 development, so this would need to be updated. Once we merge this we can backport it to 9.0.200 pretty easily with a /backport command.
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 am open to backporting it to whatever versions are available. Technically it should be able to be backported as far back as .NET 5.0.300
since I patterned all of my code after the kebabCase
just above it.
I am not sure how that is done though, and will require some assistance. This is my first contribution to this repo.
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.
At this point in time the only real options are:
- 9.0.200 for February's release
- 10.0.100 for next November's release
We don't generally add new features to an already-published release except for very special circumstances.
Once this PR is merged we'll take care of the back-porting, no worries :)
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.
9.0.200 would be fantastic but I can understand not doing it as well. I do have selfish motives for wanting to more easily snake_case
my .proto
file names in my template, but whatever timeline the dotnet team determines is fine by me. I'm just happy to contribute 😄
@@ -1051,6 +1052,14 @@ | |||
"enum": ["kebabCase"] | |||
} | |||
} | |||
}, |
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.
Thanks for adding this! Post-PR can you also update the canonical schema at schemastore?
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.
Absolutely. Is there some documentation on this process that I can read up on? I have not contributed here before so want to make sure I am doing it correctly.
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 no heavy process here - it's just that many tools (including VSCode) integrate easily with Schemastore so we also have the schema mirrored there at https://github.com/SchemaStore/schemastore/blob/master/src/schemas/json/template.json - you can submit changes to it same as you did here and tag me on the PR and I'll approve it. The one here in the repo is the 'most correct' one, but the one in Schemastore is the most tooling-friendly one.
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.
@baronfel I opened SchemaStore/schemastore#4230
If the NuGet package dependency regressions could get fixed in time for 17.14 GA, sure, we could consider taking it. I wouldn't want to push us from 8.x to 9.x in Visual Studio 2022 servicing releases after 17.14 GA unless they were for critical issues. |
@phenning thanks for the clarity - if a new form was added an a template engine host didn't support it, I expect the host would crash on template invocation? If so then we should probably keep this PR on main only for .NET 10 until and unless we get the dependency fixes completed. |
@brinehart no need to rebase or anything - we just wouldn't do the backport of this fix to 9.0.200 until/unless the dependency fixes described in this issue are resolved (the PR you linked to is unrelated). |
Introduce
snake_case
transformation in template engine. This includes both implementation and unit tests forsnake_case
, as well as updates to documentation and sample configurations.Problem
Resolves #8302
The existing system does not support snake_case transformation for value forms.
Solution
Implement snake_case transformation to support more flexible data handling.
Checks: