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

String interpolation #614

Merged
merged 91 commits into from
Aug 24, 2022
Merged

Conversation

Kampfkarren
Copy link
Contributor

@Kampfkarren Kampfkarren commented Jul 27, 2022

Implements the string interpolation RFC (#165).

TODOs are commented as "INTERP TODO:", things I want highlighted in code review as "INTERP CODE REVIEW:"

This is going to diverge from RFC a bit in making {{ a syntax error, as you never want to format a table you just created, and it is likely to be a common error from those coming from Rust or Python when escaping braces.

local name = "world"
print(`Hello {name}!`) -- Hello world!

Big TODOs:

  • FFlag (Does not include %* string.format specifier)
  • Check that this resolves to string
  • Does other work need to be done for linters/type checker?
  • JSON encoder?
  • Fix MSVC compiler warnings
  • Reserve, implement, or concretely reject format specifiers -- Needs comment from maintainers
  • Support f`x`

@Kampfkarren Kampfkarren changed the title String interpolation [WIP] String interpolation Jul 27, 2022
@Kampfkarren
Copy link
Contributor Author

Also reminding myself I need a should have a test for something like f`` within an interpolation expression

Ast/include/Luau/Lexer.h Outdated Show resolved Hide resolved
Ast/include/Luau/Lexer.h Show resolved Hide resolved
tests/conformance/stringinterp.lua Outdated Show resolved Hide resolved
Ast/include/Luau/Ast.h Show resolved Hide resolved
Ast/src/Lexer.cpp Outdated Show resolved Hide resolved
tests/Parser.test.cpp Outdated Show resolved Hide resolved
tests/conformance/stringinterp.lua Show resolved Hide resolved
Ast/src/Lexer.cpp Outdated Show resolved Hide resolved
Ast/src/Lexer.cpp Outdated Show resolved Hide resolved
tests/Parser.test.cpp Show resolved Hide resolved
tests/Lexer.test.cpp Outdated Show resolved Hide resolved
Ast/src/Lexer.cpp Outdated Show resolved Hide resolved
@Kampfkarren
Copy link
Contributor Author

Without changing anything about Autocomplete.cpp, this test passed, and properly didn't pass when I put @1 outside of braces:

TEST_CASE_FIXTURE(ACFixture, "autocomplete_interpolated_string")
{
    check(R"(f(`expression = {@1}`))");

    auto ac = autocomplete('1');
    CHECK(ac.entryMap.count("table"));
    CHECK_EQ(ac.context, AutocompleteContext::Expression);
}

Not sure why immediately.

Do I also need to handle making sure that pressing one backtick creates the other?

@alexmccord
Copy link
Contributor

Without changing anything about Autocomplete.cpp, this test passed, and properly didn't pass when I put @1 outside of braces:

Ah, my bad. It worked out for free in our case, because autocomplete will look up for the best scope given a location in the document, which is what happened in the test above.

Do I also need to handle making sure that pressing one backtick creates the other?

I don't think so, this sounds like a script editor feature. I don't think there's any extra special behavior we'd want autocompleter to do here, so if it works let's call it a day for that.

Copy link
Contributor

@alexmccord alexmccord left a comment

Choose a reason for hiding this comment

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

LGTM. This is fantastic work!

Ast/src/Lexer.cpp Show resolved Hide resolved
@zeux
Copy link
Collaborator

zeux commented Aug 23, 2022

Thanks for working through this! Just a heads up that I submitted a couple commits to this PR to make it merge-ready, most significantly outlawing the function calls without parentheses yet again - this will be added back to the RFC: #648. We may add this in the future but there are some exciting possibilities for how string interpolation interacts with DSLs and I'd like to keep this road open even if we end up not taking it for now. No action necessary since I've updated both the RFC and this PR accordingly, but third party implementations like full-moon may need to get updated.

@Kampfkarren
Copy link
Contributor Author

Doesn't look like there's anything I filed left :)

@alexmccord alexmccord merged commit da9d8e8 into luau-lang:master Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants