Fix nested data dir variable names

[westonganger]

Fixes #28

We also need to document that the variable names are set by the data dir name.

[westonganger]

Is this define_singleton_method a good pattern? Does it leave possibility for name clashes with existing methods. Or consider the following example?

register_data_dirs([
  'site/data',
  'other/data',
])

Another comment, I would almost prefer that all data is nested under the single data method but maybe thats not flexible enough.

[DevinCodes]

I think in the example you're providing, we would want to throw an error. IMO, data files should be:

• placed in a single directory, that you then register. (in your example, there's two ways to address this: register the site and other directory and access everything with site.data.x or other.data.x, or have a data directory with other and site directories in it. You would only register the data directory).
• Or, have a unique name so that there's no chance of collision.

Either way, the names of the data directory should not collide, and if it's the case I think we should raise an error so that the user can rename or restructure the data files.

What are your thoughts on this?

[westonganger]

Yes I think the error based approach for conflicts could work.

Heres another idea which makes the variable definition a lot more explicit and understandable.

register_data_dirs([
  site_data: 'site/data',
  other_data: 'other/data',
])

# Then access via
site_data.config.title
# OR
data(:site_data).config.title ### this format wont pollute the global namespace.

[DevinCodes]

I like the API you're suggesting, it is indeed clear and a lot more explicit.

However, it would introduce a breaking change for what I believe to be an optimization for a situation that shouldn't arise. In theory a site could have multiple data directories, in practice I don't think this will happen too often (e.g. Middleman only supports one data directory, which doesn't seem to generate too much complaints).

For now, I believe we should go with the error-based approach for conflicts. However, let's keep an eye on how people use this: if it turns out that the current working is quite problematic for our users, I have no issue to refactor this into something more like you proposed.

I would love to get @sarahdayan 's opinion on the topic, feel free to join the discussion!

[westonganger]

I agree the breaking change is not needed as multiple folders are an edge case. However I would like to be able to support both the original and the renaming approach in a backwards compatible way.

For example if my data folder is named yaml but I want to be able to use the data in my templates.

### Default Syntax
register_data_dirs([
  'app/yaml'
])

yaml.site.title

### Alternate Syntax
register_data_dirs({
  data: 'app/yaml',
})

data.site.title

def register_data_dirs(dirs)
  # etc

 if dirs.is_a?(Array)
   array = dirs
   dirs = {}
   array.each do |x|
     dirs[x] = x
   end
 end

 # etc
end

[DevinCodes]

If we can keep backwards compatibility, I'm all in favor of adding this! It's an awesome addition, thank you @westonganger 😄

[westonganger]

Ok I have now updated this PR.

[DevinCodes]

Updated the docs to reflect this change: https://github.com/algolia/frontman/wiki/Data-files

[DevinCodes]

We just released this in version 0.0.4, thank you for the contribution! 🎉