From 7917961d59b0dc216e49f59dc0255f9b46534115 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Wed, 28 Jun 2023 10:27:39 +0200 Subject: Misc permalinks adjustments * Move config loading to the page package * Fix a lower bound panic for the `:sections` slice syntax. * Always return the `:title` * Add some permalinks integration tests * Also see issues below Fixes #9448 Fixes #11184 See #8523 --- config/allconfig/alldecoders.go | 46 +----- hugofs/files/classifier.go | 1 - hugolib/content_map.go | 9 +- hugolib/integrationtest_builder.go | 4 + hugolib/page__paths.go | 4 +- hugolib/site_url_test.go | 22 +++ resources/page/permalinks.go | 80 +++++++++-- resources/page/permalinks_integration_test.go | 198 ++++++++++++++++++++++++++ resources/page/permalinks_test.go | 47 +++--- resources/page/zero_file.autogen.go | 6 - source/fileInfo.go | 8 -- 11 files changed, 336 insertions(+), 89 deletions(-) create mode 100644 resources/page/permalinks_integration_test.go diff --git a/config/allconfig/alldecoders.go b/config/allconfig/alldecoders.go index bda3122d3..a40a02372 100644 --- a/config/allconfig/alldecoders.go +++ b/config/allconfig/alldecoders.go @@ -206,49 +206,9 @@ var allDecoderSetups = map[string]decodeWeight{ "permalinks": { key: "permalinks", decode: func(d decodeWeight, p decodeConfig) error { - p.c.Permalinks = make(map[string]map[string]string) - - p.c.Permalinks["page"] = make(map[string]string) - p.c.Permalinks["section"] = make(map[string]string) - p.c.Permalinks["taxonomy"] = make(map[string]string) - p.c.Permalinks["term"] = make(map[string]string) - - config := maps.CleanConfigStringMap(p.p.GetStringMap(d.key)) - for k, v := range config { - switch v := v.(type) { - case string: - // [permalinks] - // key = '...' - - // To sucessfully be backward compatible, "default" patterns need to be set for both page and term - p.c.Permalinks["page"][k] = v - p.c.Permalinks["term"][k] = v - - case maps.Params: - // [permalinks.key] - // xyz = ??? - - if (k == "page") || (k == "section") || (k == "taxonomy") || (k == "term") { - // TODO: warn if we overwrite an already set value - for k2, v2 := range v { - switch v2 := v2.(type) { - case string: - p.c.Permalinks[k][k2] = v2 - - default: - return fmt.Errorf("permalinks configuration invalid: unknown value %q for key %q for kind %q", v2, k2, k) - } - } - } else { - return fmt.Errorf("permalinks configuration only allows per-kind configuration 'page', 'section', 'taxonomy' and 'term'; unknown kind: %q", k) - } - - default: - return fmt.Errorf("permalinks configuration invalid: unknown value %q for key %q", v, k) - } - } - - return nil + var err error + p.c.Permalinks, err = page.DecodePermalinksConfig(p.p.GetStringMap(d.key)) + return err }, }, "sitemap": { diff --git a/hugofs/files/classifier.go b/hugofs/files/classifier.go index 5690795ed..09b239c21 100644 --- a/hugofs/files/classifier.go +++ b/hugofs/files/classifier.go @@ -93,7 +93,6 @@ const ( ContentClassBranch ContentClass = "branch" ContentClassFile ContentClass = "zfile" // Sort below ContentClassContent ContentClass = "zcontent" - ContentClassZero ContentClass = "zero" // Special value for zeroFile ) func (c ContentClass) IsBundle() bool { diff --git a/hugolib/content_map.go b/hugolib/content_map.go index a7f344004..9abf0d5b0 100644 --- a/hugolib/content_map.go +++ b/hugolib/content_map.go @@ -680,7 +680,14 @@ func (m *contentMap) splitKey(k string) []string { return nil } - return strings.Split(k, "/")[1:] + parts := strings.Split(k, "/")[1:] + if len(parts) == 0 { + return nil + } + if parts[len(parts)-1] == "" { + parts = parts[:len(parts)-1] + } + return parts } func (m *contentMap) testDump() string { diff --git a/hugolib/integrationtest_builder.go b/hugolib/integrationtest_builder.go index 9c40fa7d0..3910e2b97 100644 --- a/hugolib/integrationtest_builder.go +++ b/hugolib/integrationtest_builder.go @@ -205,6 +205,10 @@ func (s *IntegrationTestBuilder) Build() *IntegrationTestBuilder { return s } +func (s *IntegrationTestBuilder) LogString() string { + return s.logBuff.String() +} + func (s *IntegrationTestBuilder) BuildE() (*IntegrationTestBuilder, error) { s.Helper() if err := s.initBuilder(); err != nil { diff --git a/hugolib/page__paths.go b/hugolib/page__paths.go index f98531c10..a834a1d9d 100644 --- a/hugolib/page__paths.go +++ b/hugolib/page__paths.go @@ -109,13 +109,13 @@ func createTargetPathDescriptor(s *Site, p page.Page, pm *pageMeta) (page.Target ) d := s.Deps - classifier := files.ContentClassZero + var classifier files.ContentClass if !p.File().IsZero() { dir = p.File().Dir() baseName = p.File().TranslationBaseName() contentBaseName = p.File().ContentBaseName() - classifier = p.File().Classifier() + classifier = p.File().FileInfo().Meta().Classifier } if classifier == files.ContentClassLeaf { diff --git a/hugolib/site_url_test.go b/hugolib/site_url_test.go index 62483093c..cb69be4ca 100644 --- a/hugolib/site_url_test.go +++ b/hugolib/site_url_test.go @@ -160,3 +160,25 @@ Do not go gentle into that good night. th.assertFileContent(filepath.Join("public", "ss1", "index.html"), "P1|URL: /ss1/|Next: /ss1/page/2/") th.assertFileContent(filepath.Join("public", "ss1", "page", "2", "index.html"), "P2|URL: /ss1/page/2/|Next: /ss1/page/3/") } + +func TestSectionsEntries(t *testing.T) { + files := ` +-- hugo.toml -- +-- content/withfile/_index.md -- +-- content/withoutfile/p1.md -- +-- layouts/_default/list.html -- +SectionsEntries: {{ .SectionsEntries }} + + +` + + b := NewIntegrationTestBuilder( + IntegrationTestConfig{ + T: t, + TxtarString: files, + }, + ).Build() + + b.AssertFileContent("public/withfile/index.html", "SectionsEntries: [withfile]") + b.AssertFileContent("public/withoutfile/index.html", "SectionsEntries: [withoutfile]") +} diff --git a/resources/page/permalinks.go b/resources/page/permalinks.go index ab418a20b..3b12e1154 100644 --- a/resources/page/permalinks.go +++ b/resources/page/permalinks.go @@ -25,8 +25,8 @@ import ( "errors" + "github.com/gohugoio/hugo/common/maps" "github.com/gohugoio/hugo/helpers" - ) // PermalinkExpander holds permalin mappings per section. @@ -252,16 +252,12 @@ func (l PermalinkExpander) pageToPermalinkDate(p Page, dateField string) (string // pageToPermalinkTitle returns the URL-safe form of the title func (l PermalinkExpander) pageToPermalinkTitle(p Page, _ string) (string, error) { - if p.File().TranslationBaseName() == "_index" { - return "", nil - } - return l.urlize(p.Title()), nil } // pageToPermalinkFilename returns the URL-safe form of the filename func (l PermalinkExpander) pageToPermalinkFilename(p Page, _ string) (string, error) { - name := p.File().TranslationBaseName() + name := l.translationBaseName(p) if name == "index" { // Page bundles; the directory name will hopefully have a better name. dir := strings.TrimSuffix(p.File().Dir(), helpers.FilePathSeparator) @@ -297,6 +293,13 @@ func (l PermalinkExpander) pageToPermalinkSections(p Page, _ string) (string, er return p.CurrentSection().SectionsPath(), nil } +func (l PermalinkExpander) translationBaseName(p Page) string { + if p.File().IsZero() { + return "" + } + return p.File().TranslationBaseName() +} + var ( nilSliceFunc = func(s []string) []string { return nil @@ -349,7 +352,10 @@ func (l PermalinkExpander) toSliceFunc(cut string) func(s []string) []string { return func(ss []string) int { // Prevent out of bound situations. It would not make // much sense to panic here. - if n > len(ss) { + if n >= len(ss) { + if low { + return -1 + } return len(ss) } return n @@ -365,7 +371,11 @@ func (l PermalinkExpander) toSliceFunc(cut string) func(s []string) []string { if len(s) == 0 { return nil } - v := s[toN(s)] + n := toN(s) + if n < 0 { + return []string{} + } + v := s[n] if v == "" { return nil } @@ -379,7 +389,59 @@ func (l PermalinkExpander) toSliceFunc(cut string) func(s []string) []string { if len(s) == 0 { return nil } - return s[toN1(s):toN2(s)] + n1, n2 := toN1(s), toN2(s) + if n1 < 0 || n2 < 0 { + return []string{} + } + return s[n1:n2] } } + +var permalinksKindsSuppurt = []string{KindPage, KindSection, KindTaxonomy, KindTerm} + +// DecodePermalinksConfig decodes the permalinks configuration in the given map +func DecodePermalinksConfig(m map[string]any) (map[string]map[string]string, error) { + permalinksConfig := make(map[string]map[string]string) + + permalinksConfig[KindPage] = make(map[string]string) + permalinksConfig[KindSection] = make(map[string]string) + permalinksConfig[KindTaxonomy] = make(map[string]string) + permalinksConfig[KindTerm] = make(map[string]string) + + config := maps.CleanConfigStringMap(m) + for k, v := range config { + switch v := v.(type) { + case string: + // [permalinks] + // key = '...' + + // To sucessfully be backward compatible, "default" patterns need to be set for both page and term + permalinksConfig[KindPage][k] = v + permalinksConfig[KindTerm][k] = v + + case maps.Params: + // [permalinks.key] + // xyz = ??? + + if helpers.InStringArray(permalinksKindsSuppurt, k) { + // TODO: warn if we overwrite an already set value + for k2, v2 := range v { + switch v2 := v2.(type) { + case string: + permalinksConfig[k][k2] = v2 + + default: + return nil, fmt.Errorf("permalinks configuration invalid: unknown value %q for key %q for kind %q", v2, k2, k) + } + } + } else { + return nil, fmt.Errorf("permalinks configuration not supported for kind %q, supported kinds are %v", k, permalinksKindsSuppurt) + } + + default: + return nil, fmt.Errorf("permalinks configuration invalid: unknown value %q for key %q", v, k) + } + } + return permalinksConfig, nil +} diff --git a/resources/page/permalinks_integration_test.go b/resources/page/permalinks_integration_test.go new file mode 100644 index 000000000..6c2411ad7 --- /dev/null +++ b/resources/page/permalinks_integration_test.go @@ -0,0 +1,198 @@ +// Copyright 2023 The Hugo Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package page_test + +import ( + "testing" + + "github.com/bep/logg" + qt "github.com/frankban/quicktest" + "github.com/gohugoio/hugo/hugolib" +) + +func TestPermalinks(t *testing.T) { + t.Parallel() + + files := ` +-- layouts/_default/list.html -- +List|{{ .Kind }}|{{ .RelPermalink }}| +-- layouts/_default/single.html -- +Single|{{ .Kind }}|{{ .RelPermalink }}| +-- hugo.toml -- +[taxonomies] +tag = "tags" +[permalinks.page] +withpageslug = '/pageslug/:slug/' +withallbutlastsection = '/:sections[:last]/:slug/' +[permalinks.section] +withfilefilename = '/sectionwithfilefilename/:filename/' +withfilefiletitle = '/sectionwithfilefiletitle/:title/' +withfileslug = '/sectionwithfileslug/:slug/' +nofileslug = '/sectionnofileslug/:slug/' +nofilefilename = '/sectionnofilefilename/:filename/' +nofiletitle1 = '/sectionnofiletitle1/:title/' +nofiletitle2 = '/sectionnofiletitle2/:sections[:last]/' +[permalinks.term] +tags = '/tagsslug/tag/:slug/' +[permalinks.taxonomy] +tags = '/tagsslug/:slug/' +-- content/withpageslug/p1.md -- +--- +slug: "p1slugvalue" +tags: ["mytag"] +--- +-- content/withfilefilename/_index.md -- +-- content/withfileslug/_index.md -- +--- +slug: "withfileslugvalue" +--- +-- content/nofileslug/p1.md -- +-- content/nofilefilename/p1.md -- +-- content/nofiletitle1/p1.md -- +-- content/nofiletitle2/asdf/p1.md -- +-- content/withallbutlastsection/subsection/p1.md -- +-- content/tags/_index.md -- +--- +slug: "tagsslug" +--- +-- content/tags/mytag/_index.md -- +--- +slug: "mytagslug" +--- + + +` + + b := hugolib.NewIntegrationTestBuilder( + hugolib.IntegrationTestConfig{ + T: t, + TxtarString: files, + LogLevel: logg.LevelWarn, + }).Build() + + t.Log(b.LogString()) + // No .File.TranslationBaseName on zero object etc. warnings. + b.Assert(b.H.Log.LoggCount(logg.LevelWarn), qt.Equals, 0) + b.AssertFileContent("public/pageslug/p1slugvalue/index.html", "Single|page|/pageslug/p1slugvalue/|") + b.AssertFileContent("public/sectionwithfilefilename/index.html", "List|section|/sectionwithfilefilename/|") + b.AssertFileContent("public/sectionwithfileslug/withfileslugvalue/index.html", "List|section|/sectionwithfileslug/withfileslugvalue/|") + b.AssertFileContent("public/sectionnofilefilename/index.html", "List|section|/sectionnofilefilename/|") + b.AssertFileContent("public/sectionnofileslug/nofileslugs/index.html", "List|section|/sectionnofileslug/nofileslugs/|") + b.AssertFileContent("public/sectionnofiletitle1/nofiletitle1s/index.html", "List|section|/sectionnofiletitle1/nofiletitle1s/|") + b.AssertFileContent("public/sectionnofiletitle2/index.html", "List|section|/sectionnofiletitle2/|") + + b.AssertFileContent("public/tagsslug/tag/mytagslug/index.html", "List|term|/tagsslug/tag/mytagslug/|") + b.AssertFileContent("public/tagsslug/tagsslug/index.html", "List|taxonomy|/tagsslug/tagsslug/|") + + permalinksConf := b.H.Configs.Base.Permalinks + b.Assert(permalinksConf, qt.DeepEquals, map[string]map[string]string{ + "page": {"withallbutlastsection": "/:sections[:last]/:slug/", "withpageslug": "/pageslug/:slug/"}, + "section": {"nofilefilename": "/sectionnofilefilename/:filename/", "nofileslug": "/sectionnofileslug/:slug/", "nofiletitle1": "/sectionnofiletitle1/:title/", "nofiletitle2": "/sectionnofiletitle2/:sections[:last]/", "withfilefilename": "/sectionwithfilefilename/:filename/", "withfilefiletitle": "/sectionwithfilefiletitle/:title/", "withfileslug": "/sectionwithfileslug/:slug/"}, + "taxonomy": {"tags": "/tagsslug/:slug/"}, + "term": {"tags": "/tagsslug/tag/:slug/"}, + }) + +} + +func TestPermalinksOldSetup(t *testing.T) { + t.Parallel() + + files := ` +-- layouts/_default/list.html -- +List|{{ .Kind }}|{{ .RelPermalink }}| +-- layouts/_default/single.html -- +Single|{{ .Kind }}|{{ .RelPermalink }}| +-- hugo.toml -- +[permalinks] +withpageslug = '/pageslug/:slug/' +-- content/withpageslug/p1.md -- +--- +slug: "p1slugvalue" +--- + + + + +` + + b := hugolib.NewIntegrationTestBuilder( + hugolib.IntegrationTestConfig{ + T: t, + TxtarString: files, + LogLevel: logg.LevelWarn, + }).Build() + + t.Log(b.LogString()) + // No .File.TranslationBaseName on zero object etc. warnings. + b.Assert(b.H.Log.LoggCount(logg.LevelWarn), qt.Equals, 0) + b.AssertFileContent("public/pageslug/p1slugvalue/index.html", "Single|page|/pageslug/p1slugvalue/|") + + permalinksConf := b.H.Configs.Base.Permalinks + b.Assert(permalinksConf, qt.DeepEquals, map[string]map[string]string{ + "page": {"withpageslug": "/pageslug/:slug/"}, + "section": {}, + "taxonomy": {}, + "term": {"withpageslug": "/pageslug/:slug/"}, + }) + +} + +func TestPermalinksNestedSections(t *testing.T) { + t.Parallel() + + files := ` +-- hugo.toml -- +[permalinks.page] +books = '/libros/:sections[1:]/:filename' + +[permalinks.section] +books = '/libros/:sections[1:]' +-- content/books/_index.md -- +--- +title: Books +--- +-- content/books/fiction/_index.md -- +--- +title: Fiction +--- +-- content/books/fiction/2023/_index.md -- +--- +title: 2023 +--- +-- content/books/fiction/2023/book1/index.md -- +--- +title: Book 1 +--- +-- layouts/_default/single.html -- +Single. +-- layouts/_default/list.html -- +List. +` + + b := hugolib.NewIntegrationTestBuilder( + hugolib.IntegrationTestConfig{ + T: t, + TxtarString: files, + LogLevel: logg.LevelWarn, + }).Build() + + t.Log(b.LogString()) + // No .File.TranslationBaseName on zero object etc. warnings. + b.Assert(b.H.Log.LoggCount(logg.LevelWarn), qt.Equals, 0) + + b.AssertFileContent("public/libros/index.html", "List.") + b.AssertFileContent("public/libros/fiction/index.html", "List.") + b.AssertFileContent("public/libros/fiction/2023/book1/index.html", "Single.") + +} diff --git a/resources/page/permalinks_test.go b/resources/page/permalinks_test.go index 24eb27f5b..194387d5c 100644 --- a/resources/page/permalinks_test.go +++ b/resources/page/permalinks_test.go @@ -179,36 +179,45 @@ func TestPermalinkExpansionSliceSyntax(t *testing.T) { c := qt.New(t) exp, err := NewPermalinkExpander(urlize, nil) c.Assert(err, qt.IsNil) - slice := []string{"a", "b", "c", "d"} - fn := func(s string) []string { - return exp.toSliceFunc(s)(slice) + slice4 := []string{"a", "b", "c", "d"} + fn4 := func(s string) []string { + return exp.toSliceFunc(s)(slice4) + } + + slice1 := []string{"a"} + fn1 := func(s string) []string { + return exp.toSliceFunc(s)(slice1) } c.Run("Basic", func(c *qt.C) { - c.Assert(fn("[1:3]"), qt.DeepEquals, []string{"b", "c"}) - c.Assert(fn("[1:]"), qt.DeepEquals, []string{"b", "c", "d"}) - c.Assert(fn("[:2]"), qt.DeepEquals, []string{"a", "b"}) - c.Assert(fn("[0:2]"), qt.DeepEquals, []string{"a", "b"}) - c.Assert(fn("[:]"), qt.DeepEquals, []string{"a", "b", "c", "d"}) - c.Assert(fn(""), qt.DeepEquals, []string{"a", "b", "c", "d"}) - c.Assert(fn("[last]"), qt.DeepEquals, []string{"d"}) - c.Assert(fn("[:last]"), qt.DeepEquals, []string{"a", "b", "c"}) + c.Assert(fn4("[1:3]"), qt.DeepEquals, []string{"b", "c"}) + c.Assert(fn4("[1:]"), qt.DeepEquals, []string{"b", "c", "d"}) + c.Assert(fn4("[:2]"), qt.DeepEquals, []string{"a", "b"}) + c.Assert(fn4("[0:2]"), qt.DeepEquals, []string{"a", "b"}) + c.Assert(fn4("[:]"), qt.DeepEquals, []string{"a", "b", "c", "d"}) + c.Assert(fn4(""), qt.DeepEquals, []string{"a", "b", "c", "d"}) + c.Assert(fn4("[last]"), qt.DeepEquals, []string{"d"}) + c.Assert(fn4("[:last]"), qt.DeepEquals, []string{"a", "b", "c"}) + c.Assert(fn1("[last]"), qt.DeepEquals, []string{"a"}) + c.Assert(fn1("[:last]"), qt.DeepEquals, []string{}) + c.Assert(fn1("[1:last]"), qt.DeepEquals, []string{}) + c.Assert(fn1("[1]"), qt.DeepEquals, []string{}) }) c.Run("Out of bounds", func(c *qt.C) { - c.Assert(fn("[1:5]"), qt.DeepEquals, []string{"b", "c", "d"}) - c.Assert(fn("[-1:5]"), qt.DeepEquals, []string{"a", "b", "c", "d"}) - c.Assert(fn("[5:]"), qt.DeepEquals, []string{}) - c.Assert(fn("[5:]"), qt.DeepEquals, []string{}) - c.Assert(fn("[5:32]"), qt.DeepEquals, []string{}) + c.Assert(fn4("[1:5]"), qt.DeepEquals, []string{"b", "c", "d"}) + c.Assert(fn4("[-1:5]"), qt.DeepEquals, []string{"a", "b", "c", "d"}) + c.Assert(fn4("[5:]"), qt.DeepEquals, []string{}) + c.Assert(fn4("[5:]"), qt.DeepEquals, []string{}) + c.Assert(fn4("[5:32]"), qt.DeepEquals, []string{}) c.Assert(exp.toSliceFunc("[:1]")(nil), qt.DeepEquals, []string(nil)) c.Assert(exp.toSliceFunc("[:1]")([]string{}), qt.DeepEquals, []string(nil)) // These all return nil - c.Assert(fn("[]"), qt.IsNil) - c.Assert(fn("[1:}"), qt.IsNil) - c.Assert(fn("foo"), qt.IsNil) + c.Assert(fn4("[]"), qt.IsNil) + c.Assert(fn4("[1:}"), qt.IsNil) + c.Assert(fn4("foo"), qt.IsNil) }) diff --git a/resources/page/zero_file.autogen.go b/resources/page/zero_file.autogen.go index 979f92a0f..72d98998e 100644 --- a/resources/page/zero_file.autogen.go +++ b/resources/page/zero_file.autogen.go @@ -18,7 +18,6 @@ package page import ( "github.com/gohugoio/hugo/common/loggers" "github.com/gohugoio/hugo/hugofs" - "github.com/gohugoio/hugo/hugofs/files" "github.com/gohugoio/hugo/source" ) @@ -35,11 +34,6 @@ func (zeroFile) IsZero() bool { return true } -func (z zeroFile) Classifier() files.ContentClass { - z.log.Warnln(".File.Classifier on zero object. Wrap it in if or with: {{ with .File }}{{ .Classifier }}{{ end }}") - return files.ContentClassZero -} - func (z zeroFile) Path() (o0 string) { z.log.Warnln(".File.Path on zero object. Wrap it in if or with: {{ with .File }}{{ .Path }}{{ end }}") return diff --git a/source/fileInfo.go b/source/fileInfo.go index 03bb1b16c..c58a0c3b9 100644 --- a/source/fileInfo.go +++ b/source/fileInfo.go @@ -92,9 +92,6 @@ type FileWithoutOverlap interface { // if file is a leaf bundle. ContentBaseName() string - // Classifier is the ContentClass of the file - Classifier() files.ContentClass - // UniqueID is the MD5 hash of the file's path and is for most practical applications, // Hugo content files being one of them, considered to be unique. UniqueID() string @@ -173,11 +170,6 @@ func (fi *FileInfo) ContentBaseName() string { return fi.contentBaseName } -// Classifier is the ContentClass of the file -func (fi *FileInfo) Classifier() files.ContentClass { - return fi.classifier; -} - // Section returns a file's section. func (fi *FileInfo) Section() string { fi.init() -- cgit v1.2.3