resources/page: Allow section&taxonomy pages to have a permalink configuration #10847

Mai-Lapyst · 2023-03-17T23:56:51Z

Allows using permalink configuration for sections (branch bundles) and also for taxonomy pages. Extends the current permalink configuration to be able to specified per page kind while also staying backward compatible: all permalink patterns not dedicated to a certain kind, get automatically added for both normal pages and term pages.

Fixes #8523

Example config:

[permalinks]
  docs = "/docs/1.0/:sections[1:]/:filename"  # Will get added to both page & term permalinks
[permalinks.section]
  docs = "/docs/1.0/:sections[1:]"
[permalinks.page]
[permalinks.term]
[permalinks.taxonomy]

jmooring · 2023-03-18T17:00:55Z

@Mai-Lapyst I did some quick testing and the first thing I found was...

If an _index.md does not exist for a top level section, the permalink setting is not applied. It is somewhat common to not create an _index.md for top-level content sections, and very common to not create an _index.md file for taxonomy or term pages.

content/
├── posts/
│   └── post-1.md
└── _index.md

[permalinks]
posts = '/my-posts/:slug'
[permalinks.section]
posts = '/my-posts'

public/
├── my-posts/
│   └── post-1/
│       └── index.html
├── posts/            
│   └── index.html  <-- unexpected
└── index.html

But with this I get the desired result:

content/
├── posts/
│   ├── _index.md
│   └── post-1.md
└── _index.md

Mai-Lapyst · 2023-03-18T18:05:29Z

@jmooring Thanks for the feedback.

Turns out that the if around the expand is completly redundant now. Removed it and it works like intended.

Also added some debug logging to verify that the permalink for any given file/section is set and to what value exactly. (--debug)

jmooring · 2023-03-19T01:05:11Z

WARN 2023/03/18 17:12:36 Permalinks configuration overwrites pattern in kind 'page' for key 'books' with '/my-books2/:slug' (which was '/my-books/:slug')

Nice!

I spent a couple of hours testing this, and functionally it looks great. It behaves as expected:

When url is set in front matter (trumps everything)
Monolingual
Multilingual
Multilingual with permalinks per language
etc.

FYI, when I run v0.111.3 (no patch) with the new permalink settings in site config, it panics, but that's much better than creating new keys. I don't see that as a problem.

Mai-Lapyst · 2023-03-19T04:07:04Z

Thanks!

FYI, when I run v0.111.3 (no patch) with the new permalink settings in site config, it panics, but that's much better than creating new keys. I don't see that as a problem.

I noticed that too; digged a bit into it and you can get the same crash even in the patched version, by simply using an empty string:

[permalinks]
    posts = ""

It seems that an unchecked string slice access seems to be the problem:

hugo/resources/page/permalinks.go

