Fix language menu config regression
authorBjørn Erik Pedersen <bjorn.erik.pedersen@gmail.com>
Tue, 22 Jun 2021 16:17:49 +0000 (18:17 +0200)
committerBjørn Erik Pedersen <bjorn.erik.pedersen@gmail.com>
Thu, 24 Jun 2021 11:03:09 +0000 (13:03 +0200)
Fixes #8672

config/defaultConfigProvider.go
hugolib/config.go
hugolib/config_test.go
hugolib/language_test.go

index fd32c08a69203035a123a809c4175391094c0322..a5e2d09fdd0d50b112c4684dddd04ae62e43fcf2 100644 (file)
@@ -197,6 +197,12 @@ func (c *defaultConfigProvider) Merge(k string, v interface{}) {
        defer c.mu.Unlock()
        k = strings.ToLower(k)
 
+       const (
+               languagesKey = "languages"
+               paramsKey    = "params"
+               menusKey     = "menus"
+       )
+
        if k == "" {
                rs, f := c.root.GetMergeStrategy()
                if f && rs == maps.ParamsMergeStrategyNone {
@@ -210,8 +216,44 @@ func (c *defaultConfigProvider) Merge(k string, v interface{}) {
                        // those as a special case.
                        for kk, vv := range p {
                                if pp, ok := vv.(maps.Params); ok {
-                                       if ppp, ok := c.root[kk]; ok {
-                                               ppp.(maps.Params).Merge(pp)
+                                       if pppi, ok := c.root[kk]; ok {
+                                               ppp := pppi.(maps.Params)
+                                               if kk == languagesKey {
+                                                       // Languages is currently a special case.
+                                                       // We may have languages with menus or params in the
+                                                       // right map that is not present in the left map.
+                                                       // With the default merge strategy those items will not
+                                                       // be passed over.
+                                                       var hasParams, hasMenus bool
+                                                       for _, rv := range pp {
+                                                               if lkp, ok := rv.(maps.Params); ok {
+                                                                       _, hasMenus = lkp[menusKey]
+                                                                       _, hasParams = lkp[paramsKey]
+                                                               }
+                                                       }
+
+                                                       if hasMenus || hasParams {
+                                                               for _, lv := range ppp {
+                                                                       if lkp, ok := lv.(maps.Params); ok {
+                                                                               if hasMenus {
+                                                                                       if _, ok := lkp[menusKey]; !ok {
+                                                                                               p := maps.Params{}
+                                                                                               p.SetDefaultMergeStrategy(maps.ParamsMergeStrategyShallow)
+                                                                                               lkp[menusKey] = p
+                                                                                       }
+                                                                               }
+                                                                               if hasParams {
+                                                                                       if _, ok := lkp[paramsKey]; !ok {
+                                                                                               p := maps.Params{}
+                                                                                               p.SetDefaultMergeStrategy(maps.ParamsMergeStrategyShallow)
+                                                                                               lkp[paramsKey] = p
+                                                                                       }
+                                                                               }
+                                                                       }
+                                                               }
+                                                       }
+                                               }
+                                               ppp.Merge(pp)
                                        } else {
                                                // We need to use the default merge strategy for
                                                // this key.
index 90ac7eb0172727f7eae585b7b6c6b87b9245d12d..a0ce9804252206a854a1ddef3b40acd28eab231f 100644 (file)
@@ -93,20 +93,6 @@ func LoadConfig(d ConfigSourceDescriptor, doWithConfig ...func(cfg config.Provid
                }
        }
 
-       // TODO(bep) improve this. This is currently needed to get the merge correctly.
-       if l.cfg.IsSet("languages") {
-               langs := l.cfg.GetParams("languages")
-               for _, lang := range langs {
-                       langp := lang.(maps.Params)
-                       if _, ok := langp["menus"]; !ok {
-                               langp["menus"] = make(maps.Params)
-                       }
-                       if _, ok := langp["params"]; !ok {
-                               langp["params"] = make(maps.Params)
-                       }
-               }
-
-       }
        l.cfg.SetDefaultMergeStrategy()
 
        // We create languages based on the settings, so we need to make sure that
index 65cb246b9c934339bf4d49ea57d1f17bdf9f492b..c5e77c94668a3adf99c7f63e6f780912ab62ebab 100644 (file)
@@ -76,7 +76,7 @@ func TestLoadMultiConfig(t *testing.T) {
        c.Assert(cfg.GetString("DontChange"), qt.Equals, "same")
 }
 
-func TestLoadConfigFromTheme(t *testing.T) {
+func TestLoadConfigFromThemes(t *testing.T) {
        t.Parallel()
 
        c := qt.New(t)
@@ -185,11 +185,15 @@ name = "menu-theme"
 
 `
 
-       buildForStrategy := func(t testing.TB, s string) *sitesBuilder {
-               mainConfig := strings.ReplaceAll(mainConfigTemplate, "MERGE_PARAMS", s)
+       buildForConfig := func(mainConfig, themeConfig string) *sitesBuilder {
                b := newTestSitesBuilder(t)
                b.WithConfigFile("toml", mainConfig).WithThemeConfigFile("toml", themeConfig)
-               return b.CreateSites().Build(BuildCfg{})
+               return b.Build(BuildCfg{})
+       }
+
+       buildForStrategy := func(t testing.TB, s string) *sitesBuilder {
+               mainConfig := strings.ReplaceAll(mainConfigTemplate, "MERGE_PARAMS", s)
+               return buildForConfig(mainConfig, themeConfig)
        }
 
        c.Run("Merge default", func(c *qt.C) {
@@ -316,6 +320,64 @@ name = "menu-theme"
                })
        })
 
+       c.Run("Merge no params in project", func(c *qt.C) {
+               b := buildForConfig(
+                       "baseURL=\"https://example.org\"\ntheme = \"test-theme\"\n",
+                       "[params]\np1 = \"p1 theme\"\n",
+               )
+
+               got := b.Cfg.Get("").(maps.Params)
+
+               b.Assert(got["params"], qt.DeepEquals, maps.Params{
+                       "p1": "p1 theme",
+               })
+       })
+
+       c.Run("Merge language no menus or params in project", func(c *qt.C) {
+               b := buildForConfig(
+                       `
+theme = "test-theme"
+baseURL = "https://example.com/"
+
+[languages]
+[languages.en]
+languageName = "English"
+
+`,
+                       `
+[languages]
+[languages.en]
+languageName = "EnglishTheme"
+
+[languages.en.params]
+p1="themep1"
+
+[[languages.en.menus.main]]
+name   = "menu-theme"
+`,
+               )
+
+               got := b.Cfg.Get("").(maps.Params)
+
+               b.Assert(got["languages"], qt.DeepEquals,
+                       maps.Params{
+                               "en": maps.Params{
+                                       "languagename": "English",
+                                       "menus": maps.Params{
+                                               "main": []map[string]interface{}{
+                                                       {
+                                                               "name": "menu-theme",
+                                                       },
+                                               },
+                                       },
+                                       "params": maps.Params{
+                                               "p1": "themep1",
+                                       },
+                               },
+                       },
+               )
+       })
+
 }
 
 func TestLoadConfigFromThemeDir(t *testing.T) {
index 16dcbcb030862f2e7ff09677edd6724827d21287..da8ecd22b1bc210ad359d7d41d0edf57490dffbb 100644 (file)
@@ -54,3 +54,28 @@ weight = 1
                b.AssertFileContent("public/index.html", "Hello: Hello")
        })
 }
+
+func TestLanguageBugs(t *testing.T) {
+       c := qt.New(t)
+
+       // Issue #8672
+       c.Run("Config with language, menu in root only", func(c *qt.C) {
+               b := newTestSitesBuilder(c)
+               b.WithConfigFile("toml", `
+theme = "test-theme"
+[[menus.foo]]
+name = "foo-a"
+[languages.en]
+
+`,
+               )
+
+               b.WithThemeConfigFile("toml", `[languages.en]`)
+
+               b.Build(BuildCfg{})
+
+               menus := b.H.Sites[0].Menus()
+               c.Assert(menus, qt.HasLen, 1)
+
+       })
+}