From 2f2ea42c091931fe4735e0ca7b37dc05cb8c044b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Fri, 10 Mar 2017 20:54:50 +0100 Subject: [PATCH] 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 | 6 +-- hugolib/page.go | 9 ++-- hugolib/page_collections.go | 13 ++++++ hugolib/shortcode.go | 83 +++++++++++++++++++++++++------------ hugolib/shortcode_test.go | 5 ++- hugolib/site.go | 23 +++++++++- 6 files changed, 102 insertions(+), 37 deletions(-) diff --git a/hugolib/hugo_sites.go b/hugolib/hugo_sites.go index 0e7aafe96..d0ad57525 100644 --- 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 5ee31c2a8..e23434a78 100644 --- 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 a6468125c..3bf313282 100644 --- 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 823989f7d..d165c778b 100644 --- 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 + s.contentShortCodes = renderShortcodes(s.shortcodes, p) - renderedShortcodes := renderShortcodes(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 fe57d7af1..0b429306e 100644 --- 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 007ad4246..4c8aac018 100644 --- 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)