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

Allow acessing :pre snapshot in item itself #538

Merged
merged 1 commit into from Apr 25, 2015

Conversation

@ddfreyne
Copy link
Member

@ddfreyne ddfreyne commented Mar 7, 2015

Fix for #537.

The :pre snapshot can now be accessed within the item itself, provided that the :post snapshot is present (because at that point, the :pre snapshot stops moving).

The implementation of the fix in this PR is conceptually the same as in the aforementioned bug, but refactored.

CC @nanoc/contributors @jethrogb

@ddfreyne
Copy link
Member Author

@ddfreyne ddfreyne commented Apr 18, 2015

Can I get a code review?

Loading

@ddfreyne ddfreyne added this to the 3.7.6 milestone Apr 19, 2015
@ddfreyne ddfreyne added this to the 3.7.6 milestone Apr 19, 2015
@ddfreyne ddfreyne removed this from the 3.7.6 milestone Apr 19, 2015
@ddfreyne ddfreyne removed this from the 3.7.6 milestone Apr 19, 2015
@mpapis
Copy link
Member

@mpapis mpapis commented Apr 19, 2015

👍 looks reasonable - although I do not have full understanding of it so I might be missing something

Loading

@bobthecow
Copy link
Member

@bobthecow bobthecow commented Apr 19, 2015

Is the :pre snapshot usable after other (explicit) snapshots have been created but before :post is present?

Loading

@ddfreyne
Copy link
Member Author

@ddfreyne ddfreyne commented Apr 19, 2015

Yeah, :pre is always usable.

Edit: :pre is usable except in the item itself. See this comment.

Loading

@bobthecow
Copy link
Member

@bobthecow bobthecow commented Apr 20, 2015

Then maybe I missed something? I thought this was setting it up to allow accessing :pre once :post is present?

Loading

@ddfreyne
Copy link
Member Author

@ddfreyne ddfreyne commented Apr 20, 2015

@bobthecow This allows accessing :pre from the item itself, once :post is set. Previously, accessing :pre was only possible from other items.

Loading

@bobthecow
Copy link
Member

@bobthecow bobthecow commented Apr 20, 2015

Right. So I was asking if you should be able to access :pre from the item itself, once, e.g., :mysnapshot is set.

Loading

@ddfreyne
Copy link
Member Author

@ddfreyne ddfreyne commented Apr 21, 2015

@bobthecow Hm, re-reading the original question, I think my answer is misleading.

From within the item itself, using :pre when there is no :post snapshot will lead to an exception (since you’d recursively let compiled content depend on itself). When there is a :post snapshot, that works from within the item itself (since :pre cannot move anymore).

When accessing the compiled content from another item, accessing :pre is possible even if there is no :post: nanoc will compile the accessed item first, guaranteeing that :pre is in its final position.

These moving pre-snapshots and post-snapshots are difficult to reason about and one of the reasons why they’ll be gone in nanoc 4!

Loading

@gpakosz
Copy link
Member

@gpakosz gpakosz commented Apr 21, 2015

Does it make :pre accessible at the time you're laying out an item? A while ago, when I tried to extract a TOC from @item.compiled_content(:snapshot => :pre) inside my default layout.haml layout, I think nanoc bailed out. So I used yield instead:

%main#content.container
  - toc = extract_toc(yield, :depth => @item[:toc_depth])

Loading

@ddfreyne
Copy link
Member Author

@ddfreyne ddfreyne commented Apr 21, 2015

@gpakosz That’s still not possible, but yield is intended to be used in that case anyway.

Loading

ddfreyne added a commit that referenced this issue Apr 25, 2015
Allow acessing :pre snapshot in item itself
@ddfreyne ddfreyne merged commit fb21273 into release-3.7.x Apr 25, 2015
2 checks passed
Loading
@ddfreyne ddfreyne deleted the allow-accessing-pre-in-self branch Apr 25, 2015
ddfreyne added a commit that referenced this issue May 1, 2015
@ddfreyne ddfreyne added this to the 3.7.6 milestone May 4, 2015
@ddfreyne ddfreyne added this to the 3.7.6 milestone May 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants