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

Disable default layouts for documents with a `layout: none` declaration #5933

Merged
merged 2 commits into from May 15, 2017

Conversation

Projects
None yet
5 participants
@ashmaroli
Member

ashmaroli commented Mar 4, 2017

Inspired by discussion in #5929

While layout: null will still continue to be overridden by the defaults, with this pull in place, the user will be able to process and output a Jekyll::Document object without being placed into a layout by
simply declaring layout: none in the Front Matter

/cc @jekyll/ecosystem

Let me know if this results in any unintended behavior.. I'll update as required.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Mar 7, 2017

Member

This should be layout: null. I don't know if I like us fiddling with the way YAML works.

Member

parkr commented Mar 7, 2017

This should be layout: null. I don't know if I like us fiddling with the way YAML works.

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Mar 7, 2017

Member

This should be layout: null. I don't know if I like us fiddling with the way YAML works.

IMO, the proposed approach is better because it introduces a new route and only affects Jekyll::Document objects..
Modifying layout: null will require modification of #deep_merge_hashes which is used by various other objects

Member

ashmaroli commented Mar 7, 2017

This should be layout: null. I don't know if I like us fiddling with the way YAML works.

IMO, the proposed approach is better because it introduces a new route and only affects Jekyll::Document objects..
Modifying layout: null will require modification of #deep_merge_hashes which is used by various other objects

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Mar 31, 2017

Member

Ugh, I don't love this. If I had a _layouts/none.html file and layout: none today, then after this PR, I wouldn't have a layout on my page anymore. That seems like a breaking change.

A brief search shows that it's probably safe....

@benbalter, I think this could be an acceptable breaking change. Thoughts?

Member

parkr commented Mar 31, 2017

Ugh, I don't love this. If I had a _layouts/none.html file and layout: none today, then after this PR, I wouldn't have a layout on my page anymore. That seems like a breaking change.

A brief search shows that it's probably safe....

@benbalter, I think this could be an acceptable breaking change. Thoughts?

@parkr parkr requested a review from benbalter Mar 31, 2017

@parkr

parkr approved these changes Mar 31, 2017

@mattr-

This comment has been minimized.

Show comment
Hide comment
@mattr-

mattr- May 12, 2017

Member

IMHO, this is definitely a breaking change. I'm also not sure what the use case for this would be when we have the standard YAML version already in layout: null.

Member

mattr- commented May 12, 2017

IMHO, this is definitely a breaking change. I'm also not sure what the use case for this would be when we have the standard YAML version already in layout: null.

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks May 12, 2017

Member

we have the standard YAML version already in layout: null.

That's the problem: we don't. When we merge the defaults into a document’s front matter, we will override layout: null the same way we would if no layout were specified at all.

Member

pathawks commented May 12, 2017

we have the standard YAML version already in layout: null.

That's the problem: we don't. When we merge the defaults into a document’s front matter, we will override layout: null the same way we would if no layout were specified at all.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr May 15, 2017

Member

@jekyllbot: merge +minor

Member

parkr commented May 15, 2017

@jekyllbot: merge +minor

@jekyllbot jekyllbot merged commit 4d9c93e into jekyll:master May 15, 2017

2 checks passed

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

jekyllbot added a commit that referenced this pull request May 15, 2017

@ashmaroli ashmaroli deleted the ashmaroli:null-layout branch May 16, 2017

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