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

FileSystemUnified data source: added configurable dirs #412

Merged
merged 1 commit into from Apr 3, 2014

Conversation

gpakosz
Copy link
Member

@gpakosz gpakosz commented Apr 1, 2014

I added configurable prefixes to FileSystemUnified data source:

data_sources:
  - type:           filesystem_unified
    items_prefix:   items
    layouts_prefix: layouts

@denisdefreyne
Copy link
Member

For consistency with Filesystem in nanoc 4.0, I’d change the names slightly:

  • content_dir for items
  • layouts_dir for layouts

I’d also like to have methods content_dir_name and layouts_dir_name that do @config.fetch(:content_dir, 'content') and @config.fetch(:layouts_dir, 'layouts'), respectively.

(The Filesystem data source in nanoc 4.0 is not that nice yet. It doesn’t have content_dir_name and layouts_dir_name, for instance. I also noticed it uses %w( content layouts ) which is not parameterised, oops.)

# prefix: assets
# data_sources:
# - type: static
# prefix: assets
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for spotting this :)

@gpakosz
Copy link
Member Author

gpakosz commented Apr 2, 2014

I'm about to replace this pull request with a commit that does what you asked for.

But some tests fail because @config returns nil. Should it be @config.fetch(:content_dir, 'content') rescue 'content' and @config.fetch(:layouts_dir, 'layouts') rescue 'layouts'?

@denisdefreyne
Copy link
Member

Hmm, @config should ideally never be nil… can you fix the code so that it is never nil?

Alternative, you could have def config ; @config || [] ; end.

@gpakosz
Copy link
Member Author

gpakosz commented Apr 2, 2014

I started modifying the tests so that @config isn't nil. Which is kinda convoluted because lots of tests just assume it's ok to pass nil for config.

Somehow, it seems to me it's quite acceptable to change https://github.com/nanoc/nanoc/blob/master/lib/nanoc/base/source_data/data_source.rb#L61 to@config = config || {}.

What do you think?

PS: @config or config? FilesystemUnified uses the former while Static (which is more recent) uses the later.

@denisdefreyne
Copy link
Member

I’m OK with @config = config || {}.

I’d like to only use @config in the initializer, and config elsewhere in the file.

@gpakosz
Copy link
Member Author

gpakosz commented Apr 2, 2014

@gpakosz gpakosz changed the title FileSystemUnified data source: added configurable prefixes FileSystemUnified data source: added configurable dirs Apr 2, 2014
…layouts

    content_dir for items
    layouts_dir for layouts
@denisdefreyne
Copy link
Member

Looks good, thanks!

denisdefreyne added a commit that referenced this pull request Apr 3, 2014
FileSystemUnified data source: added configurable dirs
@denisdefreyne denisdefreyne merged commit 3ea38fe into nanoc:master Apr 3, 2014
@gpakosz gpakosz deleted the filesystem_unified-prefixes branch July 15, 2014 14:17
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