Support for @-prefixed keywords in the each block helper

[lai]

This adds the support for data keywords prefixed by @ such as @index, @key in Handlebars' each block helper. Prior discussion about supporting @index at #112

Note that support for @first and @index in object iteration was added in Handlebars in 1.2.0. handlebars-lang/handlebars.js#661

[machty]

I screwed up here by prematurely merging in the previous change to add support for @index; we don't want to handle @key and @index as individual special case keywords; rather, we want to support any data keyword that's prefixed with an @. This way, the helpers have full control over what data vars to add.

So this rule needs to be changed to handle @ followed by any valid identifier, not just @key and @index and whatever else might come in the future. Would you mind making this change?

[lai]

Sounds good. Does using attributeName here seem reasonable? It's defined as

attributeName = $attributeChar*
attributeChar = alpha / [0-9] /'_' / '-'

So it would just be

eachBlockHelperVariable
  = !'{' _ '@' variableName:attributeName

[machty]

That seems fine to me.

[lai]

I'm writing this test case that uses @first as the if block argument, it seems the syntax is not covered though currently:

// Support for data keywords that are prefixed with @ in the each
// block helper such as @index, @key, @first, @last
eachBlockHelperVariable
  =  _ '@' variableName:attributeName
{
  var params = {};
  var idNode = new AST.IdNode([{part: variableName}]);
  var dataNode = new AST.DataNode(idNode);
  params = dataNode;
  return new AST.MustacheNode([params], null, false);
}


recursivelyParsedMustacheContent
  = !'{' text:$[^}]*
{
  // Force interpretation as mustache.
  // TODO: change to just parse with a specific rule?
  text = "=" + text;
  return Emblem.parse(text).statements[0];
}

mustacheContent
 = eachBlockHelperVariable / recursivelyParsedMustacheContent

Is there a better place to define the eachBlockHelperVariable rather than just as some kind of mustacheContent?

[machty]

Hmm, honestly, this commit should be reverted: f15ffef

There's no reason the parsing of @data keyword should be a sibling of recursive mustache content. It should be parsed like any other mustache-y identifier.

Sorry you've been on this wild goose chase; if you still have energy, and I hope you do, here's what you should do:

1. Revert the above commit
2. Add rules somewhere closer to pathIdNode; pathIdNode is used in a few different places, both as a line-starting mustache, an argument to helpers, and a value for attributes.
3. Keep the test cases you already have as well as the one from the above reverted commit, and add additional ones for the cases described in Disable 'view' capitalization helper if suffix modifiers are in place #2.

The recursiveMustacheParse thing is used in the case that you have something like

p Some text and a #{rescursivelyParsedMustache}

It basically restarts the Emblem parser on everything between the curly braces, but you shouldn't have to touch that in any way to get data vars to work.

It shouldn't be an insane amount of work, but you're correct that support should also exist for using data keywords as helper args, in addition to making the logic universal beyond just @index

[lai]

Tests all passed now for Handlebars 1.3.0 and 1.2.1. Since Handlebars 1.1.2 doesn't have support for these iteration variables, is there any way we can conditionally skip some of the tests?

[machty]

@lai Awesome; yeah, i suppose you can check Handlebars.VERSION to decide whether or not to run that test.

[lai]

@machty Please review the amended commit.

[machty]

Looks good, thank you!

[lai]

Thanks! Can't wait to fully embrace Emblem here :)