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

Limit Posts Doesn't Support Drafts #5898

Closed
5 of 17 tasks
jhulst opened this issue Feb 22, 2017 · 10 comments · May be fixed by #7298
Closed
5 of 17 tasks

Limit Posts Doesn't Support Drafts #5898

jhulst opened this issue Feb 22, 2017 · 10 comments · May be fixed by #7298
Labels
frozen-due-to-age has-pull-request Somebody suggested a solution to fix this issue

Comments

@jhulst
Copy link

jhulst commented Feb 22, 2017

  • 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 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

  • Run build on project that has greater than 10 posts in _posts and a single post in _drafts
  • Observe that the draft is built and shows in the blog listing page
  • Add limit_posts: 10 option to configuration
  • Run build
  • Only the posts from _posts are built, the draft is not processed

The Output I Wanted

The draft posts should be built and included in the site output.

@pathawks
Copy link
Member

So what is the expected behavior here? Should all drafts plus 10 non-drafts be built? Should 10 posts total be built, including up to 10 drafts?

@jhulst
Copy link
Author

jhulst commented Feb 22, 2017

I would suggest that the date ordering that is done for drafts should take precedent rather than only showing the published posts. So if the 10 drafts would be the first 10 that are generated, then show those.

As is, it seems that drafts are categorically excluded which was very confusing and is not the behavior that would be helpful for people who are using limit_posts to speed up development builds (since they are probably most interested in drafts over published posts).

@parkr
Copy link
Member

parkr commented Feb 27, 2017

I agree that if --show-drafts is passed (or drafts: true), then limit_posts should take drafts + posts and limit the combination of the two.

Would you please submit a PR for this? Search the codebase for limit_posts and you should find the relevant code that trims the array. Thanks!

@parkr parkr closed this as completed Feb 27, 2017
@parkr parkr added the bug label Feb 27, 2017
@tom-power
Copy link

Hi, when I pass --drafts and --limit-posts to build, drafts and posts are included? Added some tests here that are passing to show: master...tom-power:limit-posts-and-show-drafts

@epugh
Copy link

epugh commented Oct 4, 2018

I saw that @parkr closed this issue, however, I believe the issue is still a valid issue and is just waiting for a PR!

@pathawks
Copy link
Member

pathawks commented Oct 4, 2018

@epugh Yeah, if somebody submits a PR and mentions me, I would be happy to review 👍

@epugh
Copy link

epugh commented Oct 4, 2018

So, is it as simple (maybe??) as swapping lines 65 and 66?

site.posts.docs.concat(post_reader.read_posts(dir))

Put the drafts in the front, and then if your limit is 10, but you have 20 drafts, you get the first 10. Andif your limit is 10, and you have 5 drafts and 10 published blogs, then you get the first 5 drafts and the first 5 posts???!

@pathawks
Copy link
Member

pathawks commented Oct 4, 2018

@epugh That seems pretty straightforward. Care to open a PR where we can hash it out?

@epugh
Copy link

epugh commented Oct 4, 2018 via email

@jekyllbot jekyllbot added the has-pull-request Somebody suggested a solution to fix this issue label Oct 4, 2018
@epugh
Copy link

epugh commented Oct 4, 2018

@pathawks so it turns out this is fixed in the latest Jekyll, and I am on 3.7.3. I tested out the unit tests that @tom-power wrote, and they passed. So added a PR...

@jekyll jekyll locked and limited conversation to collaborators Oct 4, 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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants