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

Properly render post content with Jekyll #73

Merged
merged 2 commits into from
Dec 13, 2015
Merged

Properly render post content with Jekyll #73

merged 2 commits into from
Dec 13, 2015

Conversation

pathawks
Copy link
Member

Taking a shot at rendering the sitemap with Jekyll instead of writing it ourself. This means that posts will be properly rendered, instead of having us render them with the Markdown filter and hoping for the best.

This will conflict with #67 in two ways:

My goal was just to make things work while changing as little as possible, so please let me know if you see anything goofy.

@pathawks
Copy link
Member Author

Travis failure is due to reliance on the strip filter, which is unavailable in Jekyll 2, but provided by the other pull request.

@pathawks
Copy link
Member Author

Since there are merge conflicts between the two PRs anyway, I just went ahead and added the strip filter to this one as well, so the test passes.

@parkr
Copy link
Member

parkr commented Dec 13, 2015

Thanks, @pathawks! Please rebase when you get a second; I merged your other PR.

@pathawks
Copy link
Member Author

Thanks @parkr!

Rebased 👍

feed = PageWithoutAFile.new(@site, File.dirname(__FILE__), "", path)
feed.content = File.read(source_path).gsub(/\s*\n\s*/, "\n").gsub(/\s+{%/, "{%").gsub(/\s+</,"<")
feed.data["layout"] = nil
feed.data['sitemap'] = false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind unifying the string literals? Probably go with double quotes as they're used above.

@parkr
Copy link
Member

parkr commented Dec 13, 2015

One style question, then good to merge. 👍

@pathawks
Copy link
Member Author

Thanks for catching that.

@psfrolov
Copy link

psfrolov commented Jan 1, 2016

v0.4.0 changelog lists this as fixed, but I still see all the liquid tags inside <content> node unexpanded as before (with jekyll 3.0.1).

@pathawks
Copy link
Member Author

pathawks commented Jan 1, 2016

@arkfps Are you using the latest version of the plugin? Is your site's source public?

@psfrolov
Copy link

psfrolov commented Jan 1, 2016

Using jekyll 3.0.1 + jekyll-feed 0.4.0. The source is local currently but here are my gemfile.lock and relevant portion of _config.yml.

GEM
  remote: https://rubygems.org/
  specs:
    addressable (2.3.8)
    colorator (0.1)
    faraday (0.9.2)
      multipart-post (>= 1.2, < 3)
    fastimage (1.8.1)
      addressable (~> 2.3, >= 2.3.5)
    ffi (1.9.10-x64-mingw32)
    jekyll (3.0.1)
      colorator (~> 0.1)
      jekyll-sass-converter (~> 1.0)
      jekyll-watch (~> 1.1)
      kramdown (~> 1.3)
      liquid (~> 3.0)
      mercenary (~> 0.3.3)
      rouge (~> 1.7)
      safe_yaml (~> 1.0)
    jekyll-feed (0.4.0)
    jekyll-gist (1.4.0)
      octokit (~> 4.2)
    jekyll-paginate (1.1.0)
    jekyll-sass-converter (1.4.0)
      sass (~> 3.4)
    jekyll-sitemap (0.9.0)
    jekyll-tagging (1.0.1)
      ruby-nuggets
    jekyll-watch (1.3.0)
      listen (~> 3.0)
    kramdown (1.9.0)
    liquid (3.0.6)
    liquid_pluralize (1.0.2)
      liquid (>= 2.6, < 4.0)
    listen (3.0.5)
      rb-fsevent (>= 0.9.3)
      rb-inotify (>= 0.9)
    mercenary (0.3.5)
    mini_portile2 (2.0.0)
    multipart-post (2.0.0)
    nokogiri (1.6.7.1-x64-mingw32)
      mini_portile2 (~> 2.0.0.rc2)
    nuggets (1.0.0)
    octokit (4.2.0)
      sawyer (~> 0.6.0, >= 0.5.3)
    rb-fsevent (0.9.7)
    rb-inotify (0.9.5)
      ffi (>= 0.5.0)
    rouge (1.10.1)
    ruby-nuggets (1.0.0)
      nuggets (= 1.0.0)
    safe_yaml (1.0.4)
    sass (3.4.20)
    sawyer (0.6.0)
      addressable (~> 2.3.5)
      faraday (~> 0.8, < 0.10)
    wdm (0.1.1)

PLATFORMS
  x64-mingw32

DEPENDENCIES
  fastimage
  jekyll
  jekyll-feed
  jekyll-gist
  jekyll-paginate
  jekyll-sitemap
  jekyll-tagging
  liquid_pluralize
  nokogiri
  wdm (~> 0.1.1)

BUNDLED WITH
   1.11.2
# Build settings.
source: app
destination: dist/manual-jekyll-build
include: [.nojekyll]  # disable Jekyll on GitHub Pages
kramdown:
  # GitHub Flavored Markdown.
  input: GFM
  hard_wrap: false
gems:
  - jekyll-gist
  - jekyll-paginate
  - jekyll/tagging
  - liquid_pluralize
  - jekyll-feed
  # Sitemap plugin must be last to cover other plugins's (possible) output.
  - jekyll-sitemap
permalink: pretty

# Default page layouts.
defaults:
  - scope:
      type: pages
    values:
      schema: "http://schema.org/WebPage"
      og_prefix: "website: http://ogp.me/ns/website#"
      og_type: website
  - scope:
      type: posts
    values:
      layout: post
      comments: true
      schema: "http://schema.org/BlogPosting"
      og_prefix: "article: http://ogp.me/ns/article#"
      og_type: article

# Pagination.
paginate: 5

# Web feed (jekyll-feed).
feed:
  path: atom.xml
  # Jekyll-feed also makes use of url, name, description and author variables.

# Tagging (jekyll-tagging).
tag_page_layout: tag-page
tag_page_dir: tag

# Custom variables.
url: https://arkfps.github.io
name: &sitename SITENAME
title: *sitename

@pathawks
Copy link
Member Author

pathawks commented Jan 1, 2016

This is not enough for me to reproduce your error.

Can you create a public repo with a bare-bones Jekyll site that exhibits this error? You should also probably open this as a new issue, since this PR has already been merged.

@psfrolov
Copy link

psfrolov commented Jan 1, 2016

I've found the cause: the `jekyll` gem must be explicitly mentioned in `_config.yml`: ``` gems: - jekyll # omitting this gem leaves liquid markup unexpanded in jekyll-feed output - jekyll-feed ``` Pretty subtle. It would be nice to make the requirement explicit and fail the build if not satisfied.

The real culprit is Jekyll 3.x incremental build feature. The first build works fine, while subsequent incremental builds are not.

@pathawks
Copy link
Member Author

pathawks commented Jan 1, 2016

That is... bizarre. Please do open a new issue; this needs a proper fix.

@psfrolov
Copy link

psfrolov commented Jan 1, 2016

Done. #78

@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants