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

Change StringM Debug, String, and Debug representations to escaped #186

Merged
merged 3 commits into from
Dec 1, 2023

Conversation

leighmcculloch
Copy link
Member

What

Change StringM Debug, String, and Debug representations to escaped printable-ASCII. But keep ability to convert to/from UTF-8 Strings.

Why

The Debug, Display, and FromStr functions are intended for converting to and from a reliable String representation for the type. Today these do not render reliable String representations because StringM can actually contains arbitrary byte sequences that have no representation in ASCII or UTF-8.

These representations are also used in the XDR<>JSON encoding, and so there are values of StringM that are not roundtrip convertible.

It is however reasonable that someone would want to convert to and from a StringM type using Unicode/UTF-8 String values. The TryFrom/TryInto conversions with String have been retained for this, as well as some inherent functions on the StringM type. The inherent functions were renamed to make it clearer their intent.

Unfortunately this change does make the interface of StringM harder to understand. It's easy to forget that FromStr is for parsing strings into types, or that Display (and its auto-derived cousin ToString) are for formatting values for printing, and that both of those traits are good for converting to a reliable format. These traits are not great for conversion to and from unreliable conversions where conversion is not necessarily supported for all values.

There might be a way for us to improve this in a future release and reduce the ambiguity. An option might be to remove the conversions to and from String, and require folks to go through bytes first.

@leighmcculloch leighmcculloch marked this pull request as ready for review December 1, 2023 10:07
@leighmcculloch leighmcculloch merged commit e90b9ee into master Dec 1, 2023
3 checks passed
@leighmcculloch leighmcculloch deleted the stringm-escaped branch December 1, 2023 10:07
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.

None yet

2 participants