Lines 259 to 260 in fb35e7f

    
           func (l PermalinkExpander) validate(pp string) bool { 
        
           	fragments := strings.Split(pp[1:], "/")

Will add a patch for it as well.

jmooring · 2023-05-19T18:15:07Z

@bep Functionally this is great, and scratches a long-term itch. Would appreciate a review if/when you have the chance.

bep · 2023-05-19T18:25:30Z

OK, I guess this is what lots of people would want, and I like the config.

I will merge this if we could

Get this rebased against master and get this integrated into my new allconfig.Config struct.
Get a basic integration test up and running.

…iguration Allows using permalink configuration for sections (branch bundles) and also for taxonomy pages. Extends the current permalink configuration to be able to specified per page kind while also staying backward compatible: all permalink patterns not dedicated to a certain kind, get automatically added for both normal pages and term pages. Fixes gohugoio#8523

Mai-Lapyst · 2023-05-26T09:06:52Z

@bep Thanks for looking over it. Have rebased it onto master and reworked it to work with the new allconfig. Also added a test for it; let me know if more tests are needed.

bep · 2023-06-26T11:19:55Z

hugofs/files/classifier.go

@@ -93,6 +93,7 @@ const (
 	ContentClassBranch  ContentClass = "branch"
 	ContentClassFile    ContentClass = "zfile" // Sort below
 	ContentClassContent ContentClass = "zcontent"
+	ContentClassZero    ContentClass = "zero"  // Special value for zeroFile


I don't think we need/want this. A zero file would have an empty ContentClass string, which is the natural zero value, no need to make one up.

bep · 2023-06-26T11:22:04Z

hugolib/page__paths.go

+		desc.ExpandedPermalink = opath
+
+		if !p.File().IsZero() {
+			s.Log.Debugf("Set expanded permalink path for %s %s to %#v", p.Kind(), p.File().Path(), opath)


Can you remove these debug statement, it makes the debug log very busy,.

bep · 2023-06-26T11:23:27Z

config/allconfig/alldecoders.go

+					// [permalinks.key]
+					//   xyz = ???
+
+					if (k == "page") || (k == "section") || (k == "taxonomy") || (k == "term") {


I think you can use the constants in pagekinds.Page etc.

bep · 2023-06-26T11:25:05Z

config/allconfig/alldecoders.go

-			p.c.Permalinks = maps.CleanConfigStringMapString(p.p.GetStringMapString(d.key))
+			p.c.Permalinks = make(map[string]map[string]string)
+
+			p.c.Permalinks["page"] = make(map[string]string)


pagekinds.Page etc.

bep · 2023-06-26T11:33:10Z

resources/page/permalinks_test.go

@@ -69,6 +69,7 @@ func TestPermalinkExpansion(t *testing.T) {
 	page.date = d


This all looks good, but I think we need a test that shows the new setup working. I suggest you create a new permalink_integration_test.go and follow the the cue of similar tests (using the NewIntegrationTest...) that tests the new permalink setup in a site build.

bep · 2023-06-26T13:29:58Z

Never mind the comments above. I suspect we need to test this out on some real projects and then do some adjustments/add some tests, so I might as well merge this early.

See gohugoio#8523 See gohugoio#10847

See #8523 See #10847

github-actions · 2024-06-27T01:51:32Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

Mai-Lapyst force-pushed the allow-permalinks-conf-for-branchbundles branch from 1f25526 to fb35e7f Compare March 18, 2023 18:02

Mai-Lapyst force-pushed the allow-permalinks-conf-for-branchbundles branch 3 times, most recently from 4636e32 to 2306a9f Compare March 19, 2023 04:12

jmooring mentioned this pull request Apr 19, 2023

Hugo with uglyURLs = true generates /public/section.html instead of /public/section/index.html #4428

Open

bep added this to the v0.112.0 milestone May 19, 2023

bep modified the milestones: v0.112.0, v0.113.0 May 23, 2023

Mai-Lapyst force-pushed the allow-permalinks-conf-for-branchbundles branch from 2306a9f to 9caaf86 Compare May 26, 2023 09:03

bep modified the milestones: v0.113.0, v0.115.0 Jun 13, 2023

bep requested changes Jun 26, 2023

View reviewed changes

bep merged commit cc14c6a into gohugoio:master Jun 26, 2023

This was referenced Jun 26, 2023

Add sensible fallback for permalink :token when not backed by a file #9448

Closed

disableKinds with taxonomy should be recursive (also disable term) #8498

Closed

jmooring added a commit to jmooring/hugo that referenced this pull request Jun 26, 2023

docs: Update permalinks documentation

9e0d8ee

See gohugoio#8523 See gohugoio#10847

jmooring mentioned this pull request Jun 26, 2023

docs: Update permalinks documentation #11173

Merged

bep pushed a commit that referenced this pull request Jun 27, 2023

docs: Update permalinks documentation

12e4c4d

See #8523 See #10847

Mai-Lapyst deleted the allow-permalinks-conf-for-branchbundles branch June 27, 2023 19:47

github-actions bot locked as resolved and limited conversation to collaborators Jun 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

resources/page: Allow section&taxonomy pages to have a permalink configuration #10847

resources/page: Allow section&taxonomy pages to have a permalink configuration #10847

Mai-Lapyst commented Mar 17, 2023

jmooring commented Mar 18, 2023 •

edited

Loading

Mai-Lapyst commented Mar 18, 2023 •

edited

Loading

jmooring commented Mar 19, 2023

Mai-Lapyst commented Mar 19, 2023

jmooring commented May 19, 2023

bep commented May 19, 2023

Mai-Lapyst commented May 26, 2023

bep Jun 26, 2023

bep Jun 26, 2023

bep Jun 26, 2023

bep Jun 26, 2023

bep Jun 26, 2023

bep commented Jun 26, 2023

github-actions bot commented Jun 27, 2024

		@@ -69,6 +69,7 @@ func TestPermalinkExpansion(t *testing.T) {
		page.date = d

resources/page: Allow section&taxonomy pages to have a permalink configuration #10847

resources/page: Allow section&taxonomy pages to have a permalink configuration #10847

Conversation

Mai-Lapyst commented Mar 17, 2023

jmooring commented Mar 18, 2023 • edited Loading

Mai-Lapyst commented Mar 18, 2023 • edited Loading

jmooring commented Mar 19, 2023

Mai-Lapyst commented Mar 19, 2023

jmooring commented May 19, 2023

bep commented May 19, 2023

Mai-Lapyst commented May 26, 2023

bep Jun 26, 2023

Choose a reason for hiding this comment

bep Jun 26, 2023

Choose a reason for hiding this comment

bep Jun 26, 2023

Choose a reason for hiding this comment

bep Jun 26, 2023

Choose a reason for hiding this comment

bep Jun 26, 2023

Choose a reason for hiding this comment

bep commented Jun 26, 2023

github-actions bot commented Jun 27, 2024

jmooring commented Mar 18, 2023 •

edited

Loading

Mai-Lapyst commented Mar 18, 2023 •

edited

Loading