hugolib: Fix reloading corner cases for shortcodes

This commit fixes two different, but related issues:

1) Live-reload when a new shortcode was defined in the content file before the shortcode itself was created.
2) Live-reload when a newly defined shortcode changed its "inner content" status.

This commit also improves the shortcode related error messages to include the full path to the content file in question.

Fixes #3156

diff --git a/hugolib/hugo_sites.go b/hugolib/hugo_sites.go
index 0e7aafe96306e2d1c76ef09c52aed69f033ac4c9..d0ad57525663bccb1b4d452593d81a8367387b05 100644 (file)
--- a/hugolib/hugo_sites.go
+++ b/hugolib/hugo_sites.go
@@ -571,9 +571,9 @@ func (h *HugoSites) Pages() Pages {
 }
 
 func handleShortcodes(p *Page, rawContentCopy []byte) ([]byte, error) {
-       if len(p.contentShortCodes) > 0 {
-               p.s.Log.DEBUG.Printf("Replace %d shortcodes in %q", len(p.contentShortCodes), p.BaseFileName())
-               shortcodes, err := executeShortcodeFuncMap(p.contentShortCodes)
+       if p.shortcodeState != nil && len(p.shortcodeState.contentShortCodes) > 0 {
+               p.s.Log.DEBUG.Printf("Replace %d shortcodes in %q", len(p.shortcodeState.contentShortCodes), p.BaseFileName())
+               shortcodes, err := executeShortcodeFuncMap(p.shortcodeState.contentShortCodes)
 
                if err != nil {
                        return rawContentCopy, err

diff --git a/hugolib/page.go b/hugolib/page.go
index 5ee31c2a8592b2bf9b0f95927ccaf7d83e3bdbe4..e23434a78e99145852bcfa658fd57925b411e1a7 100644 (file)
--- a/hugolib/page.go
+++ b/hugolib/page.go
@@ -138,9 +138,7 @@ type Page struct {
        // whether the content is in a CJK language.
        isCJKLanguage bool
 
-       // shortcode state
-       contentShortCodes map[string]func() (string, error)
-       shortcodes        map[string]shortcode
+       shortcodeState *shortcodeHandler
 
        // the content stripped for HTML
        plain      string // TODO should be []byte
@@ -1484,9 +1482,10 @@ func (p *Page) SaveSource() error {
 }
 
 func (p *Page) ProcessShortcodes() {
-       tmpContent, tmpContentShortCodes, _ := extractAndRenderShortcodes(string(p.workContent), p)
+       p.shortcodeState = newShortcodeHandler()
+       tmpContent, _ := p.shortcodeState.extractAndRenderShortcodes(string(p.workContent), p)
        p.workContent = []byte(tmpContent)
-       p.contentShortCodes = tmpContentShortCodes
+
 }
 
 func (p *Page) FullFilePath() string {

diff --git a/hugolib/page_collections.go b/hugolib/page_collections.go
index a6468125c8976c64cab8238e8f243de5dbcaa70d..3bf3132824c183b4c22ae55dc604c782587aea12 100644 (file)
--- a/hugolib/page_collections.go
+++ b/hugolib/page_collections.go
@@ -120,6 +120,19 @@ func (c *PageCollections) removePage(page *Page) {
        }
 }
 
+func (c *PageCollections) findPagesByShortcode(shortcode string) Pages {
+       var pages Pages
+
+       for _, p := range c.rawAllPages {
+               if p.shortcodeState != nil {
+                       if _, ok := p.shortcodeState.nameSet[shortcode]; ok {
+                               pages = append(pages, p)
+                       }
+               }
+       }
+       return pages
+}
+
 func (c *PageCollections) replacePage(page *Page) {
        // will find existing page that matches filepath and remove it
        c.removePage(page)

diff --git a/hugolib/shortcode.go b/hugolib/shortcode.go
index 823989f7db1277696185c258dc15f36fa36a8e00..d165c778b223966645ddb56584a18a0124548b02 100644 (file)
--- a/hugolib/shortcode.go
+++ b/hugolib/shortcode.go
@@ -149,6 +149,26 @@ func (sc shortcode) String() string {
        return fmt.Sprintf("%s(%q, %t){%s}", sc.name, params, sc.doMarkup, sc.inner)
 }
 
+type shortcodeHandler struct {
+       // Maps the shortcodeplaceholder with the shortcode rendering func.
+       contentShortCodes map[string]func() (string, error)
+
+       // Maps the shortcodeplaceholder with the actual shortcode.
+       shortcodes map[string]shortcode
+
+       // All the shortcode names in this set.
+       nameSet map[string]bool
+}
+
+func newShortcodeHandler() *shortcodeHandler {
+       return &shortcodeHandler{
+               contentShortCodes: make(map[string]func() (string, error)),
+               shortcodes:        make(map[string]shortcode),
+               nameSet:           make(map[string]bool),
+       }
+}
+
+// TODO(bep) make it non-global
 var isInnerShortcodeCache = struct {
        sync.RWMutex
        m map[string]bool
@@ -177,6 +197,12 @@ func isInnerShortcode(t *template.Template) (bool, error) {
        return match, nil
 }
 
+func clearIsInnerShortcodeCache() {
+       isInnerShortcodeCache.Lock()
+       defer isInnerShortcodeCache.Unlock()
+       isInnerShortcodeCache.m = make(map[string]bool)
+}
+
 func createShortcodePlaceholder(id int) string {
        return fmt.Sprintf("HAHA%s-%dHBHB", shortcodePlaceholderPrefix, id)
 }
@@ -189,7 +215,7 @@ func renderShortcode(sc shortcode, parent *ShortcodeWithPage, p *Page) string {
        tmpl := getShortcodeTemplate(sc.name, p.s.Tmpl)
 
        if tmpl == nil {
-               p.s.Log.ERROR.Printf("Unable to locate template for shortcode '%s' in page %s", sc.name, p.BaseFileName())
+               p.s.Log.ERROR.Printf("Unable to locate template for shortcode '%s' in page %q", sc.name, p.Path())
                return ""
        }
 
@@ -207,8 +233,8 @@ func renderShortcode(sc shortcode, parent *ShortcodeWithPage, p *Page) string {
                        case shortcode:
                                inner += renderShortcode(innerData.(shortcode), data, p)
                        default:
-                               p.s.Log.ERROR.Printf("Illegal state on shortcode rendering of '%s' in page %s. Illegal type in inner data: %s ",
-                                       sc.name, p.BaseFileName(), reflect.TypeOf(innerData))
+                               p.s.Log.ERROR.Printf("Illegal state on shortcode rendering of %q in page %q. Illegal type in inner data: %s ",
+                                       sc.name, p.Path(), reflect.TypeOf(innerData))
                                return ""
                        }
                }
@@ -255,22 +281,17 @@ func renderShortcode(sc shortcode, parent *ShortcodeWithPage, p *Page) string {
        return renderShortcodeWithPage(tmpl, data)
 }
 
-func extractAndRenderShortcodes(stringToParse string, p *Page) (string, map[string]func() (string, error), error) {
-
-       content, shortcodes, err := extractShortcodes(stringToParse, p)
+func (s *shortcodeHandler) extractAndRenderShortcodes(stringToParse string, p *Page) (string, error) {
+       content, err := s.extractShortcodes(stringToParse, p)
 
        if err != nil {
                //  try to render what we have whilst logging the error
                p.s.Log.ERROR.Println(err.Error())
        }
 
-       // Save for reuse
-       // TODO(bep) refactor this
-       p.shortcodes = shortcodes
-
-       renderedShortcodes := renderShortcodes(shortcodes, p)
+       s.contentShortCodes = renderShortcodes(s.shortcodes, p)
 
-       return content, renderedShortcodes, err
+       return content, err
 
 }
 
@@ -311,7 +332,7 @@ var errShortCodeIllegalState = errors.New("Illegal shortcode state")
 // pageTokens state:
 // - before: positioned just before the shortcode start
 // - after: shortcode(s) consumed (plural when they are nested)
-func extractShortcode(pt *pageTokens, p *Page) (shortcode, error) {
+func (s *shortcodeHandler) extractShortcode(pt *pageTokens, p *Page) (shortcode, error) {
        sc := shortcode{}
        var isInner = false
 
@@ -332,7 +353,10 @@ Loop:
                        if cnt > 0 {
                                // nested shortcode; append it to inner content
                                pt.backup3(currItem, next)
-                               nested, err := extractShortcode(pt, p)
+                               nested, err := s.extractShortcode(pt, p)
+                               if nested.name != "" {
+                                       s.nameSet[nested.name] = true
+                               }
                                if err == nil {
                                        sc.inner = append(sc.inner, nested)
                                } else {
@@ -374,15 +398,16 @@ Loop:
                case tScName:
                        sc.name = currItem.val
                        tmpl := getShortcodeTemplate(sc.name, p.s.Tmpl)
-
+                       {
+                       }
                        if tmpl == nil {
-                               return sc, fmt.Errorf("Unable to locate template for shortcode '%s' in page %s", sc.name, p.BaseFileName())
+                               return sc, fmt.Errorf("Unable to locate template for shortcode %q in page %q", sc.name, p.Path())
                        }
 
                        var err error
                        isInner, err = isInnerShortcode(tmpl)
                        if err != nil {
-                               return sc, fmt.Errorf("Failed to handle template for shortcode '%s' for page '%s': %s", sc.name, p.BaseFileName(), err)
+                               return sc, fmt.Errorf("Failed to handle template for shortcode %q for page %q: %s", sc.name, p.Path(), err)
                        }
 
                case tScParam:
@@ -429,15 +454,13 @@ Loop:
        return sc, nil
 }
 
-func extractShortcodes(stringToParse string, p *Page) (string, map[string]shortcode, error) {
-
-       shortCodes := make(map[string]shortcode)
+func (s *shortcodeHandler) extractShortcodes(stringToParse string, p *Page) (string, error) {
 
        startIdx := strings.Index(stringToParse, "{{")
 
        // short cut for docs with no shortcodes
        if startIdx < 0 {
-               return stringToParse, shortCodes, nil
+               return stringToParse, nil
        }
 
        // the parser takes a string;
@@ -455,7 +478,6 @@ func extractShortcodes(stringToParse string, p *Page) (string, map[string]shortc
        // … it's safe to keep some "global" state
        var currItem item
        var currShortcode shortcode
-       var err error
 
 Loop:
        for {
@@ -467,8 +489,15 @@ Loop:
                case tLeftDelimScWithMarkup, tLeftDelimScNoMarkup:
                        // let extractShortcode handle left delim (will do so recursively)
                        pt.backup()
-                       if currShortcode, err = extractShortcode(pt, p); err != nil {
-                               return result.String(), shortCodes, err
+
+                       currShortcode, err := s.extractShortcode(pt, p)
+
+                       if currShortcode.name != "" {
+                               s.nameSet[currShortcode.name] = true
+                       }
+
+                       if err != nil {
+                               return result.String(), err
                        }
 
                        if currShortcode.params == nil {
@@ -477,7 +506,7 @@ Loop:
 
                        placeHolder := createShortcodePlaceholder(id)
                        result.WriteString(placeHolder)
-                       shortCodes[placeHolder] = currShortcode
+                       s.shortcodes[placeHolder] = currShortcode
                        id++
                case tEOF:
                        break Loop
@@ -485,11 +514,11 @@ Loop:
                        err := fmt.Errorf("%s:%d: %s",
                                p.FullFilePath(), (p.lineNumRawContentStart() + pt.lexer.lineNum() - 1), currItem)
                        currShortcode.err = err
-                       return result.String(), shortCodes, err
+                       return result.String(), err
                }
        }
 
-       return result.String(), shortCodes, nil
+       return result.String(), nil
 
 }
 

diff --git a/hugolib/shortcode_test.go b/hugolib/shortcode_test.go
index fe57d7af1be8f4572db851bc8ada3758a0aa0da1..0b429306edf122070f7a9408eda083658fffdd4f 100644 (file)
--- a/hugolib/shortcode_test.go
+++ b/hugolib/shortcode_test.go
@@ -352,7 +352,8 @@ func TestExtractShortcodes(t *testing.T) {
                        return nil
                })
 
-               content, shortCodes, err := extractShortcodes(this.input, p)
+               s := newShortcodeHandler()
+               content, err := s.extractShortcodes(this.input, p)
 
                if b, ok := this.expect.(bool); ok && !b {
                        if err == nil {
@@ -371,6 +372,8 @@ func TestExtractShortcodes(t *testing.T) {
                        }
                }
 
+               shortCodes := s.shortcodes
+
                var expected string
                av := reflect.ValueOf(this.expect)
                switch av.Kind() {

diff --git a/hugolib/site.go b/hugolib/site.go
index 007ad42460d8c492455a2ffb2a782baa237acac7..4c8aac01895c93bcd134163714154ee332657660 100644 (file)
--- a/hugolib/site.go
+++ b/hugolib/site.go
@@ -557,7 +557,7 @@ func (s *Site) reProcess(events []fsnotify.Event) (whatChanged, error) {
        tmplChanged := []fsnotify.Event{}
        dataChanged := []fsnotify.Event{}
        i18nChanged := []fsnotify.Event{}
-
+       shortcodesChanged := make(map[string]bool)
        // prevent spamming the log on changes
        logger := helpers.NewDistinctFeedbackLogger()
 
@@ -569,6 +569,13 @@ func (s *Site) reProcess(events []fsnotify.Event) (whatChanged, error) {
                if s.isLayoutDirEvent(ev) {
                        logger.Println("Template changed", ev.Name)
                        tmplChanged = append(tmplChanged, ev)
+
+                       if strings.Contains(ev.Name, "shortcodes") {
+                               clearIsInnerShortcodeCache()
+                               shortcode := filepath.Base(ev.Name)
+                               shortcode = strings.TrimSuffix(shortcode, filepath.Ext(shortcode))
+                               shortcodesChanged[shortcode] = true
+                       }
                }
                if s.isDataDirEvent(ev) {
                        logger.Println("Data changed", ev.Name)
@@ -681,6 +688,20 @@ func (s *Site) reProcess(events []fsnotify.Event) (whatChanged, error) {
 
        }
 
+       for shortcode, _ := range shortcodesChanged {
+               // There are certain scenarios that, when a shortcode changes,
+               // it isn't sufficient to just rerender the already parsed shortcode.
+               // One example is if the user adds a new shortcode to the content file first,
+               // and then creates the shortcode on the file system.
+               // To handle these scenarios, we must do a full reprocessing of the
+               // pages that keeps a reference to the changed shortcode.
+               pagesWithShortcode := s.findPagesByShortcode(shortcode)
+               for _, p := range pagesWithShortcode {
+                       p.rendered = false
+                       pageChan <- p
+               }
+       }
+
        // we close the filechan as we have sent everything we want to send to it.
        // this will tell the sourceReaders to stop iterating on that channel
        close(filechan)