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

Clarification on root-level implicit iterators #87

Closed
Mr0grog opened this issue Apr 17, 2015 · 11 comments · Fixed by #176
Closed

Clarification on root-level implicit iterators #87

Mr0grog opened this issue Apr 17, 2015 · 11 comments · Fixed by #176
Labels
documentation human-oriented text could be improved omission accidentally left unspecified

Comments

@Mr0grog
Copy link
Contributor

Mr0grog commented Apr 17, 2015

Can the implicit iterator be used to iterate over a list if that list is the root level item on the context stack? e.g. should this: {{#.}} {{name}} {{/.}} work with this data: [{name: Alice}, {name: Bob}] to generate: Alice Bob?

From a reading of the interpolation spec it sounds like it should:

A single period (.) indicates that the item currently sitting atop the context stack should be used

But there’s actually no mention of implicit iterators in the descriptive part of the sections spec and the tests it contains do not cover this case. It just so happens that some implementations also have conflicting behavior here.

I recently finished switching all of a client’s Mustache.js-rendered templates to being precompiled with Hogan.js. In Mustache, the scenario described above works fine, but in Hogan it does not. In a true test of Murphy’s Law, I ran afoul of this issue during the switch.

It would be great to get some clarification in the tests themselves and, ideally, in the descriptive part of the spec as well.

Hogan has a long-standing bug for this; some clarity from this spec would probably go a long way towards resolving the issue, either by forcing Hogan to change through failing tests or by clarifying that behavior isn’t one Hogan needs to support.

@groue
Copy link

groue commented Apr 17, 2015

Exceptions are not good: it would be better if . (dot) would always evaluate to the current item, regardless of its position in the context stack, at the root or not.

And yes, the spec needs a clarification on that.

@Mr0grog
Copy link
Contributor Author

Mr0grog commented Apr 20, 2015

Is there a process for proposing the change? Do I just open a PR?

I think the right thing here is to add the . language from the interpolation spec to the sections spec: “A single period (.) indicates that the item currently sitting atop the context stack should be used”

And add this test to it, as well:

- name: Implicit Iterator - Root-level
  desc: Implicit iterators should work on root-level lists.
  data: [ { value: 'a' }, { value: 'b' } ]
  template: '"{{#.}}({{value}}){{/.}}"'
  expected: '"(a)(b)"'

Not sure if it makes sense to add test(s) to the interpolation spec, too, e.g:

- name: Dotted Names - Single Period
  desc: A single period should resolve to the top of the context stack.
  data:
    name: 'Alice'
  template: '{{#name}}({{.}}){{/name}}'
  expected: '(Alice)'

- name: Dotted Names - Root-level Single Period
  desc: A single period at the root level should resolve to the passed-in data.
  data: 'Alice'
  template: '"{{.}}"'
  expected: '"Alice"'

In particular, I can imagine that last one might not be reasonable for a lot of implementations.

@Mr0grog
Copy link
Contributor Author

Mr0grog commented Apr 27, 2015

Any updates or feedback on this? Anyone?

@groue
Copy link

groue commented Apr 27, 2015

The repo maintainers have not given a sign of life for years.

@Mr0grog
Copy link
Contributor Author

Mr0grog commented Apr 27, 2015

:(

@groue
Copy link

groue commented Apr 28, 2015

This does not mean that Hogan itself is not maintained!

@Mr0grog
Copy link
Contributor Author

Mr0grog commented Apr 28, 2015

Oh, for sure. I have an actual PR sitting (equally long) over there.

kevinoid added a commit to kevinoid/mustache-spec that referenced this issue Nov 3, 2018
Based on the discussion at
mustache#87 (comment)

Signed-off-by: Kevin Locke <[email protected]>
@jgonggrijp jgonggrijp added the documentation human-oriented text could be improved label Nov 6, 2023
@jgonggrijp
Copy link
Member

@Mr0grog We have active maintenance again and I encourage you to submit a pull request with the clarifications that you have in mind.

@Mr0grog
Copy link
Contributor Author

Mr0grog commented Nov 6, 2023

Oh wow, I haven’t thought about this issue in ages! Will try to bring myself back up to speed on this and submit something when I have time. (If someone else watching is more actively experiencing this and is more up-to-speed, feel free to take this on.)

@bobthecow
Copy link
Member

Mustache.php does the "right thing" with the previous examples, as well as this monstrosity:

- name: Shenanigans
  desc: This is silly, but should be allowed.
  data: [ 'a', 'b' ]
  template: '"{{#.}}({{.}}){{/.}}"'
  expected: '"(a)(b)"'

… to be honest I don't hate it? I mean, you shouldn't use it. But it's consistent. I don't hate that it's possible.

Mr0grog added a commit to Mr0grog/mustache-spec that referenced this issue Nov 15, 2023
Since some implementations differ on whether the root of the context stack can be iterated over, this adds langauge to the "Sections" spec clarifying that it can. It also adds a test for this case.

Fixes mustache#87. I did not add the two additional tests for the "Interpolation" spec that I recommended in that issue, since there are tests that cover them now (not sure if these are new or if I just missed them before).
@Mr0grog
Copy link
Contributor Author

Mr0grog commented Nov 15, 2023

@jgonggrijp OK, I added a PR for this in #176. I changed the spec language in a slightly different way from what I proposed here, but I think it’s better. The two extra tests I suggested adding to the "Interpolation" spec already exist now, so I skipped them (not sure if I missed them before or if they are new).

@jgonggrijp jgonggrijp added the omission accidentally left unspecified label Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation human-oriented text could be improved omission accidentally left unspecified
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants