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

fix(frontmatter): Support empty frontmatters #203

Merged
merged 3 commits into from
Apr 24, 2017

Conversation

badboy
Copy link
Contributor

@badboy badboy commented Mar 21, 2017

On trying to port my site over, I encoutered crashes due to invalid index access with files that have a front matter such as:

---
name: ...
---

This gets parsed as an empty front matter due to beginning with dashes, which lead to the index error.

I'm not sure if empty frontmatter = error is the right thing or if we just want to treat it without problems.

@epage
Copy link
Member

epage commented Apr 22, 2017

Sorry for not getting back to you on this.

Thank you for taking the time to address this.

I'm not sure if empty frontmatter = error is the right thing or if we just want to treat it without problems.

I'm leaning towards gracefully handling it because Cobalt can or "soon" will be able to handle all required attributes being defaulted.

Also, would you be willing to add a test case for this to prevent us from breaking this again in the future? If not, thats ok, I'll try to get to it.

For now, the easiest way to test this is adding to mod.rs

#[test]
pub fn empty_frontmatter() {
    run_test("empty_frontmatter").expect("Build error");
}

Put a cobalt project in tests/fixtures/empty_frontmatter that has an empty front matter. Put an expected output in tests/target/empty_frontmatter.

@epage
Copy link
Member

epage commented Apr 22, 2017

As for the CI failure, that was due to changes in Appveyor that have been fixed in master. I'd recommend pulling down the latest changes from master so your branch can build without problems.

@badboy
Copy link
Contributor Author

badboy commented Apr 23, 2017

Rebased against master, made it handle no frontmatter gracefully (simply not trying to extract any data from it) and added the test as advised.

@epage epage changed the title Return error on empty front matter fix(frontmatter): Support empty frontmatters Apr 24, 2017
@epage epage merged commit 5aa5813 into cobalt-org:master Apr 24, 2017
@epage
Copy link
Member

epage commented Apr 24, 2017

Thanks for contributing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants