From b3563b40a4a63a522a86e9a5a473820fb1a9b867 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Sat, 13 Aug 2016 00:33:17 +0200 Subject: [PATCH] Fix multilingual reload when shortcode changes This commit also refines the partial rebuild logic, to make sure we do not do more work than needed. Updates #2309 --- hugolib/handler_page.go | 10 +++++ hugolib/hugo_sites.go | 59 ++++++++++++++++++++++------- hugolib/hugo_sites_test.go | 34 +++++++++++++---- hugolib/page.go | 77 ++++++++++++++++++++------------------ hugolib/shortcode.go | 4 -- hugolib/site.go | 25 ++++++------- 6 files changed, 135 insertions(+), 74 deletions(-) diff --git a/hugolib/handler_page.go b/hugolib/handler_page.go index fcc5b9561..c1f83d716 100644 --- a/hugolib/handler_page.go +++ b/hugolib/handler_page.go @@ -15,6 +15,7 @@ package hugolib import ( "bytes" + "fmt" "github.com/spf13/hugo/helpers" "github.com/spf13/hugo/source" @@ -67,6 +68,10 @@ type htmlHandler struct { func (h htmlHandler) Extensions() []string { return []string{"html", "htm"} } func (h htmlHandler) PageConvert(p *Page, t tpl.Template) HandledResult { + if p.rendered { + panic(fmt.Sprintf("Page %q already rendered, does not need conversion", p.BaseFileName())) + } + p.ProcessShortcodes(t) return HandledResult{err: nil} @@ -100,6 +105,11 @@ func (h mmarkHandler) PageConvert(p *Page, t tpl.Template) HandledResult { } func commonConvert(p *Page, t tpl.Template) HandledResult { + + if p.rendered { + panic(fmt.Sprintf("Page %q already rendered, does not need conversion", p.BaseFileName())) + } + p.ProcessShortcodes(t) // TODO(bep) these page handlers need to be re-evaluated, as it is hard to diff --git a/hugolib/hugo_sites.go b/hugolib/hugo_sites.go index 8fe27e506..36b797e7e 100644 --- a/hugolib/hugo_sites.go +++ b/hugolib/hugo_sites.go @@ -220,7 +220,7 @@ func (h *HugoSites) Build(config BuildCfg) error { } } - if err := h.preRender(); err != nil { + if err := h.preRender(config, whatChanged{source: true, other: true}); err != nil { return err } @@ -261,6 +261,10 @@ func (h *HugoSites) Rebuild(config BuildCfg, events ...fsnotify.Event) error { return errors.New("Rebuild does not support 'ResetState'. Use Build.") } + if !config.Watching { + return errors.New("Rebuild called when not in watch mode") + } + h.runMode.Watching = config.Watching firstSite := h.Sites[0] @@ -270,7 +274,7 @@ func (h *HugoSites) Rebuild(config BuildCfg, events ...fsnotify.Event) error { s.resetBuildState() } - sourceChanged, err := firstSite.reBuild(events) + changed, err := firstSite.reBuild(events) if err != nil { return err @@ -279,7 +283,7 @@ func (h *HugoSites) Rebuild(config BuildCfg, events ...fsnotify.Event) error { // Assign pages to sites per translation. h.setupTranslations(firstSite) - if sourceChanged { + if changed.source { for _, s := range h.Sites { if err := s.postProcess(); err != nil { return err @@ -287,7 +291,7 @@ func (h *HugoSites) Rebuild(config BuildCfg, events ...fsnotify.Event) error { } } - if err := h.preRender(); err != nil { + if err := h.preRender(config, changed); err != nil { return err } @@ -391,7 +395,7 @@ func (h *HugoSites) setupTranslations(master *Site) { // preRender performs build tasks that need to be done as late as possible. // Shortcode handling is the main task in here. // TODO(bep) We need to look at the whole handler-chain construct with he below in mind. -func (h *HugoSites) preRender() error { +func (h *HugoSites) preRender(cfg BuildCfg, changed whatChanged) error { for _, s := range h.Sites { if err := s.setCurrentLanguageConfig(); err != nil { @@ -416,13 +420,13 @@ func (h *HugoSites) preRender() error { if err := s.setCurrentLanguageConfig(); err != nil { return err } - renderShortcodesForSite(s) + s.preparePagesForRender(cfg, changed) } return nil } -func renderShortcodesForSite(s *Site) { +func (s *Site) preparePagesForRender(cfg BuildCfg, changed whatChanged) { pageChan := make(chan *Page) wg := &sync.WaitGroup{} @@ -431,14 +435,37 @@ func renderShortcodesForSite(s *Site) { go func(pages <-chan *Page, wg *sync.WaitGroup) { defer wg.Done() for p := range pages { + + if !changed.other && p.rendered { + // No need to process it again. + continue + } + + // If we got this far it means that this is either a new Page pointer + // or a template or similar has changed so wee need to do a rerendering + // of the shortcodes etc. + + // Mark it as rendered + p.rendered = true + + // If in watch mode, we need to keep the original so we can + // repeat this process on rebuild. + if cfg.Watching { + p.rawContentCopy = make([]byte, len(p.rawContent)) + copy(p.rawContentCopy, p.rawContent) + } else { + // Just reuse the same slice. + p.rawContentCopy = p.rawContent + } + if err := handleShortcodes(p, s.owner.tmpl); err != nil { jww.ERROR.Printf("Failed to handle shortcodes for page %s: %s", p.BaseFileName(), err) } if p.Markup == "markdown" { - tmpContent, tmpTableOfContents := helpers.ExtractTOC(p.rawContent) + tmpContent, tmpTableOfContents := helpers.ExtractTOC(p.rawContentCopy) p.TableOfContents = helpers.BytesToHTML(tmpTableOfContents) - p.rawContent = tmpContent + p.rawContentCopy = tmpContent } if p.Markup != "html" { @@ -449,19 +476,25 @@ func renderShortcodesForSite(s *Site) { if err != nil { jww.ERROR.Printf("Failed to set use defined summary: %s", err) } else if summaryContent != nil { - p.rawContent = summaryContent.content + p.rawContentCopy = summaryContent.content } - p.Content = helpers.BytesToHTML(p.rawContent) - p.rendered = true + p.Content = helpers.BytesToHTML(p.rawContentCopy) if summaryContent == nil { p.setAutoSummary() } + + } else { + p.Content = helpers.BytesToHTML(p.rawContentCopy) } + // no need for this anymore + p.rawContentCopy = nil + //analyze for raw stats p.analyzePage() + } }(pageChan, wg) } @@ -490,7 +523,7 @@ func handleShortcodes(p *Page, t tpl.Template) error { return err } - p.rawContent, err = replaceShortcodeTokens(p.rawContent, shortcodePlaceholderPrefix, shortcodes) + p.rawContentCopy, err = replaceShortcodeTokens(p.rawContentCopy, shortcodePlaceholderPrefix, shortcodes) if err != nil { jww.FATAL.Printf("Failed to replace short code tokens in %s:\n%s", p.BaseFileName(), err.Error()) diff --git a/hugolib/hugo_sites_test.go b/hugolib/hugo_sites_test.go index bfbcd914e..b19e9ea2e 100644 --- a/hugolib/hugo_sites_test.go +++ b/hugolib/hugo_sites_test.go @@ -51,7 +51,7 @@ func doTestMultiSitesMainLangInRoot(t *testing.T, defaultInSubDir bool) { testCommonResetState() viper.Set("DefaultContentLanguageInSubdir", defaultInSubDir) - sites := createMultiTestSites(t, multiSiteTomlConfig) + sites := createMultiTestSites(t, multiSiteTOMLConfig) err := sites.Build(BuildCfg{}) @@ -166,7 +166,7 @@ func TestMultiSitesBuild(t *testing.T) { content string suffix string }{ - {multiSiteTomlConfig, "toml"}, + {multiSiteTOMLConfig, "toml"}, {multiSiteYAMLConfig, "yml"}, {multiSiteJSONConfig, "json"}, } { @@ -323,8 +323,8 @@ func doTestMultiSitesBuild(t *testing.T, configContent, configSuffix string) { func TestMultiSitesRebuild(t *testing.T) { testCommonResetState() - sites := createMultiTestSites(t, multiSiteTomlConfig) - cfg := BuildCfg{} + sites := createMultiTestSites(t, multiSiteTOMLConfig) + cfg := BuildCfg{Watching: true} err := sites.Build(cfg) @@ -350,6 +350,10 @@ func TestMultiSitesRebuild(t *testing.T) { docFr := readDestination(t, "public/fr/sect/doc1/index.html") assert.True(t, strings.Contains(docFr, "Bonjour"), "No Bonjour") + // check single page content + assertFileContent(t, "public/fr/sect/doc1/index.html", true, "Single", "Shortcode: Bonjour") + assertFileContent(t, "public/en/sect/doc1-slug/index.html", true, "Single", "Shortcode: Hello") + for i, this := range []struct { preFunc func(t *testing.T) events []fsnotify.Event @@ -474,6 +478,22 @@ func TestMultiSitesRebuild(t *testing.T) { }, }, + // Change a shortcode + { + func(t *testing.T) { + writeSource(t, "layouts/shortcodes/shortcode.html", "Modified Shortcode: {{ i18n \"hello\" }}") + }, + []fsnotify.Event{ + {Name: "layouts/shortcodes/shortcode.html", Op: fsnotify.Write}, + }, + func(t *testing.T) { + assert.Len(t, enSite.Pages, 4) + assert.Len(t, enSite.AllPages, 10) + assert.Len(t, frSite.Pages, 4) + assertFileContent(t, "public/fr/sect/doc1/index.html", true, "Single", "Modified Shortcode: Salut") + assertFileContent(t, "public/en/sect/doc1-slug/index.html", true, "Single", "Modified Shortcode: Hello") + }, + }, } { if this.preFunc != nil { @@ -516,7 +536,7 @@ func assertShouldNotBuild(t *testing.T, sites *HugoSites) { func TestAddNewLanguage(t *testing.T) { testCommonResetState() - sites := createMultiTestSites(t, multiSiteTomlConfig) + sites := createMultiTestSites(t, multiSiteTOMLConfig) cfg := BuildCfg{} err := sites.Build(cfg) @@ -525,7 +545,7 @@ func TestAddNewLanguage(t *testing.T) { t.Fatalf("Failed to build sites: %s", err) } - newConfig := multiSiteTomlConfig + ` + newConfig := multiSiteTOMLConfig + ` [Languages.sv] weight = 15 @@ -573,7 +593,7 @@ title = "Svenska" } -var multiSiteTomlConfig = ` +var multiSiteTOMLConfig = ` DefaultExtension = "html" baseurl = "http://example.com/blog" DisableSitemap = false diff --git a/hugolib/page.go b/hugolib/page.go index b09e2b1f7..7892f222d 100644 --- a/hugolib/page.go +++ b/hugolib/page.go @@ -48,28 +48,42 @@ var ( ) type Page struct { - Params map[string]interface{} - Content template.HTML - Summary template.HTML - Aliases []string - Status string - Images []Image - Videos []Video - TableOfContents template.HTML - Truncated bool - Draft bool - PublishDate time.Time - ExpiryDate time.Time - Markup string - translations Pages - extension string - contentType string - renderable bool - Layout string - layoutsCalculated []string - linkTitle string - frontmatter []byte - rawContent []byte + Params map[string]interface{} + Content template.HTML + Summary template.HTML + Aliases []string + Status string + Images []Image + Videos []Video + TableOfContents template.HTML + Truncated bool + Draft bool + PublishDate time.Time + ExpiryDate time.Time + Markup string + translations Pages + extension string + contentType string + renderable bool + Layout string + layoutsCalculated []string + linkTitle string + frontmatter []byte + + // rawContent isn't "raw" as in the same as in the content file. + // Hugo cares about memory consumption, so we make changes to it to do + // markdown rendering etc., but it is "raw enough" so we can do rebuilds + // when shortcode changes etc. + rawContent []byte + + // When running Hugo in watch mode, we do partial rebuilds and have to make + // a copy of the rawContent to be prepared for rebuilds when shortcodes etc. + // have changed. + rawContentCopy []byte + + // state telling if this is a "new page" or if we have rendered it previously. + rendered bool + contentShortCodes map[string]func() (string, error) shortcodes map[string]shortcode plain string // TODO should be []byte @@ -84,7 +98,6 @@ type Page struct { Source Position `json:"-"` Node - rendered bool } type Source struct { @@ -220,7 +233,7 @@ var ( // Returns the page as summary and main if a user defined split is provided. func (p *Page) setUserDefinedSummaryIfProvided() (*summaryContent, error) { - sc := splitUserDefinedSummaryAndContent(p.Markup, p.rawContent) + sc := splitUserDefinedSummaryAndContent(p.Markup, p.rawContentCopy) if sc == nil { // No divider found @@ -1024,19 +1037,9 @@ func (p *Page) SaveSource() error { } func (p *Page) ProcessShortcodes(t tpl.Template) { - - // these short codes aren't used until after Page render, - // but processed here to avoid coupling - // TODO(bep) Move this and remove p.contentShortCodes - if !p.rendered { - tmpContent, tmpContentShortCodes, _ := extractAndRenderShortcodes(string(p.rawContent), p, t) - p.rawContent = []byte(tmpContent) - p.contentShortCodes = tmpContentShortCodes - } else { - // shortcode template may have changed, rerender - p.contentShortCodes = renderShortcodes(p.shortcodes, p, t) - } - + tmpContent, tmpContentShortCodes, _ := extractAndRenderShortcodes(string(p.rawContent), p, t) + p.rawContent = []byte(tmpContent) + p.contentShortCodes = tmpContentShortCodes } func (p *Page) FullFilePath() string { diff --git a/hugolib/shortcode.go b/hugolib/shortcode.go index 52cd6893b..854f89899 100644 --- a/hugolib/shortcode.go +++ b/hugolib/shortcode.go @@ -281,10 +281,6 @@ func renderShortcode(sc shortcode, parent *ShortcodeWithPage, p *Page, t tpl.Tem func extractAndRenderShortcodes(stringToParse string, p *Page, t tpl.Template) (string, map[string]func() (string, error), error) { - if p.rendered { - panic("Illegal state: Page already marked as rendered, please reuse the shortcodes") - } - content, shortcodes, err := extractShortcodes(stringToParse, p, t) if err != nil { diff --git a/hugolib/site.go b/hugolib/site.go index 9f435218c..22b22773d 100644 --- a/hugolib/site.go +++ b/hugolib/site.go @@ -450,9 +450,14 @@ func (s *Site) timerStep(step string) { s.timer.Step(step) } +type whatChanged struct { + source bool + other bool +} + // reBuild partially rebuilds a site given the filesystem events. // It returns whetever the content source was changed. -func (s *Site) reBuild(events []fsnotify.Event) (bool, error) { +func (s *Site) reBuild(events []fsnotify.Event) (whatChanged, error) { jww.DEBUG.Printf("Rebuild for events %q", events) @@ -500,7 +505,6 @@ func (s *Site) reBuild(events []fsnotify.Event) (bool, error) { } if len(i18nChanged) > 0 { - // TODO(bep ml s.readI18nSources() } @@ -564,16 +568,6 @@ func (s *Site) reBuild(events []fsnotify.Event) (bool, error) { go incrementalReadCollator(s, readResults, pageChan, fileConvChan, coordinator, errs) go converterCollator(s, convertResults, errs) - if len(tmplChanged) > 0 || len(dataChanged) > 0 { - // Do not need to read the files again, but they need conversion - // for shortocde re-rendering. - for _, p := range s.rawAllPages { - if p.shouldBuild() { - pageChan <- p - } - } - } - for _, ev := range sourceReallyChanged { file, err := s.reReadFile(ev.Name) @@ -610,7 +604,12 @@ func (s *Site) reBuild(events []fsnotify.Event) (bool, error) { s.timerStep("read & convert pages from source") - return len(sourceChanged) > 0, nil + changed := whatChanged{ + source: len(sourceChanged) > 0, + other: len(tmplChanged) > 0 || len(i18nChanged) > 0 || len(dataChanged) > 0, + } + + return changed, nil }