resources/page: Add :sectionslug and :sectionslugs permalink tokens #13908
Conversation
|
|
Add slugified section permalink tokens with fallback behavior and slice syntax support. Includes integration tests and documentation updates. Fixes gohugoio#13788 gofmt refactor comments errs gofmt
DustinFischer
force-pushed
the
feat/sectionslugs-permalink-13788
branch
from
938aa30 to
df23ab5
Compare
resources/page/permalinks.go
Outdated
| return "", nil | ||
| } | ||
|
|
||
| sectionPage, err := p.GetPage("/" + p.Section()) |
There was a problem hiding this comment.
p.Section() returns the first level (that method was added before we had a concept of nested section), which is, I'm guessing, not what we want here. Also, I don't see why this is needed -- isn't p the section you want the title from?
There was a problem hiding this comment.
The intention with this change was to keep the :sectionslug token symmetrical with the :section token which puts the first level section in the path (here's a related discussion from the original forum post).
For example, if we had content structure:
content/a/_index.mdwithslug: section-a-slugcontent/a/b/myarticle.mdwithmy-article-slug
If p is the Page for a/b/myarticle.md then the expected output for the following url formats would be :
/:section/:slug->/a/my-article-slug/:sectionslug/:slug->/section-a-slug/my-article-slug
There was a problem hiding this comment.
Yea, OK, i read this a little fast then. But then I suggest we do sectionPage := p.FirstSection.
There was a problem hiding this comment.
Yes that's a good point. I'll make the change.
resources/page/permalinks.go
Outdated
| var entries []string | ||
| sectionEntries := p.CurrentSection().SectionsEntries() | ||
|
|
||
| for i := range sectionEntries { |
There was a problem hiding this comment.
While this would work, it's much more effective (and simpler) to just do for _, section := range p.Ancestors().Reverse() {}
There was a problem hiding this comment.
I had considered and tried this approach initially. The issue was that p.Ancestors() includes the home page if present, so the path element for the home page would be included in the published site structure as well.
I tried to keep the behaviour of :sectionslugsas similar as possible to :sections which never includes the home page in the path. I suppose I could add a check on p.IsSection() to filter out the home page as well? In that scenario would it be necessary to look at performance considerations? It seemed to me like the GetPage() approach would require fewer tree lookup operations compared to using Ancestors() combined with IsSection().
There was a problem hiding this comment.
I'm pretty p.Ancestors() is at least as performant as using GetPage, and it has the upside of making the code much simpler, which is always a good thing. I suggest you check for .IsHome and break.
There was a problem hiding this comment.
Sure. I'm happy to make this change. I agree it will simplify code and improve readability.
…lementation Improves the permalink token implementation based on PR feedback: - Use FirstSection() instead of GetPage lookup for :sectionslug - Use Ancestors().Reverse() for efficient section hierarchy building in :sectionslugs - Update tests to use proper page structure with ancestors instead of mock functions - Implement missing IsHome(), IsSection(), Ancestors() methods in test helpers
|
@bep I've made the changes you suggested:
|
|
This looks great, thanks! I will merge once the test goes green. A side note: I plan on doing a Hugo release on Monday, mostly to get a Hugo release with Go 1.25 out there, but this new feature will also be very useful to many. |
|
Thanks @bep, that's great news! And thanks for the considerate review and feedback. Hoping that it will be a useful enhancement to many users. I'll keep an eye out for the release today. |
4 participants
Add new permalink tokens allowing slugified sections in the URL path, with fallback behavior and slice syntax support.
:sectionslug: the content's section using the section'sslugfrontmatter parameter, else falling back to itstitleparameter.:sectionslugs: the content's sections hierarchy, using each section's respectiveslugfrontmatter parameter, else falling back to itstitleparameter.Includes integration tests and documentation updates.
Closes #13788