Skip to content
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

Layouts with fully numeric names are not recognized #6190

Closed
6 of 18 tasks
denzel-morris opened this issue Jun 30, 2017 · 17 comments
Closed
6 of 18 tasks

Layouts with fully numeric names are not recognized #6190

denzel-morris opened this issue Jun 30, 2017 · 17 comments
Assignees
Labels
frozen-due-to-age has-pull-request Somebody suggested a solution to fix this issue pinned

Comments

@denzel-morris
Copy link

  • I believe this to be a bug, not a question about using Jekyll.
  • I updated to the latest Jekyll (or) if on GitHub Pages to the latest github-pages
  • I ran jekyll doctor to check my configuration
  • I read the CONTRIBUTION file at https://jekyllrb.com/docs/contributing/
  • This is a feature request.

  • I am on (or have tested on) macOS 10+
  • I am on (or have tested on) Debian/Ubuntu GNU/Linux
  • I am on (or have tested on) Fedora GNU/Linux
  • I am on (or have tested on) Arch GNU/Linux
  • I am on (or have tested on) Other GNU/Linux
  • I am on (or have tested on) Windows 10+

  • I was trying to install.
  • There is a broken Plugin API.
  • I had an error on GitHub Pages, and I have reproduced it locally.
  • I had an error on GitHub Pages, and GitHub Support said it was a Jekyll Bug.
  • I had an error on GitHub Pages and I did not test it locally.
  • I was trying to build.
  • It was another bug.

My Reproduction Steps

Create three files (one 404 file, and two layout files):

  • ./404.html
  • ./_layouts/404.html
  • ./_layouts/404a.html

Both layouts have the same content, they differ only in filename.

Firstly, the error is produced when ./404.html has the first layout specified:

---
layout: 404
---
# Hello, world!

jekyll build shows:

Build Warning: Layout '404' requested in 404.html does not exist.
                    done in 13.086 seconds.

When we change the layout to include one alphabetic character, there are no warnings:

---
layout: 404a
---
# Hello, world!

jekyll build shows no build warnings, and the layout is rendered as expected.

Gemfile

source 'https://rubygems.org'
gem 'github-pages', group: :jekyll_plugins
gem 'execjs'
gem 'therubyracer'
gem 'html-proofer'
gem 'jekyll'

/cc @jekyll/build

@mattr-
Copy link
Member

mattr- commented Jun 30, 2017

Does the site build correctly if with the 404 layout or is it incorrect?

@denzel-morris
Copy link
Author

I may be misunderstanding your question, but I believe you're asking whether the site builds correctly while ./_layouts/404.html is present.

Yes, the site builds correctly while ./_layouts/404.html is present. As long as the layout is not used by any page, the site builds correctly.

@pathawks
Copy link
Member

What if you change your YAML to:

---
layout: "404"
---

@denzel-morris
Copy link
Author

Yes, that fixed it, thanks @pathawks. Didn't expect it to be a type problem. Would we still consider this a bug or no?

I'm not sure why layout would be treated as anything other than a string, but I'm sure you have more context than I do.

@ashmaroli
Copy link
Member

Lets assume that you ignored the build warning for layout: 404.., did the page render properly?
If yes, then there's a bug in the layout detection otherwise we'll have to implement a type conversion so that the value of layout: is always a String..

@denzel-morris
Copy link
Author

@ashmaroli the page does not render properly with layout: 404 which is why I was asking about implementing the type conversion. Makes sense.

@ashmaroli
Copy link
Member

I'm not sure why layout would be treated as anything other than a string

Because, the Front Matter is just plain ol' YAML data, without any intermittent processing..

@ashmaroli
Copy link
Member

IMO, instead of silently converting, we should warn the user that layout: has been defined with an object other than String and the user should consider rectification..

@denzel-morris
Copy link
Author

Sure, I understand insofar as Jekyll has a number of predefined front matter properties that it uses for very specific purposes, e.g., to specify filenames.

I'd be more in favor of treating String and Numeric (or its YAML equiv) as valid values, and warning on other objects. It's a minor annoyance to be able to say layout: file but then layout: "404".

But, either resolution is fine with me. I appreciate you taking the time out to work on this. Thank you.

@pathawks
Copy link
Member

I'd be more in favor of treating String and Numeric (or its YAML equiv) as valid values, and warning on other objects. It's a minor annoyance to be able to say layout: file but then layout: "404".

Yup.

@mattr-
Copy link
Member

mattr- commented Jun 30, 2017

IMO, instead of silently converting, we should warn the user that layout: has been defined with an object other than String and the user should consider rectification..

Nah, we should just silently convert in this case because the user is specifying a file and shouldn't have to care about YAML's String vs. Number type conversions. If we can take care of that for a user and give them a better user experience, we should.

This is the fix I'll be implementing a later today.

@qaisjp
Copy link

qaisjp commented Aug 5, 2017

@mattr- was this implemented?

@mattr-
Copy link
Member

mattr- commented Aug 6, 2017

Nope. I never should have said "later today" because, of course, I ran out of time. I've made a note to myself to circle back around and do this. My ability to contribute to open source has been wildly inconsistent as of late.

@jekyllbot
Copy link
Contributor

This issue has been automatically marked as stale because it has not been commented on for at least two months.

The resources of the Jekyll team are limited, and so we are asking for your help.

If this is a bug and you can still reproduce this error on the 3.3-stable or master branch, please reply with all of the information you have about it in order to keep the issue open.

If this is a feature request, please consider building it first as a plugin. Jekyll 3 introduced hooks which provide convenient access points throughout the Jekyll build pipeline whereby most needs can be fulfilled. If this is something that cannot be built as a plugin, then please provide more information about why in order to keep this issue open.

This issue will automatically be closed in two months if no further activity occurs. Thank you for all your contributions.

@jekyllbot jekyllbot added the stale Nobody stepped up to work on this issue. label Oct 6, 2017
@qaisjp
Copy link

qaisjp commented Oct 6, 2017

@mattr-, how about now 😉

@jekyllbot jekyllbot removed the stale Nobody stepped up to work on this issue. label Oct 6, 2017
@mattr-
Copy link
Member

mattr- commented Oct 10, 2017

oh geez. two months already?
outrageous

@mattr- mattr- added the pinned label Oct 10, 2017
@pathawks pathawks added the has-pull-request Somebody suggested a solution to fix this issue label Oct 19, 2017
@pathawks
Copy link
Member

Closed via #6442

@jekyll jekyll locked and limited conversation to collaborators Jul 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age has-pull-request Somebody suggested a solution to fix this issue pinned
Projects
None yet
Development

No branches or pull requests

6 participants