search

Ajedi32 / metalsmith-matters

A Metalsmith plugin to read file metadata from YAML frontmatter
MIT License
8 stars 2 forks source link

Add option to namespace the metadata #7

Closed adamkiss closed 8 years ago

adamkiss commented 8 years ago

For one of my metalsmith installations, I'd like to have all files' metadata grouped under page.* (e.g. page.title).

This is a quick (and naive) implementation of this functionality to open communication about it. It passes tests, but it introduces first non-gray-matter option, so you might want me to implement it in different way (or add other tests, based on comments in test/index.js)

Ajedi32 commented 8 years ago

Seems reasonable. Just a couple things:

  1. I think perhaps a better name for this option would be namespace. Thoughts?
  2. This feature needs to be documented in the README.
adamkiss commented 8 years ago

To be honest, I haven't touched README yet, because I didn't think this was production ready; I wasn't sure if pulling what's implemented (using if option === 'implementedOption) and sending the rest to gray-matter is a good way.

Ajedi32 commented 8 years ago

Ah okay, I didn't realize this was still a WIP.

The general approach of pulling the metalsmith-matters-specific options out of the options argument and sending the rest to gray-matter seems reasonable to me. There are perhaps a few things that could be done to make the code look cleaner, but overall it looks pretty good.

Why? Did you have other ideas in mind for the API?

adamkiss commented 8 years ago

I just sometimes overthink things :)

adamkiss commented 8 years ago

Right, so I changed the option to namespace, added namespacing if only a string is passed as options, and update the readme.

If it's good for you, you can bump the version and release.

adamkiss commented 8 years ago

Go ahead and release this :)

Ajedi32 commented 8 years ago

There we go. Sorry it took me so long to get to this. I'll cut a new release now.