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

Keep track of parent path for each block in order to resolve relative paths #359

Merged
merged 3 commits into from
Aug 15, 2019

Conversation

liamrharwood
Copy link
Contributor

The way that block tags are resolved is somewhat separate from the rest of the rendering process. This means that the currentPathStack does not correctly reflect the origin file of a block tag at the time it is resolved.

In order to support resolving relative paths within block tags, we need to keep track of the parent path for each block. This PR adds a new BlockInfo object which holds both the list of nodes that are already used to resolve block stubs, as well as an optional parentPath. When each block is rendered, we push its parentPath onto the currentPathStack which can then be used in inner tags to resolve relative paths.

@liamrharwood liamrharwood requested a review from mattcoley August 13, 2019 13:32
Copy link
Collaborator

@mattcoley mattcoley left a comment

Choose a reason for hiding this comment

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

It took me some time to understand what was going on exactly but I think this approach looks good.

@mattcoley mattcoley requested a review from boulter August 13, 2019 14:21
@@ -282,17 +283,22 @@ private void resolveBlockStubs(OutputList output, Stack<String> blockNames) {
for (BlockPlaceholderOutputNode blockPlaceholder : output.getBlocks()) {

if (!blockNames.contains(blockPlaceholder.getBlockName())) {
Collection<List<? extends Node>> blockChain = blocks.get(blockPlaceholder.getBlockName());
List<? extends Node> block = Iterables.getFirst(blockChain, null);
Collection<BlockInfo> blockChain = blocks.get(blockPlaceholder.getBlockName());
Copy link
Contributor

Choose a reason for hiding this comment

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

woo hoo, we're on the blockchain! Let's mine some bitcoin!

blockValueBuilder.addNode(child.render(this));

block.getParentPath().ifPresent(path -> getContext().getCurrentPathStack().pop());
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: path is unused, can be replaced with ()

Copy link
Contributor Author

@liamrharwood liamrharwood Aug 15, 2019

Choose a reason for hiding this comment

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

I'm getting a compile error if I do that: Cannot infer functional interface type
I think it's because ifPresent takes a Consumer.

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.

4 participants