-
Notifications
You must be signed in to change notification settings - Fork 5k
Support FormattableString
as argument of BicepFunction.Interpolate
#51419
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
Support FormattableString
as argument of BicepFunction.Interpolate
#51419
Conversation
BicepFunction.Interpolate(FormattableString)
FormattableString
as argument of BicepFunction.Interpolate
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.
Pull Request Overview
This PR adds support for FormattableString
as an argument to BicepFunction.Interpolate
, addressing a user request to enable natural interpolated string syntax. The changes introduce parsing capabilities for FormattableString format strings and allow seamless conversion between FormattableString objects and Bicep interpolation expressions.
Key changes:
- Adds a new
FormattableStringHelpers
utility class for parsing FormattableString format strings - Extends
BicepStringBuilder
andBicepInterpolatedStringHandler
to handle FormattableString inputs - Adds comprehensive test coverage for the new FormattableString functionality
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
FormattableStringHelpers.cs |
New utility class that provides parsing logic for FormattableString format strings |
BicepStringBuilder.cs |
Adds FormattableString support with new overloads and conversion logic |
BicepFunction.cs |
Minor documentation formatting improvement |
BicepFunctionTests.cs |
Comprehensive test suite covering FormattableString interpolation scenarios |
Comments suppressed due to low confidence (4)
sdk/provisioning/Azure.Provisioning/src/Utilities/FormattableStringHelpers.cs:10
- [nitpick] The method name
GetFormattableStringFormatParts
contains redundant terminology. Consider renaming toGetFormatParts
since the class name already indicates it's for FormattableString helpers.
public static GetPathPartsEnumerator GetFormattableStringFormatParts(ReadOnlySpan<char> format) => new GetPathPartsEnumerator(format);
sdk/provisioning/Azure.Provisioning/src/Utilities/FormattableStringHelpers.cs:12
- The struct name
GetPathPartsEnumerator
is misleading as it processes format parts, not path parts. Consider renaming toFormatPartsEnumerator
.
public ref struct GetPathPartsEnumerator
sdk/provisioning/Azure.Provisioning/src/Utilities/FormattableStringHelpers.cs:14
- The field name
_path
is inconsistent with the actual purpose. This field holds the remaining format string, so it should be named_format
or_remaining
.
private ReadOnlySpan<char> _path;
sdk/provisioning/Azure.Provisioning/src/Utilities/FormattableStringHelpers.cs:18
- The parameter name
format
in the constructor is inconsistent with the field name_path
. Consider renaming the field to_format
to maintain consistency.
{
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
sdk/provisioning/Azure.Provisioning/src/Expressions/BicepStringBuilder.cs
Outdated
Show resolved
Hide resolved
sdk/provisioning/Azure.Provisioning/src/Utilities/FormattableStringHelpers.cs
Outdated
Show resolved
Hide resolved
sdk/provisioning/Azure.Provisioning/src/Utilities/FormattableStringHelpers.cs
Outdated
Show resolved
Hide resolved
I get some feedbacks here Azure/azure-sdk-for-net#51419 (comment) when I copy the code here to provisioning. Therefore this PR backports the same changes resolving those comments here.
Fixes #47360
Our current overloads only support the "literal interpolated strings", for instance:
this works fine now, but if we do this:
this would not compile.
This PR adds an implicit operator for the
BicepInterpolatedStringHandler
fromFormattableString
to make the second usage of above available.Why we do not make another overload that takes the
FormattableString
directly?Because if we do that, like this:
and when we call this:
this would not compile complaining about "ambiguous call from the above two overloads".
Therefore the current approach I am taking is that I added an implicit operator, therefore when you are using FormattableString as argument, the compiler would call the cast operator first, and then follow the old way to give you the interpolated result.
Contributing to the Azure SDK
Please see our CONTRIBUTING.md if you are not familiar with contributing to this repository or have questions.
For specific information about pull request etiquette and best practices, see this section.