zola icon indicating copy to clipboard operation
zola copied to clipboard

Merge default data language data in config.toml

Open Keats opened this issue 3 years ago • 3 comments

Right now we can put the data in 2 different places for the default language in config.toml: the base file and language.DEFAULT_LANGUAGE. Those are not currently merged and zola will just take language.DEFAULT_LANGUAGE data if present, resulting in missing data and hard to diagnose error. See https://github.com/justint/papaya/blob/44be3a1b39a8bad5b33e55e8f6de8953d0ca2e20/config.toml for an example

Keats avatar Jul 11 '22 21:07 Keats

Hi @Keats and @peterkelm, just a heads up that I've since updated papaya to declare the taxonomies in both the global section and [languages.en] section of the config.toml, see: https://github.com/justint/papaya/commit/8566ba158d4c34dd01845bf57f0d37b44050a6d2

Is that the right way to go about declaring the taxonomies for multilingual sites?

justint avatar Sep 14 '22 04:09 justint

It's probably better to keep it at the root level for now since you can end up missing data otherwise.

This issue is a good one if someone wants a pretty simple issue to contribute.

Keats avatar Sep 14 '22 14:09 Keats

With v0.16.1 I get the error

...has taxonomy `categories:` which is not defined in config.toml

Version v0.15.3 compiles my site fine. Copying my categories definition under [languages.en]- as mentioned above - does not work for me.

getreu avatar Sep 19 '22 03:09 getreu

I'd be happy to give this a shot!

Regarding the desired behavior: if the same variable is specified at the base section and under [language.DEFAULT_LANGUAGE], do we want base to override the latter or the other way around?

vcrn avatar Nov 18 '22 21:11 vcrn

That's a good question. I think it should probably error since that's a mistake.

Keats avatar Nov 19 '22 20:11 Keats

Yeah, that sounds reasonable.

I think I have some idea of that might work. I haven't tried any fix yet, but I think this is do-able for me.

Could you put me as assignee on this issue?

vcrn avatar Nov 20 '22 10:11 vcrn

There are some fields of LanguageOptions that make it slightly less straight-forward to merge, unlike the ones of the types Option, bool or String.

  • The field search: Search contains a number of different fields of types Option, bool and String, and one Enum that has a default type. The field search has a default implementation.
  • The field translations: HashMap<String, String>. Empty by default.
  • The field taxonomies: Vector<TaxonomyConfig>, where TaxonomyConfig consists of fields only of types Option, String and bool. Empty by default.

Similar manners exist for handling these more complex types:

  • The simple one would be to raise an error when both fields are not empty, not equal to each other, or not equal to the default implementation. Otherwise merge.
  • The more rigorous one would be to in turn compare the contents of these fields and apply the same logic of merging them as when merging the fields of LanguageOptions.

I'm leaning toward the simple approach for all of these fields and stopping the user at that point, since they should probably specify their config.toml in another way. What do you think?

Also, I suspect that if we implement the functionality of Zola raising an error during merge conflict, it will break a substantial amount of multilingual sites where the user was not fully aware of this behavior when writing the config. What do you say about simply raising a warning at merge conflict to begin with, merging by giving the language section variables priority over the base (since that's how it's been working so far), and see how that works out?

vcrn avatar Nov 21 '22 07:11 vcrn

Thinking about it more, we should probably have a warning either way if the section for the default language is defined and some fields are defined at the root of the config.toml. And an error as you mention if it's filled in both but different.

What do you say about simply raising a warning at merge conflict to begin with, merging by giving the language section variables priority over the base (since that's how it's been working so far), and see how that works out?

We can do that yes. Although since Zola releases are not super frequent, I think it might enough to document this change in the changelog and keep the error.

Keats avatar Nov 22 '22 20:11 Keats

Yeah, I'll go ahead with the approach that you're suggesting. I've started writing the first test, so hopefully I'll have something up for review in the coming days.

vcrn avatar Nov 24 '22 06:11 vcrn