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

Define semantics for include directive #328

Closed
epage opened this issue Nov 8, 2017 · 2 comments
Closed

Define semantics for include directive #328

epage opened this issue Nov 8, 2017 · 2 comments
Assignees
Labels
breaking-change bug Not as expected
Milestone

Comments

@epage
Copy link
Member

epage commented Nov 8, 2017

@gnunicorn pointed this out on gitter.

Prior Art

Jekyll

Hugo

@epage epage added breaking-change bug Not as expected labels Nov 8, 2017
@epage epage self-assigned this Nov 8, 2017
@gnunicorn
Copy link
Contributor

gnunicorn commented Nov 8, 2017

May suggest renaming the Issue? Because this isn't about _layouts but _includes-path. I think our _layouts behaviour is fine as is. Just for includes it is annoying to prefix it every time from the root folder (and partials like the menu shouldn't be in _layouts IMHO).

@epage epage changed the title {%include _layouts/menu.liquid %} should not need indexing by _layouts Define semantics for include directive Nov 8, 2017
@epage
Copy link
Member Author

epage commented Nov 8, 2017

Ok, went and gave it a more general name

I haven't seen snippets being used in the wild in that kind of idea...

The idea is you have two different kind of partials

  • theme/layout partials for sharing design elements (e.g. headers) between layouts (posts, pages, etc)
  • content partials like menus that pages or layouts may include

My current thought:

  • _theme/includes for theme-defined layout partials
  • Treat _layouts as
    • A way to override a theme like Hugo
    • A less formal way of doing design without a theme
  • Therefore, _layouts/includes that occupies the same include "namespace" as _theme/includes
  • _includes folder for content (like menus) partials, rather than layout partials, in a potentially distinct namespace or directive
    • As you mention, there isn't much precedence for this but a reasonable use case (menus)
      • I'd consider skipping this for now but I'd at least like to have the namespace/directive question answered so I don't break compatibility if we add it later
    • Jekyll only has themes or _includes, so _include looks overloaded.
    • Hugo only seems to have layout-focused partials

This will break compatibility with the current lookup path. Initially, I will submit a lookup that will merge the source dir in with a "" namespace to keep compatibility. cobalt will issue deprecation warnings when these lookups occur. Then in my planned breaking release, I'll drop support for this.

But I think I am moving a bit away from your initial question. I'd keep layouts and includes separate, with layouts being flat and not included in the include-directive. and includes have a few, ordered templates-directories, but under that have a full hierarchy (this is also how many template system, like Django or Ember, do it afaik)

My proposal above is a halfway ground. We aren't mixing layouts and layout partials but they are kept near each other.

So include "namespace" or directive

IF the snippet and layout folder behave different (or have different hierarchy) I'd be in favor of reflecting that to the template users, by actually having an include and a snippet directive respectively (though internally they might be going to the same function just having a different search path).

If not, I wonder, why have to distinct ones in the first place. It's just semantics at this point, is it not? So why not just make the included_dir a list of _snippets, _includes, _theme/default (or something) on a first-match-win-basis. This would also easily allow people to replace any include for their local thing by putting their own custom-folder first and still have distinct reusable and sharable snippets, includes and themes ...

So you recommend _includes, _layouts/includes, and _theme/includes all occupy the root ("") namespace, meaning that include menu.liquid will look in each of those folders?

I like the simplicity of it. I'm also torn because my goal is to encourage a separation of layout and content, I like the idea of distinguishing _includes somehow.

Hmm, in thinking about this more, if _layouts is a less formal way (with assets like css being in the regular source), maybe it is fine for _layouts to not have its own includes directory.

@epage epage added this to the 0.9.0 milestone Nov 9, 2017
epage added a commit to epage/cobalt.rs that referenced this issue Nov 30, 2017
Previously, liquid `includes` were processed in the `source` dir.

Why change?
- Generally, snippets are kept in a central location, making this more
  cumbersome to access
- Jekyll and Hugo have a dedicated directory
- We want to support themes having includes and users overriding them,
  this means their needs to be a virtual file system of some sort
- Knowing what files are included allows us to cache off the data read
  from disk.  For now, this optimization has mixed results because
  `liquid` requires `LiquidOptions` to be cloned for every parse
  performed.

Ideally, we'd keep user snippets and layout snippets separate.  On disk,
that will happen once we add themes, so punting on it for now for
layouts (ad hoc themeing).  They are still within the same namespace in
the virtual filesystem though.  Punting on this for now.

This was hand tested to ensure deprecation warnings were issued and to
ensure retrofitted tests work.

Fixes cobalt-org#328

BREAKING CHANGE: Though we still load files from the old location (and
issue a deprecation warning), there is a very small chance that a file
already exists in the new location and we'll load that instead.
epage added a commit that referenced this issue Nov 30, 2017
Features

* **liquid:**  Dedicated _includes dir ([dc4b9ce](dc4b9ce), closes [#328](#328), breaks [#](https://github.com/cobalt-org/cobalt.rs/issues/))

Breaking Changes

* **liquid:**  Dedicated _includes dir ([dc4b9ce](dc4b9ce), closes [#328](#328))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change bug Not as expected
Projects
None yet
Development

No branches or pull requests

2 participants