Accessing pre snapshot in layout results in Circular dependency error #537

Closed
opened this Issue Feb 23, 2015 · 10 comments

Projects
None yet
2 participants

jethrogb commented Feb 23, 2015

 When I try to access the :pre compilied snapshot for an item in a layout, I get a Circular dependency error. The final :pre snapshot gets saved at https://github.com/nanoc/nanoc/blob/7a297646/lib/nanoc/base/result_data/item_rep.rb#L380 just before the layout. Is there any reason I shouldn't be accessing this snapshot at this time? Use case layout excerpt: <% if @item.compiled_content(snapshot: :pre).include? 'type="math/tex' then %> <% end %>

Member

ddfreyne commented Feb 28, 2015

 Not being able to access the :pre snapshot seems like a bug to me. First of all, to avoid the issue you’re seeing, you can use yield to get the content right before the layout is applied. For example: <% if yield.include? 'type="math/tex' then %> item_rep.rb, line 247 has the check !self.compiled? && is_moving; if that expression is true, an unmet dependency error will be raised, resulting in the circular dependency issue. The reasoning here is that you should not be able to access any “moving” snapshot (i.e. :pre, :post, or :last) from the item itself. That check is accurate for the :post and :last snapshot, since these snapshots keep moving until the item is fully compiled. For the :pre snapshot, this check can probably be relaxed, because :pre snapshots will not move again once they’re created. So, instead of this: if @content[snapshot].nil? || (!self.compiled? && is_moving) …it’d probably be possible to have this: if @content[snapshot].nil? || ([:post, :last].include?(snapshot) && !self.compiled?) || (snapshot == :pre && @content[:pre].nil?) I don’t think there’s any reasonable situation where yield would not be usable, though.
Member

ddfreyne commented Feb 28, 2015

 On second thought, my proposed fix doesn’t work. This line: if @content[snapshot].nil? || ([:post, :last].include?(snapshot) && !self.compiled?) || (snapshot == :pre && @content[:pre].nil?) is equivalent to if @content[snapshot].nil? || ([:post, :last].include?(snapshot) && !self.compiled?) which clearly shows that :pre is not handled in a special way at all, even though it should, since it is a moving snapshot: just because a :pre snapshot exists, does not mean it cannot move in the future. The only condition under which the :pre snapshot stops moving is when a :post snapshot is present. So, the condition could be as follows: if @content[snapshot].nil? || ([:post, :last].include?(snapshot) && !self.compiled?) || (snapshot == :pre && @content[:post].nil?)

Merged

jethrogb commented Mar 10, 2015

 #538 is not sufficient for fixing this. The proposed logic in ItemRep::compiled_content might make sense, but it does not match up with ItemRep::layout: there is a state where the final :pre snapshot has been created but a :post snapshot does not yet exist: https://github.com/nanoc/nanoc/blob/7a297646/lib/nanoc/base/result_data/item_rep.rb#L380-398
Member

ddfreyne commented Mar 14, 2015

 To clarify: the final argument doesn’t have a meaning other than “the output file can be written to disk”—it doesn’t mark the snapshot as immovable. (In hindsight, probably not the best decision, but this could be difficult to change without breaking the API.) The recommended solution is to use yield in a layout. In helpers used in layouts, that means passing in the content: # in the layout <%= snippet(yield) %> # in lib/helpers/snippet.rb or so def snippet(content) if content.size > 100 content[0..99] + '…' else content end end (This is a silly example, because there’s an #excerptize function in the Text helper.) Making compiled_content(snapshot: :pre) available in layouts might be tricky, since the logic for determining whether :pre is immovable is determined by the presence of :post, which will not be there at the time of the first layout. I’m actually in favour of documenting the current behavior as well as the recommended approach to avoid the issue (in other words, don’t use compiled_content(snapshot: :pre) in layouts, but use yield).
Member

ddfreyne commented Mar 14, 2015

 I’ll look into improving this (i.e. making compiled_content(snapshot: :pre) possible within a layout), but it’s definitely an enhancement rather than a bug fix, since the content is already available using yield.
Member

ddfreyne commented May 1, 2015

 Fixed by #538.

jethrogb commented May 1, 2015

 As I mentioned before, this is not fixed by #538. Please re-open.

ddfreyne added a commit that referenced this issue May 1, 2015

 Add #537, #538 to release notes 
 04aa1fe 
Member

ddfreyne commented May 1, 2015

 @jethrogb I’m inclined to improve this in a future release rather than 3.8.0, and consider the fix that’s currently in 3.8 as good enough. Accessing the :pre content from within a layout is not particularly useful because that content is already available through yield.

ddfreyne added a commit that referenced this issue May 1, 2015

 Remove mention of #537 from release notes 
 50c94a6 

Merged

Member

ddfreyne commented May 1, 2015

 Full fix (work in progress) in #548.
Member

ddfreyne commented May 4, 2015

 Fixed by #548.
{"extraction_info": {"found_math": true, "script_math_tex": 0, "script_math_asciimath": 0, "math_annotations": 0, "math_alttext": 0, "mathml": 0, "mathjax_tag": 0, "mathjax_inline_tex": 0, "mathjax_display_tex": 0, "mathjax_asciimath": 1, "img_math": 0, "codecogs_latex": 0, "wp_latex": 0, "mimetex.cgi": 0, "/images/math/codecogs": 0, "mathtex.cgi": 0, "katex": 0, "math-container": 0, "wp-katex-eq": 0, "align": 0, "equation": 0, "x-ck12": 0, "texerror": 0, "math_score": 0.3188592493534088, "perplexity": 3799.115939551463}, "config": {"markdown_headings": false, "markdown_code": true, "boilerplate_config": {"ratio_threshold": 0.18, "absolute_threshold": 10, "end_threshold": 15, "enable": true}, "remove_buttons": true, "remove_image_figures": true, "remove_link_clusters": true, "table_config": {"min_rows": 2, "min_cols": 3, "format": "plain"}, "remove_chinese": true, "remove_edit_buttons": true, "extract_latex": true}, "warc_path": "s3://commoncrawl/crawl-data/CC-MAIN-2018-39/segments/1537267158279.11/warc/CC-MAIN-20180922084059-20180922104459-00365.warc.gz"}