-
-
Notifications
You must be signed in to change notification settings - Fork 9.9k
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
Multi-format layouts [WIP] #4742
Conversation
based on extension
i.e. if there is simple.html and simple.json, load them both into the simple layout, which is now an array.
So far, this just deals with the loading of multiple layouts into the site layouts hash. If multiple layouts are present, only the first one (order undefined) is used in rendering. |
layout = site.layouts[document.data["layout"]] | ||
# Load first layout from list, for now | ||
layouts = site.layouts[document.data["layout"]] | ||
layout = layouts ? layouts.first : nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd love use the Ruby 2.3 &.
operator, but we can't do that yet I guess because of support for older rubies. Need to check if activesupport is available for #try
, but I've not done that yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I don't have try
or &.
(forgot about that one) I tend to use &&
layout = layouts && layouts.first
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, good tip - I never quite internalised that construction...
@@ -43,7 +45,11 @@ def initialize(site, base, name) | |||
# | |||
# Returns nothing. | |||
def process(name) | |||
self.ext = File.extname(name) | |||
self.mime_type = MIME::Types.find{|t| ".#{t.preferred_extension}" == File.extname(name)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to make this reliable for cases where two MIME types have the same preferred extension. Does that happen? I bet it does somewhere...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, .html
vs .htm
, and so on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps an example: I've seen application/vnd.geo+json
in both .json
and .geojson
files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to use mime_type
?
Will this work just having a bunch of extensions?
(the question started out being rhetorical, but to be honest, I don't know)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Future proofing, really, and avoiding a partial solution when a better one exists. If mime_types
was a new dependency I'd think twice, but as it's already in the bundle, no harm in using it.
@@ -155,7 +157,7 @@ def place_in_layouts(content, payload, info) | |||
site.in_source_dir(layout.path) | |||
) if document.write? | |||
|
|||
if layout = site.layouts[layout.data["layout"]] | |||
if layout = site.layouts[layout.data["layout"]] ? site.layouts[layout.data["layout"]].first : nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having trouble reading this line.
Maybe pull it out before the if
block?
(but you don't need me to be able to read it - you need the person who is clicking merge to be able to read it)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good god, yes, that's horrible. What was I doing there?
I think mime-type is an added feature - keep it separate. This PR is quite simple. And that is a good thing. |
Sadly I didn't get very far with this. I might get chance to resurrect it, but I wouldn't hold your breath... so much else going on :( |
agreed. my jekyll blog died of bit rot. |
This is a work-in-progress experiment having a go at supporting multiple output formats (i.e. HTML & JSON) for a layout. See #3041 for discussion of the idea.
I'm exploring Jekyll as I go, so this might take a while to come together, but I thought I'd open early for discussion and see where we end up.
/cc @parkr @pikesley