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

CS1 tagged template literals (and CS2 interpolated strings as template literals) #4352

Merged
merged 34 commits into from
Nov 18, 2016

Conversation

greghuc
Copy link
Contributor

@greghuc greghuc commented Nov 5, 2016

Tested implementation of tagged template literals, as defined at: coffeescript6/discuss#28 (comment)

For CS2, we also get interpolated strings outputting as ES6 template literals. This fell out of the tagged template literal work.

@greghuc greghuc changed the title WIP: tagged template literals [wip] tagged template literals Nov 5, 2016
@connec
Copy link
Collaborator

connec commented Nov 5, 2016

I was thinking that it would be really nice to do this as two commits, if not two PRs:

  1. (hopefully) fairly simple one to compile StringWithInterpolations to template literals.
  2. support tagged template literals (tagged string interpolations?)

@greghuc
Copy link
Contributor Author

greghuc commented Nov 5, 2016

@connec the current plan is to add tagged template literal support, as that's needed for ES6 compatibility. This feature could be published as CS 1.9.x, as it's an opt-in feature. We'd then compile StringWithInterpolations to template literals separately, but that has to be on the CS2 branch as it's not opt-in (all interpolated strings would start being transpiled to ES6 code).

However, your suggestion of doing StringWithInterpolations to template literals first might make from sense from an incremental development perspective. Today was my first day hacking on Coffeescript, so I'm definitely feeling my way on this :-)

a"""#{b}"""
^^^^^^^^^^
'''
# TODO: Tagged template literals impl: disabling these tests, as now valid 'tagged template literals'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rather than commenting these out, I would move these tests into test/tagged_template_literals.coffee and update them accordingly. This is a pretty thorough review of all the ways tagged template literals can be written, from a'' to a"""b""" and so on.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Rather than moving and changing these tests, I would change all the as to 1s. For example, "a''""1''". This is about ensuring that unexpected string errors work correctly, and is still a valid thing to test after tagged template literals are added.


func = (text) -> "I saw: #{text}"

eq 'I saw: a single line string', func'a single line string'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don’t forget the test line, e.g. test "basic tagged template literal", ->

@greghuc greghuc changed the title [wip] tagged template literals [WIP] tagged template literals Nov 6, 2016
use number not identifier prefix.

Identifer prefixes are now valid as tagged
template literals
block string
"""

test "tagged template literals: string prefix must be function", ->
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not put these in test/error_messages.coffee?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lydell since you're around right now, are you up for a webchat about tagged template literals? I can do Skype on my github username, or Google Hangouts on ghuczynski at gmail.com

I'm working on tagged template literals today Sunday, and I'd appreciate talking to someone with more CS compiler knowledge than me!

Copy link
Collaborator

Choose a reason for hiding this comment

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

I’m sorry, I won’t have the time.

eq 'text: [single-line block string] expressions: []', func"""single-line block string"""

eq 'text: [multi-line single quotes] expressions: []', func'multi-line
single quotes'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please avoid this kind of crazy indentation :)

Pull tagged template definition up to Invocation
level in grammar, enabling chained invocation calls.

We can view a tagged template is a special form
of function call.
@greghuc
Copy link
Contributor Author

greghuc commented Nov 6, 2016

Tagged template literals status update.

So I've got tagged template literals working for pure, non-interpolated Strings (code will need tidy up before checkin).

Interpolated strings are next. This will require StringWithInterpolations in nodes.coffee to be custom-output as a ES6 template literal. E.g:

- CS input:   tagfunc"single-line double quotes #{6 + 7} interpolation"
- ES6 output: tagfunc`single-line double quotes ${6 + 7} interpolation`
- Not:        tagfunc"single-line double quotes " + (6 + 7) + " interpolation"

I'm still working out how to do this in nodes.coffee, so any input is appreciated. StringWithInterpolations is a very general class, since it's a Parens wrapping a body.

Rough rules for outputting StringWithInterpolations as a ES6 template literal:

  • Ignore pluses between string literals and bracketed expressions
  • Output bracketed expressions as ${}, not ()
  • String literals have no start or end quotes
  • Entire thing is wrapped in ``
  • And make sure this works recursively

@GeoffreyBooth
Copy link
Collaborator

I assume you’re not intending to support tagfunc"a " + (6 + 7) + " b"? That would seem to be ambiguous as to where the tagged template function ends—is it just tagfunc"a ", or the whole thing? I would think it should be parsed as just tagfunc"a ", and if people want to interpolate inside the string they need to use #{}, not +.

@greghuc
Copy link
Contributor Author

greghuc commented Nov 6, 2016

@GeoffreyBooth only tagfunc"a " would be parsed as a tagged template literal.

Using your example, here's the desired Coffeescript to Javascript compilation:

Coffeescript input:    tagfunc"a #{6 + 7} b"
Javascript ES6 output: tagfunc`a ${6 + 7} b`

At present, Coffeescript outputs "a #{6 + 7} b" to the Javascript of "a " + (6 + 7) + " b".

For tagged template literals, we need the Coffeescript to output instead as a ${6 + 7} b (its ES6 template literal form).

@connec
Copy link
Collaborator

connec commented Nov 6, 2016

I've had a quick look at what would need to be done to compile interpolated strings to template literals, and it's not as easy as I'd hoped since most of the work is done in the lexer.

I'd probably attempt to avoid making changes to the grammar and lexer (as far as template literals are concerned, ofc tagging will need some grammar changes). One approach could be to flesh out StringWithInterpolations to mirror the arguments given to tagged template literals, e.g. it could have a strings child for a list of StringLiteral nodes for the strings, and an interpolations child for a list of Parens nodes for the interpolated expressions:

"a #{b} c"

# StringWithInterpolations
#   strings: [
#     StringLiteral 'a '
#     StringLiteral ' c'
#   ]
#   interpolations: [
#     IdentifierLiteral 'b'
#   ]

It should be possible to build that structure from the body that's currently passed to new StringWithInterpolations in grammar.coffee by flattening out the Op + nodes. The compileNode for StringWithInterpolations would simply iterate over the strings and interpolations, compile them and wrap them in ${...}, and wrap the whole thing in ````.

@greghuc
Copy link
Contributor Author

greghuc commented Nov 13, 2016

I checked the new code with a project I have which uses yo-yo, which in turn uses bel. It worked fine: I was able to do html templating in a pure coffeescript file. Yay!

@greghuc
Copy link
Contributor Author

greghuc commented Nov 13, 2016

Having thought about unwrap more, I think the safest thing to do is to remove it temporarily, as it's not needed as part of the tagged template literal work. @GeoffreyBooth, if you want to do this then that's fine.

We should then deal with unwrap again as part of a separate work item for CS2: "output interpolated strings as template literals". For that, we should remove uncertainty by changing StringWithInterpolations to extend Base, not Parens. Parenstheses aren't needed for template literals, as a template literal is already enclosed by its ' markers. And StringWithInterpolations will have to have correct implementations for the standard methods (unwrap, children, isComplex, etc).

@GeoffreyBooth
Copy link
Collaborator

I commented out unwrap for now, with a comment that it should be reenabled for CoffeeScript 2. I think this PR is probably good to go. @jashkenas or @lydell, any notes?

@jashkenas
Copy link
Owner

This looks fine to me from a quick glance.

As an aside — for this sort of thing, it feels like you guys are stealing a little bit of your own thunder by maintaining the CS1 compatibility. If you just released the even-more-streamlined version for CS2 only, I bet folks would get more excited about CS2. In short: It's awfully nice of you.

@GeoffreyBooth
Copy link
Collaborator

@jashkenas Yeah, but the code is right here for anyone to adapt as a 1.x PR. It was written originally as a 2-only PR, but it was made 1.x-friendly by commenting out about three lines of code, so it was inevitable that people would figure it out 😄

The 2 branch has two big features that should hopefully motivate people to upgrade: classes that can extend ES classes, and async/await. Those two alone should be enough for a big chunk of people.

@GeoffreyBooth GeoffreyBooth merged commit 78e1f43 into jashkenas:master Nov 18, 2016
@GeoffreyBooth
Copy link
Collaborator

@greghuc Congratulations! Would you like to follow this up with a PR adding documentation for this? And another PR for 2 that outputs all interpolated strings as ES template literals?

@GeoffreyBooth GeoffreyBooth deleted the tagged-template-literals branch November 18, 2016 18:26
@greghuc
Copy link
Contributor Author

greghuc commented Nov 18, 2016

@GeoffreyBooth thanks! Re the PRs, I'll aim to fit them in over the next week.

Quick question: where's the right place to be doing the follow-on PRs?

  • For template literal documentation, I presume we're talking about the current CS1 documentation
  • For interpolated strings in CS2, what/where is the correct CS2 branch to use? This PR will be a bit more than just uncommenting the lines discussed. As I mentioned before, StringWithInterpolations should extend Base - not Parens - so will need some tweaking, and likely some input from people with more knowledge of nodes.coffee

@GeoffreyBooth
Copy link
Collaborator

@greghuc for documentation yes, the current 1.x docs. They’re in the documentation folder; the docs folder is for the generated output. Please don’t commit anything in the docs folder; that only gets updated when we release. Branch off of master.

For interpolated strings, branch off of 2. I’ve already merged master into it, so it should have your latest tagged template literals code.

@lydell
Copy link
Collaborator

lydell commented Nov 19, 2016

@greghuc I've done plenty of refactoring regarding strings with interpolations – all the way from the lexer to nodes.coffee. Feel free to ping me in PRs and I'll try to help out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants