groue / GRMustache.swift

Flexible Mustache templates for Swift
http://mustache.github.com/
MIT License
594 stars 155 forks source link

Fix string interpolation issues with optionals #43

Closed saagarjha closed 7 years ago

groue commented 7 years ago

Hello @saagarjha

Please provide a proper bug report, and why your PR is the correct solution to this bug.

saagarjha commented 7 years ago

Sure (would you like me to file an issue?). Basically, GRMustache.swift will build with warnings (String interpolation produces a debug description for an optional value; did you mean to make this explicit?) with the latest version of Xcode because of the adoption of the Harlan Haskins's "proposal" Disallow Optionals in String Interpolation Segments, which argued that the default representation for Optional produced the surprising result of Optional("value") when interpolated. I've migrated the direct interpolation of Optionals to instead produce their description if non-nil and "nil" if not, which satisfies the compiler and provides what I feel is a reasonable default. If you feel differently, please let me know and I'll update my pull request.

groue commented 7 years ago

Basically, GRMustache.swift will build [...]

Understood. Is the proposal accepted yet?

saagarjha commented 7 years ago

It wasn't really accepted, but it was agreed upon almost unanimously and implemented without a formal review.

groue commented 7 years ago

It wasn't really accepted, but it was agreed upon almost unanimously and implemented without a formal review.

Cool. So your PR looks like the right thing to do. But it's not really. Since we're building a debugging string, I'd rather see box.value.map { String(reflecting: $0) } ?? "nil" instead of a plain box.value ?? "nil" that makes the compiler happy but doesn't care about humans.

saagarjha commented 7 years ago

Sure thing.

groue commented 7 years ago

Great! Many thanks, @saagarjha 👍