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

FLIP 288: Simple String Interpolation #289

Merged
merged 4 commits into from
Oct 30, 2024

Conversation

RZhang05
Copy link
Contributor

Flip #288 addresses onflow/cadence#3579.

Copy link
Member

@SupunS SupunS left a comment

Choose a reason for hiding this comment

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

Nice! This is a really convenient feature to have.

cadence/20240923-simple-string-interpolation.md Outdated Show resolved Hide resolved
Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

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

Nice! This is going to be really useful

cadence/20240923-simple-string-interpolation.md Outdated Show resolved Hide resolved
cadence/20240923-simple-string-interpolation.md Outdated Show resolved Hide resolved
cadence/20240923-simple-string-interpolation.md Outdated Show resolved Hide resolved
cadence/20240923-simple-string-interpolation.md Outdated Show resolved Hide resolved
cadence/20240923-simple-string-interpolation.md Outdated Show resolved Hide resolved
@bjartek
Copy link
Contributor

bjartek commented Sep 25, 2024

I really like the concept.

@bluesign
Copy link
Collaborator

maybe little off topic but quick note on string interpolated strings vs printf; they are used for different needs; it is not one or the other. ( interpolation is better DX, but limited comparing to printf ( you cannot have dynamic templates etc ) ) So having both is a bit benefit. I think thinking with printf and building over it can be better option.

I am more into prefix or backtick to indicate interpolation personally, I think it is also easier to analyze statically. ( also by using a new prefix or backtick we can have multiline strings as additional benefit)

Something like this:

var s = `hello, ${getUsername(userId)};
you have ${getUserBalance(userId)} tokens.
`

@turbolent
Copy link
Member

maybe little off topic but quick note on string interpolated strings vs printf; they are used for different needs; it is not one or the other. ( interpolation is better DX, but limited comparing to printf ( you cannot have dynamic templates etc ) )

Regarding the limitation of not having dynamic templates: when do you ever need to dynamically construct the template? From my experience, that is almost never the case.

We might still want to also provide a string formatting facility, but we should promote the safer alternative (string interpolation). Given the benefits of string interpolation, static checking, we should prioritize it over string formatting.

@bluesign
Copy link
Collaborator

bluesign commented Sep 25, 2024

Regarding the limitation of not having dynamic templates: when do you ever need to dynamically construct the template? From my experience, that is almost never the case.

Sorry it is a bit lack of my English; when I said dynamic templates, I meant format string stored somewhere and can be changed ( vs storing in code ) ( something like https://github.com/onflow/flow-evm-bridge/blob/main/cadence/contracts/templates/mainnet/EVMBridgedNFTTemplate.cdc )

Given the benefits of string interpolation, static checking, we should prioritize it over string formatting.

100% agree here

@sisyphusSmiling
Copy link
Contributor

This would be a great feature!

Though I do agree with @bluesign that being able to use this in the context of stored strings would also be useful. As another example, the recent FlowRewards contracts (FlowRewardsMetadataViews#L106) uses an SVG template stored within a struct, rendered by joining values with the contained string chunks.

It would be much easier to simply store the whole template (rather than splitting before storing) and render using native interpolation.

e.g. In practice, we currently need to so something like

access(all)
struct Template {
  access(all) let chunks: [String]
  init(_ template: [String]) { self.chunks = template }
  
  access(all)
  fun render(param1: Int, param2: Int): String {
    return self.chunks[0].concat(param1.toString())
      .concat(self.chunks[2]).concat(param2.toString()
  }
}

Where it would be much nicer to do something like

access(all)
struct Template {
  access(all) let template: String
  init(_ template: String) { self.template = template }
  
  access(all)
  fun render(param1: Int, param2: Int): String {
    // where template == "First parameter: \(param1) | Second parameter: \(param2)"
    return printf(`self.template`, param1, param2) // or some variation
  }
}

Though perhaps the above is a separate issue entirely or is at least not mutually exclusive with this FLIP as proposed.

Comment on lines +43 to +45
### Drawbacks

None.

Choose a reason for hiding this comment

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

Nit: One little drawback I see is that \( is a bit of an uncommon escape sequence for string interpolation. I haven't seen a language that uses that. I don't think this really matters, but I do feel like it's a bit easier to guess the correct syntax or scan the code if using a common syntax (which I'm not sure is possible given backward compatibility constraints as mentioned).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Somewhat agree, it is not the most common syntax. However it is used by Swift and as you mentioned syntax such as "${}" does run into backward compatibility issues.

@SupunS
Copy link
Member

SupunS commented Oct 18, 2024

That's a good point on dynamic string formatting @bluesign and @sisyphusSmiling! Totally agree that it could be useful to have that as well. But at the same time, I think, as you pointed out as well, it could be another separate feature (can propose it as a new FLIP), and shouldn't really block us from having the static string templates/interpolations.

Copy link
Member

@joshuahannan joshuahannan left a comment

Choose a reason for hiding this comment

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

This looks great! I'm excited to use this instead of concat!

@turbolent turbolent changed the title Flip 288: Simple String Interpolation FLIP 288: Simple String Interpolation Oct 23, 2024
@RZhang05
Copy link
Contributor Author

As discussed in the last working group session, this FLIP has been accepted 🎉

@SupunS SupunS merged commit 432578b into onflow:main Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Proposed
Development

Successfully merging this pull request may close these issues.

9 participants