Shallower project structure

[dominicfreeston]

This (mostly) addresses issue #98. It will obviously require some related changes to the docs and the template (and maybe a quick migration guide for people who want to update), which I'm also happy to help with.

This leaves the output into resources/public/ because the ring server relies on this. I don't know if it can be changed.

Project Structure Changes:

• Remove resources/templates folder altogether
• Move posts and pages to content/ in root folder
• Move config.edn to content/ in root folder
• Move themes to root folder
• Move output folder public to root folder (specifiable via :public-dest)
• All user-specified resource paths are with respect to content/ folder

Other code changes:

• copy-resources now requires a root folder
• compile-sass->css! doesn't need a base-dir (it's just root folder) so just pass in whole config
• make more use of path helper function instead of str

The test coverage, although useful, is obviously not total so there's certainly a risk I've broken other things. I've ran this against all 4 included templates and everything seems to be working OK.

Thanks 😃

[yogthos]

The change looks good to me, and it should be possible to specify the public folder for the wrap-resource middleware as described here.

[dominicfreeston]

Ok so looking into it, we can make the ring server point to public instead of resource/public simply by changing resource-response to file-response and route/resources to route/files in server.clj within the Cryogen lein template.

The downside of this is that it will make migration marginally more difficult. Though if it's worth changing probably might as well break everything at once and have some migration instruction - the easiest way might just be "check out the latest template and port your content over".

[yogthos]

One idea could be to add an optional config variable for the path, so it would default to public, but you could explicitly set it to resources/public or something else entirely. This should help with the migration.

[dominicfreeston]

Yeah that certainly could be useful. A few things to think about first:

• currently the path is a constant, so we'd either need to pass the config around which means changing a bunch of functions, or we'd need to set it at the start of compile-assets which would make the code more stateful but much easier to change right now (I'm aware there's an outsanding issue aiming to make the code less stateful)
• ideally we should probably put some kinds of safe-guards in place (e.g not allow "." which would wipe the whole directory, and make sure it doesn't point to any of the the content, theme or resources paths), which would add a bit of complexity
• we'd also probably need to get the ring server to also read from the config, otherwise you'd have to go into the server code to make it match, which would somewhat defeat the point

I'm happy to look into these at some point if you think it's necessary, but it might take me a little while before I manage to get round to it. If you're happy without the safeguards and the more stateful approach then it should be really straightforward. I'm also happy to revert it back to resource/public if that's better, I'm personally much less bothered about the location of the generated content.

[yogthos]

I'd say specifying the path is mostly a nice to have, but I agree that having a guard would be good. I'd vote for merging things in without that, and creating an enhancement issue for later. Ideally, I think it would be best to pass the config around explicitly.

[lacarmen]

I do like the idea of being able to configure the output location. It would make GH pages deployment a lot simpler.

I'm okay with setting the path in compile-assets but I think we should at least have the safeguards that you mentioned.

Though if it's worth changing probably might as well break everything at once and have some migration instruction - the easiest way might just be "check out the latest template and port your content over".

I'm okay with the "break everything at once" approach as well :)

[dominicfreeston]

Ok thanks for all the feedback! Making GH deploy easier would be a nice win. Don’t mind leaving this open in case there’s need to fix something else before it’s finished, or merge into a dev branch, whatever works for you

[dominicfreeston]

OK there's now an option for setting paths (:public-dest). There's some breaking changes in there with moving things out into a separate namespace - let me know what you think.

I've gone for setting the path in the var for the sake of reducing the scope of the change - I did have a go at passing the config around but then it felt like I should be changing much more when going down that path. Probably best to leave as a separate change in the future?

I've added path-validation to ensure :public-dest, :resources and the main folders (content, themes, src, target) don't overlap, with some tests to check this. I split some things out in to a new cryogen-core.config namespace.

---

Some other thoughts:

• I was wondering if we should make the resources be with reference to the content folder rather than root, which would simplify a few things including what folders to watch for changes, and might be a bit more intuitive.
• It feels like the themes should specify their own resource paths with respect to their own folder rather than the root, but that would require changing how theme resources are handled (since they're currently just merged into the user config)

---

server.clj in the main cryogen repo would need to look something like this for things to work properly:

(ns cryogen.server
  (:require [compojure.core :refer [GET defroutes]]
            [compojure.route :as route]
            [ring.util.response :refer [redirect file-response]]
            [ring.util.codec :refer [url-decode]]
            [ring.middleware.file :refer [wrap-file]]
            [cryogen-core.watcher :refer [start-watcher!]]
            [cryogen-core.plugins :refer [load-plugins]]
            [cryogen-core.compiler :refer [compile-assets-timed]]
            [cryogen-core.config :refer [resolve-config]]
            [cryogen-core.io :refer [path]]))

(defn init []
  (load-plugins)
  (compile-assets-timed)
  (let [ignored-files (-> (resolve-config) :ignored-files)]
    (start-watcher! "content" ignored-files compile-assets-timed)))

(defn wrap-subdirectories
  [handler]
  (fn [request]
    (let [req-uri (.substring (url-decode (:uri request)) 1)
          res-path (if (or (= req-uri "") (= req-uri "/"))
                     (path "/index.html")
                     (path (str req-uri ".html")))]
      (or (file-response res-path {:root (:public-dest (resolve-config))})
          (handler request)))))

(defroutes routes
  (GET "/" [] (redirect (let [config (resolve-config)]
                          (path (:blog-prefix config)
                                (when-not (:clean-urls? config) "index.html")))))
  (route/files "/" {:root (:public-dest (resolve-config))})
  (route/not-found "Page not found"))

(def handler (wrap-subdirectories routes))

[lacarmen]

Thanks @dominicfreeston! I'll try to review/test soon. 😄

[lacarmen]

I did have a go at passing the config around but then it felt like I should be changing much more when going down that path. Probably best to leave as a separate change in the future?

I'm okay with this.

I was wondering if we should make the resources be with reference to the content folder rather than root, which would simplify a few things including what folders to watch for changes, and might be a bit more intuitive.

I'd prefer this. I find it's a bit weird having assets like css and img floating around the root folder. I think config.edn should be moved into the content folder as well, otherwise the watcher doesn't pick up on config changes (unless you have other ideas to address this).

compile-sass->css! doesn't need a base-dir (it's just root folder) so just pass in whole config

If we go with assets relative to content/ then the sass code will have to be updated a bit as well.

---

I tested with each of the templates as well and nothing seems to be broken :) I think we can merge this after the assets are addressed.

