Fix some uglyURLs issues for home, section and taxonomy kind (note)

Fixes #4428 Fixes #7497 Co-authored-by: Joe Mooring <joe.mooring@veriphor.com>
3 weeks ago · b8ba33ca95
4 changed files with 27 additions and 168 deletions
--- a/hugolib/site_test.go
+++ b/hugolib/site_test.go
@ -511,12 +511,13 @@ func TestSectionNaming(t *testing.T) {
 func doTestSectionNaming(t *testing.T, canonify, uglify, pluralize bool) {
 	c := qt.New(t)

-	var expectedPathSuffix string
-
-	if uglify {
-		expectedPathSuffix = ".html"
-	} else {
-		expectedPathSuffix = "/index.html"
+	expectedPathSuffix := func(kind string) string {
+		isUgly := uglify && (kind == kinds.KindPage || kind == kinds.KindTerm)
+		if isUgly {
+			return ".html"
+		} else {
+			return "/index.html"
+		}
 	}

 	sources := [][2]string{
@ -554,12 +555,12 @@ func doTestSectionNaming(t *testing.T, canonify, uglify, pluralize bool) {
 		pluralAware bool
 		expected    string
 	}{
-		{filepath.FromSlash(fmt.Sprintf("sect/doc1%s", expectedPathSuffix)), false, "doc1"},
-		{filepath.FromSlash(fmt.Sprintf("sect%s", expectedPathSuffix)), true, "Sect"},
-		{filepath.FromSlash(fmt.Sprintf("fish-and-chips/doc2%s", expectedPathSuffix)), false, "doc2"},
-		{filepath.FromSlash(fmt.Sprintf("fish-and-chips%s", expectedPathSuffix)), true, "Fish and Chips"},
-		{filepath.FromSlash(fmt.Sprintf("ラーメン/doc3%s", expectedPathSuffix)), false, "doc3"},
-		{filepath.FromSlash(fmt.Sprintf("ラーメン%s", expectedPathSuffix)), true, "ラーメン"},
+		{filepath.FromSlash(fmt.Sprintf("sect/doc1%s", expectedPathSuffix(kinds.KindPage))), false, "doc1"},
+		{filepath.FromSlash(fmt.Sprintf("sect%s", expectedPathSuffix(kinds.KindSection))), true, "Sect"},
+		{filepath.FromSlash(fmt.Sprintf("fish-and-chips/doc2%s", expectedPathSuffix(kinds.KindPage))), false, "doc2"},
+		{filepath.FromSlash(fmt.Sprintf("fish-and-chips%s", expectedPathSuffix(kinds.KindSection))), true, "Fish and Chips"},
+		{filepath.FromSlash(fmt.Sprintf("ラーメン/doc3%s", expectedPathSuffix(kinds.KindPage))), false, "doc3"},
+		{filepath.FromSlash(fmt.Sprintf("ラーメン%s", expectedPathSuffix(kinds.KindSection))), true, "ラーメン"},
 	}

 	for _, test := range tests {
--- a/hugolib/taxonomy_test.go
+++ b/hugolib/taxonomy_test.go
@ -65,151 +65,6 @@ YAML frontmatter with tags and categories taxonomy.`
 	}
 }

-func TestTaxonomiesWithAndWithoutContentFile(t *testing.T) {
-	for _, uglyURLs := range []bool{false, true} {
-		uglyURLs := uglyURLs
-		t.Run(fmt.Sprintf("uglyURLs=%t", uglyURLs), func(t *testing.T) {
-			t.Parallel()
-			doTestTaxonomiesWithAndWithoutContentFile(t, uglyURLs)
-		})
-	}
-}
-
-func doTestTaxonomiesWithAndWithoutContentFile(t *testing.T, uglyURLs bool) {
-	t.Helper()
-
-	siteConfig := `
-baseURL = "http://example.com/blog"
-titleCaseStyle = "firstupper"
-uglyURLs = %t
-defaultContentLanguage = "en"
-[pagination]
-pagerSize = 1
-[Taxonomies]
-tag = "tags"
-category = "categories"
-other = "others"
-empty = "empties"
-permalinked = "permalinkeds"
-[permalinks]
-permalinkeds = "/perma/:slug/"
-`
-
-	pageTemplate := `---
-title: "%s"
-tags:
-%s
-categories:
-%s
-others:
-%s
-permalinkeds:
-%s
---
-# Doc
-`
-
-	siteConfig = fmt.Sprintf(siteConfig, uglyURLs)
-
-	b := newTestSitesBuilder(t).WithConfigFile("toml", siteConfig)
-
-	b.WithContent(
-		"p1.md", fmt.Sprintf(pageTemplate, "t1/c1", "- Tag1", "- cAt1", "- o1", "- Pl1"),
-		"p2.md", fmt.Sprintf(pageTemplate, "t2/c1", "- tag2", "- cAt1", "- o1", "- Pl1"),
-		"p3.md", fmt.Sprintf(pageTemplate, "t2/c12", "- tag2", "- cat2", "- o1", "- Pl1"),
-		"p4.md", fmt.Sprintf(pageTemplate, "Hello World", "", "", "- \"Hello Hugo world\"", "- Pl1"),
-		"categories/_index.md", newTestPage("Category Terms", "2017-01-01", 10),
-		"tags/Tag1/_index.md", newTestPage("Tag1 List", "2017-01-01", 10),
-		// https://github.com/gohugoio/hugo/issues/5847
-		"/tags/not-used/_index.md", newTestPage("Unused Tag List", "2018-01-01", 10),
-	)
-
-	b.Build(BuildCfg{})
-
-	// So what we have now is:
-	// 1. categories with terms content page, but no content page for the only c1 category
-	// 2. tags with no terms content page, but content page for one of 2 tags (tag1)
-	// 3. the "others" taxonomy with no content pages.
-	// 4. the "permalinkeds" taxonomy with permalinks configuration.
-
-	pathFunc := func(s string) string {
-		if uglyURLs {
-			return strings.Replace(s, "/index.html", ".html", 1)
-		}
-		return s
-	}
-
-	// 1.
-	b.AssertFileContent(pathFunc("public/categories/cat1/index.html"), "List", "CAt1")
-	b.AssertFileContent(pathFunc("public/categories/index.html"), "Taxonomy Term Page", "Category Terms")
-
-	// 2.
-	b.AssertFileContent(pathFunc("public/tags/tag2/index.html"), "List", "tag2")
-	b.AssertFileContent(pathFunc("public/tags/tag1/index.html"), "List", "Tag1")
-	b.AssertFileContent(pathFunc("public/tags/index.html"), "Taxonomy Term Page", "Tags")
-
-	// 3.
-	b.AssertFileContent(pathFunc("public/others/o1/index.html"), "List", "o1")
-	b.AssertFileContent(pathFunc("public/others/index.html"), "Taxonomy Term Page", "Others")
-
-	// 4.
-	b.AssertFileContent(pathFunc("public/perma/pl1/index.html"), "List", "Pl1")
-
-	// This looks kind of funky, but the taxonomy terms do not have a permalinks definition,
-	// for good reasons.
-	b.AssertFileContent(pathFunc("public/permalinkeds/index.html"), "Taxonomy Term Page", "Permalinkeds")
-
-	s := b.H.Sites[0]
-
-	// Make sure that each kinds.KindTaxonomyTerm page has an appropriate number
-	// of kinds.KindTaxonomy pages in its Pages slice.
-	taxonomyTermPageCounts := map[string]int{
-		"tags":         3,
-		"categories":   2,
-		"others":       2,
-		"empties":      0,
-		"permalinkeds": 1,
-	}
-
-	for taxonomy, count := range taxonomyTermPageCounts {
-		msg := qt.Commentf(taxonomy)
-		term := s.getPageOldVersion(kinds.KindTaxonomy, taxonomy)
-		b.Assert(term, qt.Not(qt.IsNil), msg)
-		b.Assert(len(term.Pages()), qt.Equals, count, msg)
-
-		for _, p := range term.Pages() {
-			b.Assert(p.Kind(), qt.Equals, kinds.KindTerm)
-		}
-	}
-
-	cat1 := s.getPageOldVersion(kinds.KindTerm, "categories", "cat1")
-	b.Assert(cat1, qt.Not(qt.IsNil))
-	if uglyURLs {
-		b.Assert(cat1.RelPermalink(), qt.Equals, "/blog/categories/cat1.html")
-	} else {
-		b.Assert(cat1.RelPermalink(), qt.Equals, "/blog/categories/cat1/")
-	}
-
-	pl1 := s.getPageOldVersion(kinds.KindTerm, "permalinkeds", "pl1")
-	permalinkeds := s.getPageOldVersion(kinds.KindTaxonomy, "permalinkeds")
-	b.Assert(pl1, qt.Not(qt.IsNil))
-	b.Assert(permalinkeds, qt.Not(qt.IsNil))
-	if uglyURLs {
-		b.Assert(pl1.RelPermalink(), qt.Equals, "/blog/perma/pl1.html")
-		b.Assert(permalinkeds.RelPermalink(), qt.Equals, "/blog/permalinkeds.html")
-	} else {
-		b.Assert(pl1.RelPermalink(), qt.Equals, "/blog/perma/pl1/")
-		b.Assert(permalinkeds.RelPermalink(), qt.Equals, "/blog/permalinkeds/")
-	}
-
-	helloWorld := s.getPageOldVersion(kinds.KindTerm, "others", "hello-hugo-world")
-	b.Assert(helloWorld, qt.Not(qt.IsNil))
-	b.Assert(helloWorld.Title(), qt.Equals, "Hello Hugo world")
-
-	// Issue #2977
-	b.AssertFileContent(pathFunc("public/empties/index.html"), "Taxonomy Term Page", "Empties")
-}
-
 // https://github.com/gohugoio/hugo/issues/5513
 // https://github.com/gohugoio/hugo/issues/5571
 func TestTaxonomiesPathSeparation(t *testing.T) {
--- a/resources/page/page_paths.go
+++ b/resources/page/page_paths.go
@ -140,6 +140,8 @@ func CreateTargetPaths(d TargetPathDescriptor) (tp TargetPaths) {

 	pb.isUgly = (d.UglyURLs || d.Type.Ugly) && !d.Type.NoUgly
 	pb.baseNameSameAsType = !d.Path.IsBundle() && d.BaseName != "" && d.BaseName == d.Type.BaseName
+	indexIsUglyKind := d.Kind == kinds.KindHome || d.Kind == kinds.KindSection || d.Kind == kinds.KindTaxonomy
+	indexIsUglyKind = indexIsUglyKind && pb.isUgly

 	if d.ExpandedPermalink == "" && pb.baseNameSameAsType {
 		pb.isUgly = true
@ -233,13 +235,13 @@ func CreateTargetPaths(d TargetPathDescriptor) (tp TargetPaths) {

 		needsBase = needsBase && d.Addends == ""

-		if needsBase || !pb.isUgly {
+		if needsBase || (!pb.isUgly || indexIsUglyKind) {
 			pb.Add(d.Type.BaseName + pb.fullSuffix)
 		} else {
 			pb.ConcatLast(pb.fullSuffix)
 		}

-		if pb.IsHtmlIndex() {
+		if !indexIsUglyKind && pb.IsHtmlIndex() {
 			pb.linkUpperOffset = 1
 		}

@ -261,6 +263,7 @@ func CreateTargetPaths(d TargetPathDescriptor) (tp TargetPaths) {
 	}

 	link := pb.Link()
+
 	pagePath := pb.PathFile()

 	tp.TargetFilename = filepath.FromSlash(pagePath)
--- a/resources/page/path_integration_test.go
+++ b/resources/page/path_integration_test.go
@ -55,10 +55,11 @@ title: p#2
 	b.AssertFileContentExact("public/index.html", "/|/s1/p%231/|/s2/p%232/|/tags/test%23tag%23/|")
 }

-func TestOutputFormatWithPathIssue13829(t *testing.T) {
+// Issues: 13829, 4428, 7497.
+func TestMiscPathIssues(t *testing.T) {
 	t.Parallel()

-	files := `
+	filesTemplate := `
 -- hugo.toml --
 uglyURLs = false

@ -115,9 +116,11 @@ title: red
 	const code string = "TITLE: {{ .Title }} | AOFRP: {{ range .AlternativeOutputFormats }}{{ .RelPermalink }}{{ end }} | TEMPLATE: {{ templates.Current.Name }}"

 	for _, template := range templates {
-		files += "-- " + template + " --\n" + code + "\n"
+		filesTemplate += "-- " + template + " --\n" + code + "\n"
 	}

+	files := filesTemplate
+
 	b := hugolib.Test(t, files)

 	// uglyURLs: false, outputFormat: html
@ -134,14 +137,11 @@ title: red
 	b.AssertFileContent("public/print/tags/index.txt", "TITLE: tags | AOFRP: /tags/ | TEMPLATE: taxonomy.print.txt")
 	b.AssertFileContent("public/print/tags/red/index.txt", "TITLE: red | AOFRP: /tags/red/ | TEMPLATE: term.print.txt")

-	files = strings.ReplaceAll(files, "uglyURLs = false", "uglyURLs = true")
+	files = strings.ReplaceAll(filesTemplate, "uglyURLs = false", "uglyURLs = true")
 	b = hugolib.Test(t, files)

-	// The assertions below assume that https://github.com/gohugoio/hugo/issues/4428
-	// and https://github.com/gohugoio/hugo/issues/7497 have been fixed.
-
 	// uglyURLs: true, outputFormat: html
-	/*b.AssertFileContent("public/index.html", "TITLE: home | AOFRP: /print/index.txt | TEMPLATE: home.html")
+	b.AssertFileContent("public/index.html", "TITLE: home | AOFRP: /print/index.txt | TEMPLATE: home.html")
 	b.AssertFileContent("public/s1/index.html", "TITLE: s1 | AOFRP: /print/s1/index.txt | TEMPLATE: section.html")
 	b.AssertFileContent("public/s1/p1.html", "TITLE: p1 | AOFRP: /print/s1/p1.txt | TEMPLATE: page.html")
 	b.AssertFileContent("public/tags/index.html", "TITLE: tags | AOFRP: /print/tags/index.txt | TEMPLATE: taxonomy.html")
@ -152,5 +152,5 @@ title: red
 	b.AssertFileContent("public/print/s1/index.txt", "TITLE: s1 | AOFRP: /s1/index.html | TEMPLATE: section.print.txt")
 	b.AssertFileContent("public/print/s1/p1.txt", "TITLE: p1 | AOFRP: /s1/p1.html | TEMPLATE: page.print.txt")
 	b.AssertFileContent("public/print/tags/index.txt", "TITLE: tags | AOFRP: /tags/index.html | TEMPLATE: taxonomy.print.txt")
-	b.AssertFileContent("public/print/tags/red.txt", "TITLE: red | AOFRP: /tags/red.html | TEMPLATE: term.print.txt")*/
+	b.AssertFileContent("public/print/tags/red.txt", "TITLE: red | AOFRP: /tags/red.html | TEMPLATE: term.print.txt")
 }