output including file for include tag error #1746

Merged
merged 2 commits into from Nov 23, 2013

Projects

None yet

3 participants

@maul-esel
Contributor

Title sums it up pretty much.

Fixes #1745.

@parkr parkr commented on the diff Nov 22, 2013
lib/jekyll/convertible.rb
@@ -87,7 +87,7 @@ def converter
def render_liquid(content, payload, info, path = nil)
Liquid::Template.parse(content).render!(payload, info)
rescue Tags::IncludeTagError => e
- Jekyll.logger.error "Liquid Exception:", "#{e.message} in #{e.path}"
+ Jekyll.logger.error "Liquid Exception:", "#{e.message} in #{e.path}, included in #{path || self.path}"
raise e
rescue Exception => e
Jekyll.logger.error "Liquid Exception:", "#{e.message} in #{path || self.path}"
@parkr
parkr Nov 22, 2013 Member

Why do we use e.path above, but not here?

@maul-esel
maul-esel Nov 22, 2013 Contributor

Because e.path only exists for Tags::IncludeTagError exceptions, where it references the file the error occured in. Other exceptions represent errors in the convertible's file itself, thus self.path (or path for rendering of layouts).

@parkr
parkr Nov 22, 2013 Member

Thanks for the explanation. 👍

@maul-esel
Contributor

Looking at the tag and @benbalter's issue again, there's something else wrong here. In IncludeTag#render, the rescue clause should not be for the whole method. I'll fix that asap.

@maul-esel maul-esel restrict rescue-clause in IncludeTag#render
As it previously enclosed the whole method, it also
wrapped file validation errors, which is not meant to be.

Fixes #1745.
c5533e9
@mattr-
Member
mattr- commented Nov 23, 2013

Cool. Thanks! ❤️

@mattr- mattr- merged commit 7cb44fd into jekyll:master Nov 23, 2013
@mattr- mattr- added a commit that referenced this pull request Nov 23, 2013
@mattr- mattr- Update history to reflect merge of #1746 33be5b0
@maul-esel maul-esel deleted the maul-esel:missing-include branch Nov 23, 2013
@csim csim added a commit to csim/jekyll that referenced this pull request Nov 24, 2013
@mattr- @csim mattr- + csim Update history to reflect merge of #1746 73047ca
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment