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

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 {
+	if p.shortcodeState != nil && len(p.shortcodeState.contentShortCodes) > 0 {
-		p.s.Log.DEBUG.Printf("Replace %d shortcodes in %q", len(p.contentShortCodes), p.BaseFileName())
+		p.s.Log.DEBUG.Printf("Replace %d shortcodes in %q", len(p.shortcodeState.contentShortCodes), p.BaseFileName())
-		shortcodes, err := executeShortcodeFuncMap(p.contentShortCodes)
+		shortcodes, err := executeShortcodeFuncMap(p.shortcodeState.contentShortCodes)
 
 		if err != nil {
 			return rawContentCopy, err

hugolib/page.go

@@ -138,9 +138,7 @@ type Page struct {
 	// whether the content is in a CJK language.
 	isCJKLanguage bool
 
-	// shortcode state
+	shortcodeState *shortcodeHandler
-	contentShortCodes map[string]func() (string, error)
-	shortcodes        map[string]shortcode
 
 	// 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 {

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)

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 ",
+				p.s.Log.ERROR.Printf("Illegal state on shortcode rendering of %q in page %q. Illegal type in inner data: %s ",
-					sc.name, p.BaseFileName(), reflect.TypeOf(innerData))
+					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) {
+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
+	s.contentShortCodes = renderShortcodes(s.shortcodes, p)
-	// TODO(bep) refactor this
-	p.shortcodes = shortcodes
 
-	renderedShortcodes := renderShortcodes(shortcodes, p)
+	return content, err
-
-	return content, renderedShortcodes, 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) {
+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 @@ 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
 
 }
 

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() {

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)