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

Add multi-format frontmatter parser #348

Merged
merged 3 commits into from
Apr 11, 2017

Conversation

josephearl
Copy link
Contributor

@josephearl josephearl commented Apr 8, 2017

- Summary

Adds the ability to read front matter in TOML and JSON formats. Fixes #227.

Currently sortedKeys is not implemented when stringifying, neither are the custom types when parsing or stringifying since these were YAML specific. I can add these if it's a blocker.

- Test plan

Added tests for the new parser.

- Description for the changelog

Added dependency on preliminaries.
Added tests for basic parser cases expected to work in netlify-cms.

- A picture of a cute animal (not mandatory but encouraged)

A cute koala

@calavera
Copy link
Contributor

calavera commented Apr 9, 2017

I feel like the yaml-frontmatter.js and yaml-frontmatter-test.js files should be renamed to something else. Beside that, this change is fantastic, thank you!

@josephearl
Copy link
Contributor Author

Renamed to frontmatter.js and frontmatter.spec.js.

@erquhart
Copy link
Contributor

erquhart commented Apr 10, 2017

Hey @josephearl - awesome work on this! In considering preliminaries as a new dependency, I do wonder if it wouldn't be better to try getting those changes into gray-matter directly. Thoughts?

@josephearl
Copy link
Contributor Author

@erquhart There's a PR that's been open in that repository jonschlinkert/gray-matter#15 a while now to do some of the same things (separate out at the parsers), so although it would be good to get some of the changes merged upstream -- I'm not sure how quick a process that will be, if it happens.

@erquhart
Copy link
Contributor

Fair point, that repo does seem to be slow moving as of late.

Copy link
Contributor

@erquhart erquhart left a comment

Choose a reason for hiding this comment

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

LGTM

@erquhart
Copy link
Contributor

@josephearl last thought on this - might be good to at least raise an issue in gray-matter and give a heads up. You put in some serious work, and the community will likely benefit more from it if you can get the changes in on that repo. I'm guessing from the lack of movement there that the author could use some help.

@josephearl
Copy link
Contributor Author

jonschlinkert/gray-matter#38

@jonschlinkert
Copy link

I'm guessing from the lack of movement there that the author could use some help.

indeed! I would love some help on gray-matter! @josephearl you've done some great work, I added you as a collab on gray-matter. (sorry for the very late response!)

@jonschlinkert
Copy link

Hi @erquhart, I just pushed up 3.0 of gray-matter, which adds support for excerpts, and simplifies adding parsers/stringifiers for other formats.

Fwiw, after reviewing preliminaries more, I don't see anything that it really added even to the previous version of gray-matter, it looks like the code was mostly just reorganized into sub-projects (and the gray-matter pr that "had been open a while" was an anomoly on gray-matter. There had been other issues and updates since that pr was made, so I'm not sure why it was singled out).

In any case, would you be open to a pr to use gray-matter 3.0 directly instead of a fork?

@erquhart
Copy link
Contributor

Hey @jonschlinkert - definitely, that was the hope from the beginning. Thank you!

@jonschlinkert
Copy link

great, I'll put a pr together as soon as I have a chance. thanks!

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.

4 participants