Refactor and cleanup

[Keats]

Zola has grown quite a bit since the last time it had a proper cleanup. It's time for another.

The focus should be on compilation speed and ease of testing. For compilation speed we can split into more sub-crates if needed as well as identify slow dependencies/issues, even if it's a bit annoying to repeat some deps in Cargo.toml...

For ease of testing, right now the only way to setup a proper test is to add content to the kitchen sink test_site because there's no easy way to setup content and test it. #1480 is an example issue that should help with that. Overall it also needs more tests as new releases often breaks things, it would be nice to know about them before releasing.

Also overall, it would be nice to know what's hard to understand about the codebase, where it's missing comments so we can improve on that.

[kogeletey]

Hello, maybe you should also separate out the ash components in a separate library, so that they can be conveniently used in other projects.
I also think it is worth highlighting the website separately, but this is about something else.

[Keats]

ash components in a separate library

What do you mean by that? I don't really see what parts of Zola could be used in other projects

[kogeletey]

For example, a component for processing markdown and parsing it can be used on the server, and markdown can be loaded from the database. Zola doesn't know how to do this but sometimes this behavior is necessary.

[Keats]

Ah, I forgot to mention that publishing any part of Zola as a library is out of scope. You can still use the crate you want in your app if needed via a submodule or similar but it's never going to be on crates.io

[Keats]

Ok first step done in #1747 which is getting most third parties imported from one sub crate instead of being repeated many times.

The main issue is that sub-crates are too interdependent which suggests their scope is too big. Rebuilding one subcrate should not rebuild 4 seemingly unrelated sub crates.

A few tasks:

• replace lazy_static by once_cell in the config crate. Accessing the syntect strucs was causing a malloc error when running tests, didn't spend time investigating
• replace chrono with time everywhere if possible (PR in progress)
• update pulldown_cmark: it seems a feature to specify the ID of a header has been added so we likely can remove our code (make sure it's well tested before changing)
• have a look at libs/Cargo.toml to see if there are libraries we might not need or features that can be disabled
• use insta for markdown rendering tests
• check if using anyhow would give us anything for error handling
• figure out how to make better integration test (running the binary directly so we can find some Windows issues?)
• Replace ws crate, it's unmaintained, maybe https://github.com/gbaranski/ezsockets ?

And then go through every sub crate one by one to see what can be split/improved/is confusing. Also avoiding 7-8 arguments functions would be great. The rewrites should be geared towards making it easy to test.
If anyone want to pick any of the issues listed above, please mention it so multiple people don't work on the same thing.

[Keats]

cc @mwcz if you want to pick up some of those early ones

[mwcz]

@Keats I took a shot at the once_cell task. I'm not seeing any malloc errors so I'm not sure what I did differently, or if it could be a machine/os specific thing. Here's my patch, would you like a PR against the reorg branch?

[mwcz]

I've got a branch with s/chrono/time mostly working. There are some tests failing but I hope to get it into shape soon.

[Keats]

Cool, let's see if the weird hacks around TOML datetimes are still needed (utils/src/de.rs)

[mwcz]

The failing tests revealed that my branch was not, in fact, "mostly working". 😅 I'm working through the date parsing problems as time permits.

So far I haven't found a way to make de.rs obsolete.

[mwcz]

I made some good progress tonight on s/chrono/time, and came away with a couple questions:

Question 1: should datetimes include timezone offset?

Zola predominantly uses chrono's NaiveDateTime, which don't include timezone offsets. The equivalent in the time crate is PrimitiveDateTime. Here, Zola converts from a DateTime (incl timezone) to NaiveDateTime (no timezone).

zola/components/front_matter/src/page.rs

Line 74 in 3dcc080

.map(|s| s.naive_local())

time doesn't seem to have such a terse way to go timezone-free, but you can do it by shifting to UTC and then plucking out the date and time components.

let dt = datetime!(2000-01-01 1:23:45 -5);
let dt_utc = dt.to_offset(offset!(UTC));
let prim = PrimitiveDateTime::new(dt_utc.date(), dt_utc.time());

assert_eq!(
    prim,
    PrimitiveDateTime::new(date!(2000 - 01 - 01), time!(6:23:45))
);

Anyway, is it worth doing this in place of naive_local(), or should we treat all datetimes as having a timezone offset, and assume UTC when the timezone is missing? I don't think that would be a breaking change since it's effectively what's already happening, as far as I can tell. In other words, should we just use OffsetDateTime everywhere, which includes timezone?

Question 2: how bad is adding time to front_matter's Cargo.toml?

For some reason, time's macros don't seem to work when it's pulled in through libs. They throw compile errors like this:

error[E0433]: failed to resolve: could not find `time` in the list of imported crates
  --> components/front_matter/src/page.rs:78:44
   |
78 |         .or_else(|_| match Date::parse(d, &format_description!("[year]-[month]-[day]")) {
   |                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ could not find `time` in the list of imported crates

But it works great when front_matter has a dep on time.

- use libs::time::macros::{format_description, time};
+ use time::macros::{format_description, time};

We don't have to use the macros, but it would be nice since they allow for compiling the format patterns at compile-time instead of run-time.

[Keats]

Question 1:
I believe naive_local adds the offset of the datetime and return a NaiveDateTime correct? I think your reasoning makes sense. Whatever is the simplest, as long as if an offset is provided we take it into account (I don't think many people are using offsets though)

Question 2:
It's fine, macros can't be re-exported so it's the one reason to list it directly in the sub crate

[mwcz]

Yeah, that's what naive_local seems to do. The docstring was confusing but I checked the source and it just adds the datetime + the offset. I do think it will be simplest to use OffsetDateTime everywhere, but I'll give it a try and see.

Ahhh, I didn't know macros can't be re-exported. Interesting. 🌠

[Keats]

I'm taking the pulldown_cmark update + rendering test changes.

Update: rendering test changes done, somehow a -1.5k LOC and a much more simple test setup
Update2: pulldown-cmark updated

[Keats]

Found on HN: https://diataxis.fr/

It would be a great way to rewrite the docs

[mwcz]

@Keats I'm ready to submit a PR for s/chrono/time. I had been rebasing it against the reorg branch which seems to be gone now. Would you like the PR against next?

[Keats]

Yep, the commits from reorg have been merged in next

[mwcz]

Cool, I'll get that in shortly. The rebase onto next looks pretty gnarly but I'll work through it.

[Keats]

It's probably easier to cherry pick your commits (or squash them before) onto next, otherwise you end up dealing with irrelevant merge issues.

[mwcz]

It's all good. My local next branch wasn't tracking yours, once I fixed that the rebase was a piece of cake, with the same outcome as a cherry pick. PR incoming!

[Keats]

Has anyone got experience with anyhow? Using it in Zola is a pretty big switch, so I'd like to get some feedback before doing it.

Another thing to do in this release: add #1573 and solve https://zola.discourse.group/t/remove-lighter-heavier-and-earlier-later/767

[mwcz]

I have no experience personally but I did hear this episode of Rustacean Station where Jane Lusby talks about error handling crates.

Here's a link straight to the part where she talks about failure, anyhow, and eyre.

https://pca.st/episode/e4d34171-e9cd-43ed-9819-fcbb2bcdcce2?t=949

The tldr seems to be:

1. Failure is deprecated
2. Anyhow is excellent
3. If you need more customizability than anyhow, use Eyre

[Keats]

#1816 for anyhow, it was faster than I thought and the resulting code is nicer.
Next up is probably some code reorg around the pages/sections