Refactor layout resolve to a descriptor/adapter pattern

diff --git a/hugolib/page.go b/hugolib/page.go
index 99e3309b96ce437743133a656c985cff1897c6a5..e0b97a27188e865dc99cbdd20e19c52751e7e074 100644 (file)
--- a/hugolib/page.go
+++ b/hugolib/page.go
@@ -204,42 +204,24 @@ type Page struct {
        // This is the PageOutput that represents the first item in outputFormats.
        // Use with care, as there are potential for inifinite loops.
        mainPageOutput *PageOutput
-
-       // Used to pick the correct template(s)
-       layoutIdentifier pageLayoutIdentifier
-}
-
-// Implements layout.LayoutIdentifier
-type pageLayoutIdentifier struct {
-       *Page
 }
 
-// PageKind returns the page's kind.
-func (p pageLayoutIdentifier) PageKind() string {
-       return p.Kind
-}
-
-// PageLayout returns the page's layout, if set.
-func (p pageLayoutIdentifier) PageLayout() string {
-       return p.Layout
-}
+func (p *Page) createLayoutDescriptor() output.LayoutDescriptor {
+       var section string
 
-// PageType returns the page's type, if set.
-func (p pageLayoutIdentifier) PageType() string {
-       return p.Type()
-}
-
-// PageType returns the page's section in layout terms.
-// This will be empty for regular pages, the section for section pages,
-// and the singular term for taxonomy and taxonomy terms pages.
-func (p pageLayoutIdentifier) PageSection() string {
        switch p.Kind {
        case KindSection:
-               return p.sections[0]
+               section = p.sections[0]
        case KindTaxonomy, KindTaxonomyTerm:
-               return p.s.taxonomiesPluralSingular[p.sections[0]]
+               section = p.s.taxonomiesPluralSingular[p.sections[0]]
        default:
-               return ""
+       }
+
+       return output.LayoutDescriptor{
+               Kind:    p.Kind,
+               Type:    p.Type(),
+               Layout:  p.Layout,
+               Section: section,
        }
 }
 
@@ -642,7 +624,6 @@ func (s *Site) newPage(filename string) *Page {
                Site:         &s.Info,
                s:            s,
        }
-       p.layoutIdentifier = pageLayoutIdentifier{p}
 
        s.Log.DEBUG.Println("Reading from", p.File.Path())
        return p
@@ -682,7 +663,10 @@ func (p *Page) layouts(layouts ...string) []string {
                layoutOverride = layouts[0]
        }
 
-       return p.s.layoutHandler.For(p.layoutIdentifier, layoutOverride, output.HTMLType)
+       return p.s.layoutHandler.For(
+               p.createLayoutDescriptor(),
+               layoutOverride,
+               output.HTMLType)
 }
 
 // TODO(bep) consolidate and test these KindHome switches (see other layouts methods)s

