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

Implement EagerPrintTag and EagerDoTag #558

Merged
merged 11 commits into from
Dec 9, 2020
Merged

Implement EagerPrintTag and EagerDoTag #558

merged 11 commits into from
Dec 9, 2020

Conversation

jasmith-hs
Copy link
Contributor

Part of #532

Since the print tag and do tag are nearly identical (print tags include the expression output, while do tags omit the output), both of their eager implementations are included in this PR.

Print and Do tags are also similar to Set tags, but they are essentially simpler. Rather than taking the output of one or more expressions and storing it into a key on the context scope map, a single expression is simply evaluated, and in the case of a print tag, it is also output. So see #557 for an overview of the SetTag's logic, as the logic in this PR is a simplified version of it.

These are the use cases:

  1. It's a normal print/do tag that doesn't work with or around any deferred values.
    This can be interpreted as normal. The expression is resolved and the value is output if we're dealing with a print tag.

  2. It's trying to evaluate an expression using a deferred value ({% print deferred + 1%}).
    The expression will be simplified if possible by using the ChunkResolver. The print/do tag will be reconstructed/re-output and then it will get registered as an EagerToken on the context.

  3. It's trying to modify a value on the context while in protected mode.
    Just like with a set tag, the target value will become deferred, and the print/do tag will be reconstructed and included in the output to get run during a later pass. This reconstructed tag will get registered as an EagerToken on the context.
    *If the execution of the right-hand side of the equation causes any changes in the context, then additional set tags will need to get prepended to the output to preserve the initial context. *This part is true for anything running in "protected mode" that causes changes to the values on the context scope map.

cc @jboulter @Joeoh let me know if you want to see more detailed examples here

@jasmith-hs jasmith-hs mentioned this pull request Dec 1, 2020
32 tasks
@Test
public void itHandlesDeferredDo() {
context.put("foo", 2);
expectedNodeInterpreter.assertExpectedOutput("handles-deferred-do");
Copy link
Contributor

Choose a reason for hiding this comment

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

just as a personal preference, if inputs and expected files are short enough, I would just put them right in the code here. It's way easier to read when you don't have to flip back and forth between files.

Someday we'll get text blocks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I'll make a note to clean up the one-liners, and text blocks would be perfect here if we get them

throw new TemplateSyntaxException(
interpreter,
tagToken.getImage(),
"Tag 'print' expects expression"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be "do"

interpreter,
true
);
StringJoiner joiner = new StringJoiner(" ");
Copy link
Contributor

@Joeoh Joeoh Dec 3, 2020

Choose a reason for hiding this comment

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

Looks like this logic is the same for both tags, and maybe others? Should we split it out into a function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that is a good call.

Base automatically changed from eager-set to master December 9, 2020 21:21
@jasmith-hs jasmith-hs merged commit d5517d8 into master Dec 9, 2020
@jasmith-hs jasmith-hs deleted the eager-print-do branch December 9, 2020 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants