Skip to content
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

fix: Add ToString() override to TwiML class #672

Merged
merged 2 commits into from
May 23, 2023

Conversation

Swimburger
Copy link
Contributor

@Swimburger Swimburger commented Mar 10, 2023

All third-party contributors acknowledge that any contributions they provide will be made under the same open-source license that the open-source project is provided under.

Fixes

Currently, there's a ToString method with an optional SaveOptions formattingOptions parameter.
However, this is not overriding the default ToString method that all objects inherit.

As a result, in C#, when TwiML is cast to object, the ToString method will return the name of the class, for example Twilio.TwiML.MessagingResponse. This probably doesn't occur much in the wild.

However, in F#, the Object.ToString() method will always be used if you don't specify the formattingOptions argument.
Therefor, the Object.ToString() method should be overriden inside TwiML.

// before
VoiceResponse().Say("Ahoy!").ToString()                  // outputs Twilio.TwiML.VoiceResponse
VoiceResponse().Say("Ahoy!").ToString(SaveOptions.None)  // outputs TwiML correctly

// after
VoiceResponse().Say("Ahoy!").ToString()                  // outputs TwiML correctly
VoiceResponse().Say("Ahoy!").ToString(SaveOptions.None)  // outputs TwiML correctly

This PR takes care of that.

Checklist

  • I acknowledge that all my contributions will be made under the project's license
  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the Contribution Guidelines and my PR follows them
  • I have titled the PR appropriately
  • I have updated my branch with the main branch
  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary documentation about the functionality in the appropriate .md file
  • I have added inline documentation to the code I modified

@sbansla sbansla self-requested a review May 23, 2023 02:48
@sbansla sbansla merged commit ea30f5f into twilio:main May 23, 2023
@Swimburger Swimburger deleted the TwiMLToString branch May 23, 2023 03:43
AsabuHere added a commit that referenced this pull request May 23, 2023
AsabuHere added a commit that referenced this pull request May 23, 2023
…L enums via PR (#671) (#680)

* revertUse IEnumerable<T> instead of List<T> for TwiML enums (#671)"

This reverts commit dbab106.

* Revert "Add .ToString override to TwiML class (#672)"

This reverts commit ea30f5f.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants