apollographql / apollo-rs

Spec compliant GraphQL Tools in Rust.
Apache License 2.0
566 stars 45 forks source link

Serialize multi-line strings as block strings #724

Closed SimonSapin closed 9 months ago

SimonSapin commented 10 months ago

Fixes https://github.com/apollographql/apollo-rs/issues/664

SimonSapin commented 10 months ago

Should we also prefer block strings for single-line descriptions? (As opposed to string Values)

SimonSapin commented 10 months ago

It looks like graphql-js:

@IvanGoncharov did I miss something?

goto-bus-stop commented 10 months ago

Did the new pushes change something or is it just a rebase?

SimonSapin commented 10 months ago

Rebase to make CI pass + removing the obsolete comment

IvanGoncharov commented 10 months ago

StringValue objects can contain a isBLockString boolean flag… but nothing ever sets it?

@SimonSapin The flag called, block is isBLockString just an alias in this particular destruction. It is set in parser: https://github.com/graphql/graphql-js/blob/0b7590f0a2b65e6210da2e49be0d8e6c27781af2/src/language/parser.ts#L713 This allows graphql-js to do round-trip where block string is parsed and when printed as block string.

Always prefers block strings for descriptions

Yes, note that not every string can be printed as block string.

Block string printing has some heuristics to decide whether to add leading/trailing newlines

Yes, in graphql-js we had a bug where by printing string as block string we were loosing some characters so I spend bunch of time figuring out different heuristics that preserved all characters.

SimonSapin commented 10 months ago

New commit to make stylistic choices closer to graphql-js: