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

Add more tests for lambdas used in sections #47

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions specs/~lambdas.yml
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,51 @@ tests:
template: "<{{#lambda}}-{{/lambda}}>"
expected: "<-Earth->"

- name: Section - Expansion of List Elements
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you describe the thought behind this test?

I read it as:

  • lambdas is a list
  • {{#lambdas}} should iterate the list
  • If {{.}} for each iteration is a lambda:
    • It should be executed
    • It should receive the raw content string as an argument
    • The content of the iteration loop should be the return value from the lambda

This feels a bit off to me; nowhere else do we treat {{.}} specially. For the behavior you're describing, I'd expect to template {{#lambdas}}{{#.}}planet{{/.}}{{/lambdas}}.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm taking these responses one at a time. Thanks for taking a look at these and thinking about them, btw!

Can you describe the thought behind this test?

Sure. The purpose of this first test is basically to test the same thing as the following existing test--

- name: Section - Expansion
  desc: Lambdas used for sections should have their results parsed.
  ...
  template: "<{{#lambda}}-{{/lambda}}>"

but for a list of lambdas rather than a single lambda.

I'm interpreting the expected behavior for this case from this part of sections.yml:

If the data is not of a list type, it is coerced into a list as follows: if
the data is truthy (e.g. `!!data == true`), use a single-element list
containing the data, otherwise use an empty list.

For each element in the data list, the element MUST be pushed onto the
context stack, the section MUST be rendered, and the element MUST be popped
off the context stack.

combined with this part of ~lambdas.yml:

When used as the data value for a Section tag, the lambda MUST be treatable
as an arity 1 function, and invoked as such (passing a String containing the
unprocessed section contents).  The returned value MUST be rendered against
the current delimiters, then interpolated in place of the section.

The key observation (from the first excerpt above) is that a data element that is not a list is the same as a single-element list containing that data (because a non-list data type is first coerced into a list anyways).

In particular, the following existing test case:

- name: Section - Expansion
  desc: Lambdas used for sections should have their results parsed.
  data:
    planet: "Earth"
    lambda: !code
      python:  'lambda text: "%s{{planet}}%s" % (text, text)'
  template: "<{{#lambda}}-{{/lambda}}>"
  expected: "<-Earth->"

should be equivalent to the following, slightly-modified test case:

- name: Section - Expansion of Contents of Single-Element List
  desc: Lambdas used for sections should have their results parsed.
  data:
    planet: "Earth"
    lambdas:
      - !code
        python:  'lambda text: "%s{{planet}}%s" % (text, text)'
  template: "<{{#lambdas}}-{{/lambdas}}>"
  expected: "<-Earth->"

In other words, the lambda logic in the second excerpt above should be applied individually to each lambda in the list.

The test case I provided above was simply a two-element list instead of a one-element list.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a fascinating interpretation, and I love that you're investing the thought in this.

As mentioned in #46, the language around the handling of lambdas / methods in sections is not strictly accurate to the intent. In particular, the results of an arity 1 function call a) must be stringifiable and b) rendered against the context stack. This makes parts 5 and 6 completely non-applicable (okay, maybe not completely, but it does seem unlikely that you'll commonly find something useful to call on a rendered template string in a list context). The text then goes on to say that the returned data value should be "listified" if not already "listy", and that each thing in the list (i.e. just the rendered template) should be pushed onto the context stack.

The failure points in the current verbage (in sections.yml) seem to be:

  • The return result from an unary function call is described as "data".
  • Calling lambda.y seems to have useless semantics.
  • The return result should be pushed onto the context stack (and hence, never interpolated).

In practice, the desired (and codified) behavior of unary methods is the same as for lambdas: specifically, when encountered, they should be called with the raw content of the section (as per sections.yml, point 4), the returned value rendered against the current context stack and delimiters (as per ~lambdas.yml, ¶3), and the rendered value should be immediately substituted for the section tag (as per ~lambdas.yml, ¶3). Dotted name parts following such a function call have undefined behavior.

Furthermore, the "listification" of data elements doesn't happen until after name resolution (which includes function invocations) has completed. So while 'x': 'foo' and 'x': ['foo'] are equivalent for {{#x}}, 'x': lambda: 'foo' is equivalent to 'x': ['foo'], not 'x': [lambda: 'foo'].

I hope this helps clear things up. If not, please feel free to say so. :)

desc: Lambdas used for sections in a list should each have their results parsed.
data:
planet: "Earth"
lambdas:
- !code
python: 'lambda text: "~{{%s}}~" % text'
- !code
python: 'lambda text: "#{{%s}}#" % text'
template: "<{{#lambdas}}planet{{/lambdas}}>"
expected: "<~Earth~#Earth#>"

- name: Section - Mixed Lists
Copy link
Contributor

Choose a reason for hiding this comment

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

This again comes back to special handling for {{.}}.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just a variation (i.e. slightly more complex version) of the test case above, so we can discuss this one after settling the above. Again, the key is to apply the same logic you would to a non-list data type, but for each element of the list sequentially.

desc: Sections should permit mixed lists of lambdas and non-lambdas.
data:
planet: "Earth"
lambdas:
- !code
python: 'lambda text: "~{{%s}}~" % text'
- 1
template: "<{{#lambdas}}planet{{/lambdas}}>"
expected: "<~Earth~planet>"

- name: Section - Context Stack
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like a really useful test, but I'm having trouble reasoning about how it plays with the expectations we've set up.

Then again, if {{.}} from inside a lambda isn't useful, that seems like an oversight. Any idea what kind of implementation "tricks" are required to make this work?

desc: |
Lambdas used for sections should not be pushed onto the context
stack before rendering their return value.
data:
planet: "Earth"
star: "Sun"
lambda: !code
python: 'lambda text: "~{{star}} %s {{.}}~" % text'
template: "<{{#planet}}{{#lambda}}&{{/lambda}}{{/planet}}>"
expected: "<~Sun & Earth~>"

- name: Section - No Re-interpolation
Copy link
Contributor

Choose a reason for hiding this comment

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

Absolutely. 👍

desc: The lambda return value should not be re-interpolated.
data:
planet: "Earth"
dot: "#{{.}}#"
lambda: !code
python: 'lambda text: "~{{%s}}~" % text'
template: "<{{#planet}}{{#lambda}}dot{{/lambda}}{{/planet}}>"
expected: "<~#{{.}}#~>"

- name: Section - Alternate Delimiters
desc: Lambdas used for sections should parse with the current delimiters.
data:
Expand Down