-
-
Notifications
You must be signed in to change notification settings - Fork 382
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
Render blocks in the parent context and assure a block is passed to templates that use @partial-block #620
Changes from 6 commits
7fe1f93
0b057c3
e41b907
91da4d3
cff998b
50e7eb8
0155376
90d11bf
94f37c0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,7 @@ | |
import java.io.FileNotFoundException; | ||
import java.io.IOException; | ||
import java.io.Writer; | ||
import java.util.Arrays; | ||
import java.util.Collections; | ||
import java.util.HashMap; | ||
import java.util.LinkedList; | ||
|
@@ -91,7 +92,7 @@ class Partial extends HelperResolver { | |
* @param hash Template params | ||
*/ | ||
public Partial(final Handlebars handlebars, final Template path, final String context, | ||
final Map<String, Param> hash) { | ||
final Map<String, Param> hash) { | ||
super(handlebars); | ||
this.path = notNull(path, "The path is required."); | ||
this.context = context; | ||
|
@@ -114,20 +115,42 @@ public void after(final Context context, final Writer writer) throws IOException | |
|
||
@Override | ||
protected void merge(final Context context, final Writer writer) | ||
throws IOException { | ||
throws IOException { | ||
try { | ||
String path = this.path.apply(context); | ||
/** Inline partial? */ | ||
LinkedList<Map<String, Template>> partials = context.data(Context.INLINE_PARTIALS); | ||
Map<String, Template> inlineTemplates = partials.getLast(); | ||
Template callee = context.data("callee"); | ||
|
||
final boolean pathIsPartialBlock = "@partial-block".equals(path); | ||
final Template lastPartialBlock = inlineTemplates.get("@partial-block"); | ||
final boolean parentIsNotLastPartialBlock = !isCalleeOf(callee, lastPartialBlock); | ||
|
||
if (pathIsPartialBlock && parentIsNotLastPartialBlock) { | ||
throw new IllegalArgumentException( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Already merged it, but I'm struggling to figure out why we do fail here, can you explain? Here is a test case that fails today bc of this check:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi and thanks for the merge! We'll get back to you about this asap. |
||
callee + " does not provide a @partial-block for " + this | ||
); | ||
} | ||
|
||
if (this.partial != null) { | ||
this.partial.apply(context); | ||
inlineTemplates.put("@partial-block", this.partial); | ||
if (!handlebars.lazyPartialBlockEvaluation()) { | ||
this.partial.apply(context); | ||
} | ||
|
||
inlineTemplates.put("@partial-block", | ||
new PartialBlockForwardingTemplate(this, | ||
this.partial, | ||
inlineTemplates.get("@partial-block"), | ||
callee, | ||
handlebars | ||
) | ||
); | ||
} | ||
|
||
Template template = inlineTemplates.get(path); | ||
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove empty line |
||
if (template == null) { | ||
LinkedList<TemplateSource> invocationStack = context.data(Context.INVOCATION_STACK); | ||
|
||
|
@@ -142,18 +165,20 @@ protected void merge(final Context context, final Writer writer) | |
final String reason; | ||
if (invocationStack.isEmpty()) { | ||
reason = String.format("infinite loop detected, partial '%s' is calling itself", | ||
source.filename()); | ||
source.filename()); | ||
|
||
message = String.format("%s:%s:%s: %s", caller.filename(), line, column, reason); | ||
} else { | ||
reason = String.format( | ||
"infinite loop detected, partial '%s' was previously loaded", source.filename()); | ||
"infinite loop detected, partial '%s' was previously loaded", | ||
source.filename() | ||
); | ||
|
||
message = String.format("%s:%s:%s: %s\n%s", caller.filename(), line, column, reason, | ||
"at " + join(invocationStack, "\nat ")); | ||
"at " + join(invocationStack, "\nat ")); | ||
} | ||
HandlebarsError error = new HandlebarsError(caller.filename(), line, | ||
column, reason, text(), message); | ||
column, reason, text(), message); | ||
throw new HandlebarsException(error); | ||
} | ||
|
||
|
@@ -171,18 +196,37 @@ protected void merge(final Context context, final Writer writer) | |
} | ||
|
||
} | ||
context.data("callee", this); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is something internal and it shouldn't be called |
||
Context ctx = Context.newPartialContext(context, this.scontext, hash(context)); | ||
template.apply(ctx, writer); | ||
context.data("callee", callee); | ||
} catch (IOException ex) { | ||
String reason = String.format("The partial '%s' at '%s' could not be found", | ||
loader.resolve(path.text()), ex.getMessage()); | ||
loader.resolve(path.text()), ex.getMessage()); | ||
String message = String.format("%s:%s:%s: %s", filename, line, column, reason); | ||
HandlebarsError error = new HandlebarsError(filename, line, | ||
column, reason, text(), message); | ||
column, reason, text(), message); | ||
throw new HandlebarsException(error); | ||
} | ||
} | ||
|
||
/** | ||
* @param callee parent template of the currently traversed template | ||
* @param partialBlock partial block candidate | ||
* @return returns if callee and partialBlock are the same | ||
*/ | ||
private boolean isCalleeOf(final Template callee, final Template partialBlock) { | ||
if (callee == null || partialBlock == null) { | ||
return false; | ||
} | ||
|
||
if (!callee.filename().equalsIgnoreCase(partialBlock.filename())) { | ||
return false; | ||
} | ||
|
||
return Arrays.equals(callee.position(), partialBlock.position()); | ||
} | ||
|
||
/** | ||
* True, if the file was already processed. | ||
* | ||
|
@@ -191,7 +235,7 @@ protected void merge(final Context context, final Writer writer) | |
* @return True, if the file was already processed. | ||
*/ | ||
private static boolean exists(final List<TemplateSource> invocationStack, | ||
final String filename) { | ||
final String filename) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please revert all the whitespace changes |
||
for (TemplateSource ts : invocationStack) { | ||
if (ts.filename().equals(filename)) { | ||
return true; | ||
|
@@ -268,8 +312,8 @@ private String partialInput(final String input, final String indent) { | |
public String text() { | ||
String path = this.path.text(); | ||
StringBuilder buffer = new StringBuilder(startDelimiter) | ||
.append('>') | ||
.append(path); | ||
.append('>') | ||
.append(path); | ||
|
||
if (context != null) { | ||
buffer.append(' ').append(context); | ||
|
@@ -279,7 +323,7 @@ public String text() { | |
|
||
if (this.partial != null) { | ||
buffer.append(this.partial.text()).append(startDelimiter, 0, startDelimiter.length() - 1) | ||
.append("/").append(path).append(endDelimiter); | ||
.append("/").append(path).append(endDelimiter); | ||
} | ||
return buffer.toString(); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
/** | ||
* Copyright (c) 2012-2015 Edgar Espina | ||
* | ||
* This file is part of Handlebars.java. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package com.github.jknack.handlebars.internal; | ||
|
||
import com.github.jknack.handlebars.Context; | ||
import com.github.jknack.handlebars.Handlebars; | ||
import com.github.jknack.handlebars.Template; | ||
|
||
import java.io.IOException; | ||
import java.io.Writer; | ||
import java.util.LinkedList; | ||
import java.util.Map; | ||
|
||
/** | ||
* | ||
*/ | ||
public class PartialBlockForwardingTemplate extends BaseTemplate { | ||
/** | ||
* The block to be passed as partial-block. | ||
*/ | ||
private final Template block; | ||
|
||
/** | ||
* The previous partial-block definition of the template which contains this partial. | ||
*/ | ||
private final Template parentPartialBlock; | ||
|
||
/** | ||
* The callee of the parent partial. | ||
*/ | ||
private final Template callee; | ||
|
||
/** | ||
* Constructs a PartialBlockForwardingTemplate. | ||
* | ||
* @param parent the parent partial | ||
* @param block the block to be passed as partial-block. | ||
* @param parentPartialBlock the previous partial-block definition of | ||
* the template which contains this partial. | ||
* @param callee the template that renders the parent | ||
* @param handlebars handlebars | ||
*/ | ||
public PartialBlockForwardingTemplate( | ||
final Template parent, | ||
final Template block, | ||
final Template parentPartialBlock, | ||
final Template callee, | ||
final Handlebars handlebars | ||
) { | ||
super(handlebars); | ||
this.block = block; | ||
this.parentPartialBlock = parentPartialBlock; | ||
this.callee = callee; | ||
this.filename(block.filename()); | ||
this.position(parent.position()[0], parent.position()[1]); | ||
} | ||
|
||
/** | ||
* {@inheritDoc} | ||
*/ | ||
@Override | ||
protected void merge(final Context context, final Writer writer) throws IOException { | ||
LinkedList<Map<String, Template>> partials = context.data(Context.INLINE_PARTIALS); | ||
Map<String, Template> inlineTemplates = partials.getLast(); | ||
Template oldPartialBlock = inlineTemplates.get("@partial-block"); | ||
Template oldCallee = context.data("callee"); | ||
|
||
context.data("callee", callee); | ||
inlineTemplates.put("@partial-block", parentPartialBlock); | ||
block.apply(context, writer); | ||
inlineTemplates.put("@partial-block", oldPartialBlock); | ||
context.data("callee", oldCallee); | ||
} | ||
|
||
/** | ||
* {@inheritDoc} | ||
*/ | ||
@Override | ||
public String text() { | ||
return block.text(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add two examples when this flag is on/off
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey I added two tests that deal with this pull request's changes. Is everything okay now?