External metadata files #1091

Closed
wants to merge 51 commits into
from

Projects

None yet

5 participants

@sequitur

I implemented the feature I asked for, namely: The Convertible class now reads YAML metadata either from the convertible file itself, or from a separate file with the same filename but .metadata as an extension. The metadata in the file itself is merged with the external metadata, and takes precedence. This is a small incremental change that doesn't add any other features (Such as attaching metadata to files that aren't handled by the Convertible class).

Unfortunately, I can't get the test suite to pass on the unmodified Master branch, nor Cucumber to run completely, so I can't verify that it passes. I did verify that no new failures or errors are introduced (I.e., my branch fails as much on my machine as the unmodified master branch). I also verified that Jekyll still works as expected and the new feature works with an ad-hoc test (Since I can't use the cucumber test suite). Hope that's okay and you can verify that the commits did not introduce any problems.

sequitur added some commits May 12, 2013
@sequitur sequitur Convertible now reads an external metadata file (If available)
The metadata in the file itself is still read, and takes precedence.
37d3493
@sequitur sequitur Added Cucumber feature for external metadata. 6d7536f
@sequitur sequitur Made it so files with external metadata are treated as Pages
Also, .metadata files are now filtered out of the outputted site.
caeb57d
@sequitur

Oh, also: Files that have external metadata, but no internal metadata, are treated as Pages, not static files; and files with names ending in .metadata are filtered out of the finished site.

@zachgersh

This actually implements #1082

I am 👍 on this as it adds functionality for power users while still keeping the same behavior for a default user out of the box.

@mattr- @parkr should definitely review this.

@parkr parkr commented on the diff May 22, 2013
site/docs/frontmatter.md
@@ -25,6 +25,13 @@ then be available to you to access using Liquid tags both further down in the
file and also in any layouts or includes that the page or post in question
relies on.
+A file can also have an attendant .metadata file which contains its front matter
+separately; this is useful if you want to use Jekyll alongside some other
+system which can't read Jekyll's front matter format. The .metadata file is a
+pure YAML file without the triple dashes. Any metadata in the file itself takes
@parkr
parkr May 22, 2013 Jekyll member

In your code above, it appears as though you look for these three dashes. Is that correct?

@parkr
Member
parkr commented May 22, 2013

@mattr- I'm still kind of undecided on this. I like the extensibility, but am not sure about the feature yet. This is when it'd be amazing to have @mojombo's input. What do you think, Matt?

@sequitur sequitur and 1 other commented on an outdated diff May 22, 2013
lib/jekyll/site.rb
@@ -158,9 +158,9 @@ def read_directories(dir = '')
next if self.dest.sub(/\/$/, '') == f_abs
read_directories(f_rel)
else
- first3 = File.open(f_abs) { |fd| fd.read(3) }
- if first3 == "---"
- # file appears to have a YAML header so process it as a page
+ if (File.exists? "#{f_abs}.metadata") || (File.open(f_abs) { |fd| fd.read(3) } == '---')
@sequitur
sequitur May 22, 2013

If you mean this line, it looks for a file called foo.md.metadata, OR for three dashes at the head of foo.md.

I considered making it so it ignores opening/closing dashes on the .metadata file itself, but didn't think it was worth the complexity cost, so it may break files in unspecified ways depending on what the yaml parser does/thinks.

Edit: Just checked. a .metadata file with opening/closing --- will break, and the file won't be converted.

@parkr
parkr May 22, 2013 Jekyll member

The .metadata file should be valid YAML. I'm confused as to why we'd enforce it not to have the standard --- header as all YAML files do.

@sequitur
sequitur May 22, 2013

We don't, the YAML parser does. Here:

if File.exists? File.join(base, "#{name}.metadata")
  # Yay, found metadata file.
  metadata = File.read(File.join(base, "#{name}.metadata"))
  self.data = YAML.safe_load(metadata)
end

The file is run unmodified by the YAML parser in exactly the same way the extracted front matter would be. And yes, it does break if the external metadata has ---, I'm unsure as to the specifics of why.

sequitur added some commits May 22, 2013
@sequitur sequitur Refactor test in site.rb to be more readable db058e8
@sequitur sequitur Fix bug with metadata merging.
If a Convertible instance already had metadata, the metadata on the file
itself wasn't being merged properly, because Array.merge was used
instead of Array.merge!
3538032
@sequitur sequitur Merge branch 'external-metadata' of http://github.com/sequitur/jekyll
…into external-metadata

Conflicts:
	lib/jekyll/convertible.rb
	lib/jekyll/site.rb
d111ce5
@sequitur

I've rebased to the last couple days of commits to the main repository, to keep this tracking them. So now the Travis build is failing (quite spectacularly) the same way it is for that repo.

Also: I corrected a bug with how metadata was being read. That bug was mine (Oops) but it wasn't caught by the test suite. I recommend (Regardless of whether this pull request is merged) adding a test to check if the metadata on layout files is being read (Since that would have caught the bug).

sequitur added some commits May 12, 2013
@sequitur sequitur Convertible now reads an external metadata file (If available)
The metadata in the file itself is still read, and takes precedence.
fdc57c0
@sequitur sequitur Added Cucumber feature for external metadata. b7d5944
@sequitur sequitur Made it so files with external metadata are treated as Pages
Also, .metadata files are now filtered out of the outputted site.
5022a31
@sequitur sequitur Documentation now mentions external .metadata files 3617911
@sequitur sequitur (Hopefully) fixed site.read_directories
Should pass test suite on Travis now. Or at least fail less.
451d7a6
@sequitur sequitur Fixed typos in Cucumber features... 8251d52
@sequitur sequitur Convertible now reads an external metadata file (If available)
The metadata in the file itself is still read, and takes precedence.
8a6bb31
@sequitur sequitur Added Cucumber feature for external metadata. 41f793d
@sequitur sequitur Made it so files with external metadata are treated as Pages
Also, .metadata files are now filtered out of the outputted site.
bcb5197
@sequitur sequitur Fixed typos in Cucumber features... a50fd3c
@sequitur sequitur Refactor test in site.rb to be more readable f288180
@sequitur sequitur Fix bug with metadata merging.
If a Convertible instance already had metadata, the metadata on the file
itself wasn't being merged properly, because Array.merge was used
instead of Array.merge!
b0901a4
@sequitur sequitur Merge branch 'external-metadata' of http://github.com/sequitur/jekyll
…into external-metadata
f6c032b
@parkr
Member
parkr commented May 23, 2013

I thought about it more and I think this would be an incredibly helpful/useful/powerful feature to have built-in to Jekyll. As long as it has no security issues (we'd YAML.safe_load in the metadata file), I'd 👍 to include this feature! Sorry for the delay, @sequitur!

@zachgersh

Where is @sequitur I want to see this go in :)

@sequitur
sequitur commented Jun 3, 2013

I'm here, waiting for parkr to merge or ask for rebases/merges from Master as appropriate. Is there something I'm supposed to be doing and not noticing?

@mattr-
Member
mattr- commented Jun 4, 2013

This does need to be rebased on top of current master. There's some stuff in the history file and the gemspec that seems wonky. Thanks!

@parkr
Member
parkr commented Jun 4, 2013

I had a chat with @mojombo a week or so ago and he shared some of his ideals for Jekyll features. When a new feature is suggested for the core of Jekyll, he laid out several criteria:

  1. Is this something that can be (reasonably) accomplished with a plugin?
  2. Is this something that significantly enhances the value of GitHub Pages?
  3. Will this be widely used (essentially used enough to warrant maintaining it)?
  4. Is there a better solution to the problem this feature is trying to solve?

... and several others of less consequence here. If I were to answer these question for myself, I'd assign the following answers:

  1. Not really, no.
  2. Depends on the answer to 3.
  3. Not unless it's used as a curol for adding new converters.
  4. Yes. A system of specifying certain file extnames for explicit inclusion (e.g. .scss, .coffee, .md) would mean a cleaner filesystem (no need for foo.scss.metadata files) and a cleaner file (no need for the ---'s at the top). This explicit inclusion would default to [] so as not to modify current behaviour.

That's my impression: if we can say ok, jekyll, all .scss files should be read in as pages and processed accordingly.

I'll ask @mojombo to put in his two cents here as well.

@sequitur
sequitur commented Jun 4, 2013

If you specify that all files with a certain extension should be read as pages, where would Jekyll get metadata for those files? If the files still contain metadata at their head, this wouldn't solve the problem this patch was designed to solve. The metadata would have to live somewhere else - I'm happy to have it live anywhere, really. I made it this particular way because it's how Hakyll does it.

Edit: One use we've discussed for this is that it improves the integration of Jekyll with various tools that output or read .md files, including literary programming and automatic documentation tools like literate Haskell, POD and rdoc. This may make it easier to plug source documentation right into a Jekyll page, which I think would enhance the value of Github Pages.

sequitur added some commits May 12, 2013
@sequitur sequitur Convertible now reads an external metadata file (If available)
The metadata in the file itself is still read, and takes precedence.
a17dcc6
@sequitur sequitur Added Cucumber feature for external metadata. 82f9936
@sequitur sequitur Made it so files with external metadata are treated as Pages
Also, .metadata files are now filtered out of the outputted site.
163666d
@sequitur sequitur Documentation now mentions external .metadata files 27ce8ac
@sequitur sequitur (Hopefully) fixed site.read_directories
Should pass test suite on Travis now. Or at least fail less.
f5bc1b4
@sequitur sequitur Fixed typos in Cucumber features... 3882775
@sequitur sequitur Convertible now reads an external metadata file (If available)
The metadata in the file itself is still read, and takes precedence.
152aed3
@sequitur sequitur Added Cucumber feature for external metadata. 7ecc88d
@sequitur sequitur Made it so files with external metadata are treated as Pages
Also, .metadata files are now filtered out of the outputted site.
b9b96e1
@sequitur sequitur Fixed typos in Cucumber features... 758cde2
@sequitur sequitur Refactor test in site.rb to be more readable d9490e0
@sequitur sequitur Fix bug with metadata merging.
If a Convertible instance already had metadata, the metadata on the file
itself wasn't being merged properly, because Array.merge was used
instead of Array.merge!
6c9bd8b
@sequitur sequitur Added Cucumber feature for external metadata. 89a2a1e
@sequitur sequitur Convertible now reads an external metadata file (If available)
The metadata in the file itself is still read, and takes precedence.
22bdac3
@sequitur sequitur Fixed typos in Cucumber features... fe76705
@sequitur sequitur Added Cucumber feature for external metadata. 9b4276c
@sequitur sequitur Made it so files with external metadata are treated as Pages
Also, .metadata files are now filtered out of the outputted site.
c56c044
@sequitur sequitur Fixed typos in Cucumber features... a91f79a
@sequitur sequitur Merge branch 'external-metadata' of http://github.com/sequitur/jekyll
…into external-metadata
aa3f57f
@kelvinst kelvinst commented on the diff Oct 22, 2013
lib/jekyll/convertible.rb
@@ -30,9 +30,24 @@ def read_yaml(base, name)
begin
self.content = File.read(File.join(base, name))
+ # Look for a .metadata file nearby.
+
+ if File.exists? File.join(base, "#{name}.metadata")
@kelvinst
kelvinst Oct 22, 2013

Here you can extract the File.join(base, "#{name}.metadata") to a local variable and reuse it below.

@kelvinst

Soooo, @parkr. Just looking at this... Did you got some news about this?

@sequitur

Unfortunately, it's been five months of changes to the Jekyll codebase since the last rebase and right now I don't have time to rebase the changes and make sure everything checks out. Someone who currently has their head in the Jekyll code probably wouldn't take too long to pull the external-metadata branch from my fork and rebase it onto the current master branch from @parkr. Hopefully.

@parkr parkr closed this Mar 17, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment