shithub: hugo

Download patch

ref: 2f2ea42c091931fe4735e0ca7b37dc05cb8c044b
parent: 5f443bd45bd63c10796fc340985fddf218c7ef0d
author: Bjørn Erik Pedersen <bjorn.erik.pedersen@gmail.com>
date: Fri Mar 10 15:54:50 EST 2017 

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


--- a/hugolib/hugo_sites.go
+++ b/hugolib/hugo_sites.go
@@ -571,9 +571,9 @@
 }
 
 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
--- a/hugolib/page.go
+++ b/hugolib/page.go
@@ -138,9 +138,7 @@
 	// 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) 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 {
--- a/hugolib/page_collections.go
+++ b/hugolib/page_collections.go
@@ -120,6 +120,19 @@
 	}
 }
 
+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)
--- a/hugolib/shortcode.go
+++ b/hugolib/shortcode.go
@@ -149,6 +149,26 @@
 	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 @@
 	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 @@
 	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 @@
 			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,23 +281,18 @@
 	return renderShortcodeWithPage(tmpl, data)
 }
 
-func extractAndRenderShortcodes(stringToParse string, p *Page) (string, map[string]func() (string, error), error) {
+func (s *shortcodeHandler) extractAndRenderShortcodes(stringToParse string, p *Page) (string, error) {
+	content, err := s.extractShortcodes(stringToParse, p)
 
-	content, shortcodes, err := 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
+	s.contentShortCodes = renderShortcodes(s.shortcodes, p)
 
-	renderedShortcodes := renderShortcodes(shortcodes, p)
+	return content, err
 
-	return content, renderedShortcodes, err
-
 }
 
 var emptyShortcodeFn = func() (string, error) { return "", nil }
@@ -311,7 +332,7 @@
 // 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 @@
 			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 @@
 		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 @@
 	return sc, nil
 }
 
-func extractShortcodes(stringToParse string, p *Page) (string, map[string]shortcode, error) {
+func (s *shortcodeHandler) extractShortcodes(stringToParse string, p *Page) (string, error) {
 
-	shortCodes := make(map[string]shortcode)
-
 	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 @@
 	// … it's safe to keep some "global" state
 	var currItem item
 	var currShortcode shortcode
-	var err error
 
 Loop:
 	for {
@@ -467,10 +489,17 @@
 		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 {
 				currShortcode.params = make([]string, 0)
 			}
@@ -477,7 +506,7 @@
 
 			placeHolder := createShortcodePlaceholder(id)
 			result.WriteString(placeHolder)
-			shortCodes[placeHolder] = currShortcode
+			s.shortcodes[placeHolder] = currShortcode
 			id++
 		case tEOF:
 			break Loop
@@ -485,11 +514,11 @@
 			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
 
 }
 
--- a/hugolib/shortcode_test.go
+++ b/hugolib/shortcode_test.go
@@ -352,7 +352,8 @@
 			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 {
@@ -370,6 +371,8 @@
 				t.Fatalf("[%d] %s: failed: %q", i, this.name, err)
 			}
 		}
+
+		shortCodes := s.shortcodes
 
 		var expected string
 		av := reflect.ValueOf(this.expect)
--- a/hugolib/site.go
+++ b/hugolib/site.go
@@ -557,7 +557,7 @@
 	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 @@
 		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)
@@ -679,6 +686,20 @@
 			filechan <- file
 		}
 
+	}
+
+	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.
-- 
⑨