Skip to content

String concat fix - #508

Merged
tomblind merged 3 commits into
masterfrom
string-concat-fix
Apr 5, 2019
Merged

String concat fix#508
tomblind merged 3 commits into
masterfrom
string-concat-fix

Conversation

@tomblind

@tomblind tomblind commented Apr 4, 2019

Copy link
Copy Markdown
Collaborator

fixes #504

  • wraps values that aren't guaranteed to be safe in a concat op with tostring
    • for now, safe values are string/number literals and other concat ops
  • decision to wrap is also applied to template strings, which cleans them up a bit
  • tests now include concatonating boolean values, which don't automatically coerce into strings
  • a bit unrelated, but I also added the inline flag to wrapInFunctionCall, which is used for ternaries

Comment thread src/LuaTransformer.ts Outdated
Comment thread test/unit/string.spec.ts Outdated
{ a: "test", b: 42, c: true },
{ a: false, b: 42, c: 12 },
])("String Concat Operator (%p)", ({ a, b, c }) => {
const a1 = typeof a === "string" ? "'" + a + "'" : a;

@Perryvw Perryvw Apr 5, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Probably nicer if you do `'${a}'`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's also possible to just JSON.stringify(a)

@tomblind
tomblind merged commit 1cec443 into master Apr 5, 2019
@tomblind
tomblind deleted the string-concat-fix branch April 5, 2019 12:22
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.

String concat needs to convert all values to strings

3 participants