[dominicfreeston]

OK great, I'm away this weekend so not sure I'll get to it before next week - I think making the root content will require some changes to how theme assets are handled as well (since the path is specified from root at the moment)

[dominicfreeston]

@lacarmen apologies for the delay

Now changed so all resources paths are with reference to content/ - makes it much easier to ensure the output path doesn't overlap with anything else 👍

I've also updated it so that all paths in a theme are with reference to the theme folder (this will require updating the default lotus theme) - seems more consistent and predictable, what do you think?

As part of the changes I've modified how the config-specified resources for a theme are loaded, with the nice side-effect that they now get printed to the console as part of the "copying theme resources" phase. The main risk is that I no longer use deep-merge, I just load up the sass-src and resources, is there anything else we'd expect a theme to be able to specify that I might be losing by doing this?

[lacarmen]

Thanks @dominicfreeston! I'll try to get around to testing this week.

I've also updated it so that all paths in a theme are with reference to the theme folder (this will require updating the default lotus theme) - seems more consistent and predictable, what do you think?

Sounds good to me

The main risk is that I no longer use deep-merge, I just load up the sass-src and resources, is there anything else we'd expect a theme to be able to specify that I might be losing by doing this?

Not sure about this one off the top of my head - I'll investigate.

I'd like to release this after #119. I don't think there will be any conflicts in cryogen-core, but server.clj in the main repo might need to be updated. Do you mind taking a look at this? (I just merged #119 and cryogen-project/cryogen#181)

[dominicfreeston]

Just to update, I haven't forgotten about this, just been quite busy - should be able to get to it this weekend. Todo:

1. resolve conflict on this PR
2. update server.clj, project structure + README in main cryogen repo
3. update docs repo

Let me know if there's anything else that needs doing for this

[lacarmen]

No worries @dominicfreeston. I appreciate the work you've done on this!

I think that list covers everything. The tests could use some beefing up as you mentioned, but it's not critical for this PR. (Of course I'm not going to stop you though if you want to add to them 😄)

[dominicfreeston]

@lacarmen OK I've opened a couple of PRs against the other relevant repos, I think that's everything 😅.

The main thing missing is updating version numbers (I wasn't sure if you wanted to go up to 0.2.0 because of quite big breaking changes or if you wanted to just bump it as normal).

Thanks for your help and patience!

[lacarmen]

@dominicfreeston I tested the whole thing together and looks like it's all good! Thanks a bunch for taking this on 😄

I can bump up all the numbers - the core will go up to 0.2.0 and the template will go to 0.5.0.