hugolib, output: Restrict Render to regular Pages
authorBjørn Erik Pedersen <bjorn.erik.pedersen@gmail.com>
Sun, 26 Mar 2017 17:34:30 +0000 (19:34 +0200)
committerBjørn Erik Pedersen <bjorn.erik.pedersen@gmail.com>
Mon, 27 Mar 2017 13:43:56 +0000 (15:43 +0200)
Using it for list pages doesn't work and has potential weird side-effects.

The user probably meant to range over .Site.ReqularPages, and that is now marked clearly in the log.

hugolib/page.go
hugolib/page_output.go
hugolib/site.go
hugolib/site_render.go
output/layout.go
output/layout_test.go

index 84dfa2f430592ca6f525d555f6a5724fe2f69310..fa9f409223a877e6388bba93d873f49587cd8a80 100644 (file)
@@ -1721,3 +1721,11 @@ func (p *Page) setValuesForKind(s *Site) {
                p.URLPath.URL = "/" + path.Join(p.sections...) + "/"
        }
 }
+
+// Used in error logs.
+func (p *Page) pathOrTitle() string {
+       if p.Path() != "" {
+               return p.Path()
+       }
+       return p.Title
+}
index ed0678964a8adaabf72370915cbe8970f4095b78..837e415494c52293a0d29b58f37e78f92d720f6e 100644 (file)
@@ -21,6 +21,7 @@ import (
 
        "github.com/spf13/hugo/media"
 
+       "github.com/spf13/hugo/helpers"
        "github.com/spf13/hugo/output"
 )
 
@@ -85,9 +86,9 @@ func (p *PageOutput) copy() *PageOutput {
        return c
 }
 
-func (p *PageOutput) layouts(layouts ...string) []string {
+func (p *PageOutput) layouts(layouts ...string) ([]string, error) {
        if len(layouts) == 0 && p.selfLayout != "" {
-               return []string{p.selfLayout}
+               return []string{p.selfLayout}, nil
        }
 
        layoutOverride := ""
@@ -106,7 +107,11 @@ func (p *PageOutput) Render(layout ...string) template.HTML {
                return template.HTML("")
        }
 
-       l := p.layouts(layout...)
+       l, err := p.layouts(layout...)
+       if err != nil {
+               helpers.DistinctErrorLog.Printf("in .Render: Failed to resolve layout %q for page %q", layout, p.pathOrTitle())
+               return template.HTML("")
+       }
        return p.s.Tmpl.ExecuteTemplateToHTML(p, l...)
 }
 
@@ -122,7 +127,7 @@ func (p *Page) Render(layout ...string) template.HTML {
                pageOutput, err := newPageOutput(p, true, outFormat)
 
                if err != nil {
-                       p.s.Log.ERROR.Printf("Failed to create output page for type %q for page %q: %s", outFormat.Name, p, err)
+                       p.s.Log.ERROR.Printf("Failed to create output page for type %q for page %q: %s", outFormat.Name, p.pathOrTitle(), err)
                        return
                }
 
@@ -137,7 +142,7 @@ func (p *Page) Render(layout ...string) template.HTML {
 // for list pages.
 func (p *Page) checkRender() bool {
        if p.Kind != KindPage {
-               p.s.Log.ERROR.Printf(".Render only available for regular pages, not for %q of kind %q", p.Path(), p.Kind)
+               helpers.DistinctErrorLog.Printf(".Render only available for regular pages, not for of kind %q. You probably meant .Site.RegularPages and not.Site.Pages.", p.Kind)
                return false
        }
        return true
index 9d88d7605b2754a550151dc9cd118f62b693b6f3..4aa68aacffece403222e0471786d9396a22493d3 100644 (file)
@@ -1668,7 +1668,7 @@ func (s *Site) kindFromSections(sections []string) string {
        return KindSection
 }
 
-func (s *Site) layouts(p *PageOutput) []string {
+func (s *Site) layouts(p *PageOutput) ([]string, error) {
        return s.layoutHandler.For(p.layoutDescriptor, "", p.outputFormat)
 }
 
index 33060101328e191b1ac1dbe983f087517d8c7a26..700912e654f47b1b4af9b68eabf0bb2a05febb6e 100644 (file)
@@ -91,7 +91,11 @@ func pageRenderer(s *Site, pages <-chan *Page, results chan<- error, wg *sync.Wa
                        if page.selfLayout != "" {
                                layouts = []string{page.selfLayout}
                        } else {
-                               layouts = s.layouts(pageOutput)
+                               layouts, err = s.layouts(pageOutput)
+                               if err != nil {
+                                       s.Log.ERROR.Printf("Failed to resolve layout output %q for page %q: %s", outFormat.Name, page, err)
+                                       continue
+                               }
                        }
 
                        switch pageOutput.outputFormat.Name {
@@ -161,7 +165,11 @@ func (s *Site) renderPaginator(p *PageOutput) error {
                        pageNumber := i + 1
                        addend := fmt.Sprintf("/%s/%d", paginatePath, pageNumber)
                        targetPath, _ := p.targetPath(addend)
-                       layouts := p.layouts()
+                       layouts, err := p.layouts()
+
+                       if err != nil {
+                               return err
+                       }
 
                        if err := s.renderAndWritePage(
                                pagerNode.Title,
@@ -200,10 +208,13 @@ func (s *Site) renderRSS(p *PageOutput) error {
                p.Data["Pages"] = p.Pages
        }
 
-       layouts := s.layoutHandler.For(
+       layouts, err := s.layoutHandler.For(
                p.layoutDescriptor,
                "",
                p.outputFormat)
+       if err != nil {
+               return err
+       }
 
        // TODO(bep) output deprecate/handle rssURI
        targetPath, err := p.targetPath()
index cda6eb8a0681d74ebfac85a37684a18592f62812..a2bfd7717853941a4cc25b0a7499a243a8d97f70 100644 (file)
@@ -83,19 +83,23 @@ indexes/indexes.NAME.SUFFIX indexes/indexes.SUFFIX
 `
 )
 
-func (l *LayoutHandler) For(d LayoutDescriptor, layoutOverride string, f Format) []string {
+func (l *LayoutHandler) For(d LayoutDescriptor, layoutOverride string, f Format) ([]string, error) {
 
        // We will get lots of requests for the same layouts, so avoid recalculations.
        key := layoutCacheKey{d, layoutOverride, f}
        l.mu.RLock()
        if cacheVal, found := l.cache[key]; found {
                l.mu.RUnlock()
-               return cacheVal
+               return cacheVal, nil
        }
        l.mu.RUnlock()
 
        var layouts []string
 
+       if layoutOverride != "" && d.Kind != "page" {
+               return layouts, fmt.Errorf("Custom layout (%q) only supported for regular pages, not kind %q", layoutOverride, d.Kind)
+       }
+
        layout := d.Layout
 
        if layoutOverride != "" {
@@ -106,7 +110,7 @@ func (l *LayoutHandler) For(d LayoutDescriptor, layoutOverride string, f Format)
 
        if d.Kind == "page" {
                if isRSS {
-                       return []string{}
+                       return []string{}, nil
                }
                layouts = regularPageLayouts(d.Type, layout, f)
        } else {
@@ -148,14 +152,14 @@ func (l *LayoutHandler) For(d LayoutDescriptor, layoutOverride string, f Format)
                        }
                }
 
-               return layoutsWithThemeLayouts
+               return layoutsWithThemeLayouts, nil
        }
 
        l.mu.Lock()
        l.cache[key] = layouts
        l.mu.Unlock()
 
-       return layouts
+       return layouts, nil
 }
 
 func resolveListTemplate(d LayoutDescriptor, f Format,
index b38bd9c13cab0f562de32b1acc91d3d53906de1f..e59a16fcbb882bf4a60dccc32811218bd416db05 100644 (file)
@@ -68,8 +68,9 @@ func TestLayout(t *testing.T) {
                t.Run(this.name, func(t *testing.T) {
                        l := NewLayoutHandler(this.hasTheme)
 
-                       layouts := l.For(this.d, this.layoutOverride, this.tp)
+                       layouts, err := l.For(this.d, this.layoutOverride, this.tp)
 
+                       require.NoError(t, err)
                        require.NotNil(t, layouts)
                        require.True(t, len(layouts) >= len(this.expect))
                        // Not checking the complete list for now ...
@@ -83,6 +84,10 @@ func TestLayout(t *testing.T) {
                })
        }
 
+       l := NewLayoutHandler(false)
+       _, err := l.For(LayoutDescriptor{Kind: "taxonomyTerm", Section: "tag"}, "override", RSSFormat)
+       require.Error(t, err)
+
 }
 
 func BenchmarkLayout(b *testing.B) {
@@ -90,7 +95,8 @@ func BenchmarkLayout(b *testing.B) {
        l := NewLayoutHandler(false)
 
        for i := 0; i < b.N; i++ {
-               layouts := l.For(descriptor, "", HTMLFormat)
+               layouts, err := l.For(descriptor, "", HTMLFormat)
+               require.NoError(b, err)
                require.NotEmpty(b, layouts)
        }
 }