-
Notifications
You must be signed in to change notification settings - Fork 591
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
Make TypedString preserve quote style #1679
Conversation
Thanks @iffyio, appreciate your suggestions. I've changed the method to |
Thank you so much for working on that 🙏 We have PRQL/prql#5099 on prql side. Very much appreciated |
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.
Werd! The PR / code 🚂 keeps on running. Thanks again @iffyio for all you do to keep this repo moving forward |
This reverts commit 56f4b82.
Fixes #1673. This is a breaking change.
TypedString contained a
String
without any knowledge of the used quote style. The parser usedparse_literal_string
to construct this, which doesn't support any quote styles other than single or double quotes. Namely, it doesn't support triple quotes from BigQuery, causing the issue reported in #1673. Additionally, it doesn't round-trip properly, always formatting its string using single quotes.I think the most proper fix is to have
TypedString
contain aValue
instead, similar to IntroducedString and others. This gives us immediate support for other quote styles and fixes the formatting to make it roundtrippable.This is a breaking change but should be an easy fix in users' codebases, just (un)wrapping the value. Migration path:
For convenience, I have added a method
into_string -> Option(String)
to Value to get the underlying string value.