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

Add debug output to theme rendering #5195

Merged
merged 5 commits into from Mar 5, 2018

Conversation

Projects
None yet
6 participants
@benbalter
Contributor

benbalter commented Aug 3, 2016

Trying to debug some theme issues locally, I found this debug output helpful, and thought rather than silently swallowing errors, it'd be better to at least make warning level notices for missing files or invalid directories.

@@ -139,6 +139,11 @@ def place_in_layouts(content, payload, info)
output = content.dup
layout = site.layouts[document.data["layout"]]
if layout
layout_source = layout.path.start_with?(site.source) ? :site : :theme
Jekyll.logger.debug("Layout source:", layout_source)

This comment has been minimized.

@envygeeks

envygeeks Aug 3, 2016

Contributor

I'd much rather this be folded off into a new line without the variable being created for the GC to handle.

@@ -6,6 +6,8 @@ class Theme
def initialize(name)
@name = name.downcase.strip
Jekyll.logger.debug "Theme:", name
Jekyll.logger.debug "Theme source:", root
configure_sass

This comment has been minimized.

@ashmaroli

ashmaroli Aug 4, 2016

Member

I added one more line in my local copy to check if path to layouts was as expected in Windows:

Jekyll.logger.debug "Layouts Source:", layouts_path

This comment has been minimized.

@parkr

parkr Sep 12, 2016

Member

👍

@ashmaroli ashmaroli referenced this pull request Aug 4, 2016

Closed

Blank new blog generated with Jekyll-3.2.1 on Windows #5192

5 of 5 tasks complete
@ashmaroli

This comment has been minimized.

Member

ashmaroli commented on 4da5d55 Jan 29, 2018

@benbalter I ❤️ the proposal to "memoize paths to the directories in a theme-gem"..
Can you please abstract "just those changes" to a new PR..?

@DirtyF DirtyF added the themes 🎨 label Jan 29, 2018

@DirtyF DirtyF requested a review from jekyll/core Jan 29, 2018

@parkr

This comment has been minimized.

Member

parkr commented Jan 30, 2018

@benbalter I love all of this! Have a sec to fix this up?

@ashmaroli

This comment has been minimized.

Member

ashmaroli commented Feb 16, 2018

@parkr I've resolved the conflicts (albeit made minor changes), keeping @benbalter 's original concept intact.

@DirtyF

This comment has been minimized.

Member

DirtyF commented Feb 16, 2018

gem build is now checking the validity of the values in the gemspec:

WARNING:  See http://guides.rubygems.org/specification-reference/ for help
ERROR:  While executing gem ... (Gem::InvalidSpecificationException)
    "FIXME" or "TODO" is not a summary
    "Put your gem's website or public repo URL here." is not a valid HTTP URI
@ashmaroli

This comment has been minimized.

Member

ashmaroli commented Feb 17, 2018

@DirtyF Thank you for pushing the fix but IMHO, it should be handled in a separate PR since the error is reproducible on the master branch as well.

@ashmaroli ashmaroli referenced this pull request Feb 17, 2018

Closed

Release 3.8.0 #6783

12 of 13 tasks complete

@DirtyF DirtyF added this to the v3.8.0 milestone Feb 17, 2018

@DirtyF

This comment has been minimized.

Member

DirtyF commented Feb 18, 2018

@ashmaroli fix in #6784 has been merged on master

@DirtyF

DirtyF approved these changes Feb 18, 2018

"in #{document.relative_path} does not exist."
)
elsif !layout.nil?
layout_source = layout.path.start_with?(site.source) ? :site : :theme

This comment has been minimized.

@parkr

parkr Feb 20, 2018

Member

What do you think about adding this method to the Layout class, so you could ask the layout where it's from? 😄

This comment has been minimized.

@ashmaroli

ashmaroli Feb 20, 2018

Member

I was planning to open a PR after this merged to do just what you asked.. (but very different from what's here), I was aiming to make most minimal changes to what @benbalter had originally authored..

@parkr

parkr approved these changes Feb 20, 2018

@DirtyF

This comment has been minimized.

Member

DirtyF commented Mar 5, 2018

@jekyllbot: merge +dev

@jekyllbot jekyllbot merged commit ced613c into master Mar 5, 2018

3 checks passed

WIP ready for review
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jekyllbot jekyllbot deleted the theme-debug branch Mar 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment