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(encoder,parser): escape and unescape strings #541

Merged
merged 15 commits into from
May 2, 2023

Conversation

goto-bus-stop
Copy link
Member

@goto-bus-stop goto-bus-stop commented Apr 28, 2023

This PR improves the string value printing in apollo-encoder. It tries to sensibly decide between a multi-line block string, a single-line block string, and a non-block string depending on the contents it needs to print. It escapes special characters when writing non-block strings. When writing block strings, only """ is escaped as \""" so the block isn't ended prematurely.

The parser's String::from(StringValue) implementation also un-escapes characters.

@goto-bus-stop goto-bus-stop changed the title fix(encoder): escape strings when printing fix(encoder,parser): escape and unescape strings when converting to/from rust String May 2, 2023
@goto-bus-stop goto-bus-stop changed the title fix(encoder,parser): escape and unescape strings when converting to/from rust String fix(encoder,parser): escape and unescape strings May 2, 2023
@goto-bus-stop goto-bus-stop marked this pull request as ready for review May 2, 2023 08:30
@goto-bus-stop goto-bus-stop requested a review from lrlna as a code owner May 2, 2023 08:30
@goto-bus-stop
Copy link
Member Author

goto-bus-stop commented May 2, 2023

I'm not really happy with this but I think I should revisit later rather than spend more time on it now. I'd like to refactor string lexing (to validate escapes in more detail) and parsing and also address #319 and block string indentation at that time. For now this PR should improve the string printing in apollo-encoder so it doesn't produce invalid output anymore.

Copy link
Member

@lrlna lrlna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a comment on formatting block string values. otherwise looks good!

yea, string parsing needs a brand new coat of paint. will probably be easier to solve multi-error lexing issue too.

crates/apollo-encoder/src/field.rs Outdated Show resolved Hide resolved
@goto-bus-stop goto-bus-stop merged commit ab686d0 into main May 2, 2023
@goto-bus-stop goto-bus-stop deleted the encoder-triplequotes branch May 2, 2023 10:26
@goto-bus-stop
Copy link
Member Author

goto-bus-stop commented May 2, 2023

the parser unescaping implementation that i have here does the wrong thing for block strings btw. As block strings don't actually support escapes except for \"""", but they will be processed by this PR. I'd improve that in the future string improvements.

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