diff --git a/hugolib/page_test.go b/hugolib/page_test.go
index 6a920f5889dc85b76d4ecd863939b004e2d67c95..ac615ab89a7abb958f3ed53ce73c11b065132fbc 100644 (file)
--- a/hugolib/page_test.go
+++ b/hugolib/page_test.go
@@ -1361,7 +1361,6 @@ func TestIndexPageSimpleMethods(t *testing.T) {
                {func(n *Page) bool { return n.Scratch() != nil }},
                {func(n *Page) bool { return n.Hugo() != nil }},
                {func(n *Page) bool { return n.Now().Unix() == time.Now().Unix() }},
-               {func(n *Page) bool { return n.layoutIdentifier.Kind == KindHome }},
        } {
 
                n := s.newHomePage()

diff --git a/hugolib/site.go b/hugolib/site.go
index 2c5725b0ccc0ecea570c496977cbdf76e60c8c52..30375473aef1785e2017db01c641e8ea325c80d5 100644 (file)
--- a/hugolib/site.go
+++ b/hugolib/site.go
@@ -1661,7 +1661,7 @@ func (s *Site) kindFromSections(sections []string) string {
 }
 
 func (s *Site) layouts(p *PageOutput) []string {
-       return s.layoutHandler.For(p.layoutIdentifier, "", p.outputFormat)
+       return s.layoutHandler.For(p.createLayoutDescriptor(), "", p.outputFormat)
 }
 
 func (s *Site) preparePages() error {
@@ -2062,7 +2062,6 @@ func (s *Site) newNodePage(typ string) *Page {
                Site:     &s.Info,
                s:        s}
        p.outputFormats = p.s.defaultOutputDefinitions.ForKind(typ)
-       p.layoutIdentifier = pageLayoutIdentifier{p}
        return p
 
 }

diff --git a/output/layout.go b/output/layout.go
index d9f1c7a4b066f6201619ed24003f156d35e225ee..c33ce3f0cc2fdea0f6ebac03ad417f05a898625d 100644 (file)
--- a/output/layout.go
+++ b/output/layout.go
@@ -19,12 +19,13 @@ import (
        "strings"
 )
 
-// LayoutIdentifier is used to pick the correct layout for a piece of content.
-type LayoutIdentifier interface {
-       PageType() string
-       PageSection() string // TODO(bep) name
-       PageKind() string
-       PageLayout() string
+// LayoutDescriptor describes how a layout should be chosen. This is
+// typically built from a Page.
+type LayoutDescriptor struct {
+       Type    string
+       Section string
+       Kind    string
+       Layout  string
 }
 
 // Layout calculates the layout template to use to render a given output type.
@@ -60,27 +61,27 @@ indexes/indexes.NAME.SUFFIX indexes/indexes.SUFFIX
 `
 )
 
-func (l *LayoutHandler) For(id LayoutIdentifier, layoutOverride string, f Format) []string {
+func (l *LayoutHandler) For(d LayoutDescriptor, layoutOverride string, f Format) []string {
        var layouts []string
 
-       layout := id.PageLayout()
+       layout := d.Layout
 
        if layoutOverride != "" {
                layout = layoutOverride
        }
 
-       switch id.PageKind() {
+       switch d.Kind {
        // TODO(bep) move the Kind constants some common place.
        case "home":
-               layouts = resolveTemplate(layoutsHome, id, f)
+               layouts = resolveTemplate(layoutsHome, d, f)
        case "section":
-               layouts = resolveTemplate(layoutsSection, id, f)
+               layouts = resolveTemplate(layoutsSection, d, f)
        case "taxonomy":
-               layouts = resolveTemplate(layoutTaxonomy, id, f)
+               layouts = resolveTemplate(layoutTaxonomy, d, f)
        case "taxonomyTerm":
-               layouts = resolveTemplate(layoutTaxonomyTerm, id, f)
+               layouts = resolveTemplate(layoutTaxonomyTerm, d, f)
        case "page":
-               layouts = regularPageLayouts(id.PageType(), layout, f)
+               layouts = regularPageLayouts(d.Type, layout, f)
        }
 
        if l.hasTheme {
@@ -112,11 +113,11 @@ func (l *LayoutHandler) For(id LayoutIdentifier, layoutOverride string, f Format
        return layouts
 }
 
-func resolveTemplate(templ string, id LayoutIdentifier, f Format) []string {
+func resolveTemplate(templ string, d LayoutDescriptor, f Format) []string {
        return strings.Fields(replaceKeyValues(templ,
                "SUFFIX", f.MediaType.Suffix,
                "NAME", strings.ToLower(f.Name),
-               "SECTION", id.PageSection()))
+               "SECTION", d.Section))
 }
 
 func replaceKeyValues(s string, oldNew ...string) string {

diff --git a/output/layout_test.go b/output/layout_test.go
index 8b71bbe3bb55bf2feae7b6d90ec369aa7a276c6a..e678197cae797aea80a3ec8f0e13272a25faf9b2 100644 (file)
--- a/output/layout_test.go
+++ b/output/layout_test.go
@@ -21,29 +21,6 @@ import (
        "github.com/stretchr/testify/require"
 )
 
-type testLayoutIdentifier struct {
-       pageKind    string
-       pageSection string
-       pageLayout  string
-       pageType    string
-}
-
-func (l testLayoutIdentifier) PageKind() string {
-       return l.pageKind
-}
-
-func (l testLayoutIdentifier) PageLayout() string {
-       return l.pageLayout
-}
-
-func (l testLayoutIdentifier) PageType() string {
-       return l.pageType
-}
-
-func (l testLayoutIdentifier) PageSection() string {
-       return l.pageSection
-}
-
 var ampType = Format{
        Name:      "AMP",
        MediaType: media.HTMLType,
@@ -54,35 +31,35 @@ func TestLayout(t *testing.T) {
 
        for _, this := range []struct {
                name           string
-               li             testLayoutIdentifier
+               d              LayoutDescriptor
                hasTheme       bool
                layoutOverride string
                tp             Format
                expect         []string
        }{
-               {"Home", testLayoutIdentifier{"home", "", "", ""}, true, "", ampType,
+               {"Home", LayoutDescriptor{Kind: "home"}, true, "", ampType,
                        []string{"index.amp.html", "index.html", "_default/list.amp.html", "_default/list.html", "theme/index.amp.html", "theme/index.html"}},
-               {"Section", testLayoutIdentifier{"section", "sect1", "", ""}, false, "", ampType,
+               {"Section", LayoutDescriptor{Kind: "section", Section: "sect1"}, false, "", ampType,
                        []string{"section/sect1.amp.html", "section/sect1.html"}},
-               {"Taxonomy", testLayoutIdentifier{"taxonomy", "tag", "", ""}, false, "", ampType,
+               {"Taxonomy", LayoutDescriptor{Kind: "taxonomy", Section: "tag"}, false, "", ampType,
                        []string{"taxonomy/tag.amp.html", "taxonomy/tag.html"}},
-               {"Taxonomy term", testLayoutIdentifier{"taxonomyTerm", "categories", "", ""}, false, "", ampType,
+               {"Taxonomy term", LayoutDescriptor{Kind: "taxonomyTerm", Section: "categories"}, false, "", ampType,
                        []string{"taxonomy/categories.terms.amp.html", "taxonomy/categories.terms.html", "_default/terms.amp.html"}},
-               {"Page", testLayoutIdentifier{"page", "", "", ""}, true, "", ampType,
+               {"Page", LayoutDescriptor{Kind: "page"}, true, "", ampType,
                        []string{"_default/single.amp.html", "_default/single.html", "theme/_default/single.amp.html"}},
-               {"Page with layout", testLayoutIdentifier{"page", "", "mylayout", ""}, false, "", ampType,
+               {"Page with layout", LayoutDescriptor{Kind: "page", Layout: "mylayout"}, false, "", ampType,
                        []string{"_default/mylayout.amp.html", "_default/mylayout.html"}},
-               {"Page with layout and type", testLayoutIdentifier{"page", "", "mylayout", "myttype"}, false, "", ampType,
+               {"Page with layout and type", LayoutDescriptor{Kind: "page", Layout: "mylayout", Type: "myttype"}, false, "", ampType,
                        []string{"myttype/mylayout.amp.html", "myttype/mylayout.html", "_default/mylayout.amp.html"}},
-               {"Page with layout and type with subtype", testLayoutIdentifier{"page", "", "mylayout", "myttype/mysubtype"}, false, "", ampType,
+               {"Page with layout and type with subtype", LayoutDescriptor{Kind: "page", Layout: "mylayout", Type: "myttype/mysubtype"}, false, "", ampType,
                        []string{"myttype/mysubtype/mylayout.amp.html", "myttype/mysubtype/mylayout.html", "myttype/mylayout.amp.html"}},
-               {"Page with overridden layout", testLayoutIdentifier{"page", "", "mylayout", "myttype"}, false, "myotherlayout", ampType,
+               {"Page with overridden layout", LayoutDescriptor{Kind: "page", Layout: "mylayout", Type: "myttype"}, false, "myotherlayout", ampType,
                        []string{"myttype/myotherlayout.amp.html", "myttype/myotherlayout.html"}},
        } {
                t.Run(this.name, func(t *testing.T) {
                        l := NewLayoutHandler(this.hasTheme)
 
-                       layouts := l.For(this.li, this.layoutOverride, this.tp)
+                       layouts := l.For(this.d, this.layoutOverride, this.tp)
 
                        require.NotNil(t, layouts)
                        require.True(t, len(layouts) >= len(this.expect))