Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misc fixes and improvements #11940

Merged
merged 4 commits into from
Jan 30, 2024
Merged

Conversation

bep
Copy link
Member

@bep bep commented Jan 29, 2024

This is 4 separate and (mostly) related commits. See the commit messages for details.

The one thing that may need some explaining is the addition of kind, path and lang as front matter keys, and the consolidation of the page configuration into this struct:

type Dates struct {
	Date        time.Time
	Lastmod     time.Time
	PublishDate time.Time
	ExpiryDate  time.Time
}

// PageConfig configures a Page, typically from front matter.
// Note that all the top level fields are reserved Hugo keywords.
// Any custom configuration needs to be set in the Params map.
type PageConfig struct {
	Dates                   // Dates holds the fource core dates for this page.
	Title          string   // The title of the page.
	LinkTitle      string   // The link title of the page.
	Type           string   // The content type of the page.
	Layout         string   // The layout to use for to render this page.
	Markup         string   // The markup used in the content file.
	Weight         int      // The weight of the page, used in sorting if set to a non-zero value.
	Kind           string   // The kind of page, e.g. "page", "section", "home" etc. This is usually derived from the content path.
	Path           string   // The canonical path to the page, e.g. /sect/mypage. Note: Leading slash, no trailing slash, no extensions or language identifiers.
	Lang           string   // The language code for this page. This is usually derived from the module mount or filename.
	Slug           string   // The slug for this page.
	Description    string   // The description for this page.
	Summary        string   // The summary for this page.
	Draft          bool     // Whether or not the content is a draft.
	Headless       bool     // Whether or not the page should be rendered.
	IsCJKLanguage  bool     // Whether or not the content is in a CJK language.
	TranslationKey string   // The translation key for this page.
	Keywords       []string // The keywords for this page.
	Aliases        []string // The aliases for this page.
	Outputs        []string // The output formats to render this page in. If not set, the site's configured output formats for this page kind will be used.

	// These build options are set in the front matter,
	// but not passed on to .Params.
	Resources []map[string]any
	Cascade   map[page.PageMatcher]maps.Params // Only relevant for branch nodes.
	Sitemap   config.SitemapConfig
	Build     BuildConfig

	// User defined params.
	Params maps.Params
}

I think the above should be complete. Note that

  1. Use cases for kind, path and lang in front matter may be rare, but this is mostly a preparation for other data formats.
  2. None of these can currently be set in cascade.
  3. I have renamed the _build to build (but both will work).

@bep bep force-pushed the feat/param-lang-path-11544 branch 2 times, most recently from 256d953 to 8526aef Compare January 30, 2024 09:48
@bep bep changed the title Add path, kind and lang to content front matter Misc fixes and improvements Jan 30, 2024
bep added 3 commits January 30, 2024 11:03
Note that none of these can be set via cascade (you will get an error)

Fixes gohugoio#11544
Also rename config `ignoreErrors` => `ignoreLogs`

But the old still works.

Closes gohugoio#9189
@bep bep force-pushed the feat/param-lang-path-11544 branch from 8526aef to 7ea2bae Compare January 30, 2024 10:03
@bep bep force-pushed the feat/param-lang-path-11544 branch from 7ea2bae to 2cb6f23 Compare January 30, 2024 10:09
@bep bep marked this pull request as ready for review January 30, 2024 10:35
@bep bep requested a review from jmooring January 30, 2024 10:35
@jmooring
Copy link
Member

jmooring commented Jan 30, 2024

None of these can currently be set in cascade.

To clarify, "these" refers to kind, path, and lang. For example, type and layout may be set in cascade.

@jmooring
Copy link
Member

jmooring commented Jan 30, 2024

For the release notes...

Anyone who has been ignoring the ".Path when the page is backed by a file is deprecated" warning for the past two years (since v0.92.0) may encounter what they consider to be a breaking change.

v0.122.0: .Page.Pathposts/post-1.md
v0.123.0: .Page.Path/posts/post-1

The value returned by .Page.File.Path (which the warning message recommends) remains the same (e.g., posts/post-1.md).

@jmooring
Copy link
Member

type Dates struct {
	Date        time.Time
	Lastmod     time.Time
	PublishDate time.Time
	ExpiryDate  time.Time
}

I was expecting that I'd be able to do {{ .Dates.Lastmod }}. Is that incorrect?

can’t evaluate field Dates in type page.Page

@bep
Copy link
Member Author

bep commented Jan 30, 2024

To clarify, "these" refers to kind, path, and lang. For example, type and layout may be set in cascade.

Yes, I was a little quick: This refers to kind, path, and lang only (the new values).

@jmooring
Copy link
Member

This is a bit contrived, but...

content/
└── posts/
    └── post-1/
        ├── index.md
        ├── my image.jpg
        └── my-image.jpg

The table below shows the value returned by .RelPermalink.

  .Resources.Get "my-image" .Resources.Get "my image"
v0.122.0 /posts/post-1/my-image.jpg /posts/post-1/my%20image.jpg
this PR /posts/post-1/my-image.jpg indeterminate

By indeterminate I mean: sometimes it finds the resource, and sometimes it does not (i.e., .Resources.Get returns nil).

FYI, there are a couple of open issues related to case-sensitivity when matching resource filenames:

@bep
Copy link
Member Author

bep commented Jan 30, 2024

This is a bit contrived, but...

I'm pretty sure that case is documented somewhere in my issues, but given this bundle:

mybundle/index.md
mybundle/Foo Bar.txt

The new canonical content paths for the two resources above is:

/mybundle
/mybundle/foo-bar.txt

WIth that, this list:

mybundle/index.md
mybundle/index.html
mybundle/Foo Bar.txt
mybundle/foo bar.txt
mybundle/foo-bar.txt

Will still only produce 1 bundle with 1 resource. We have some deduplication and ordering of content files, but for the resources, when you have duplicates, which one you end up with is undefined.

@jmooring
Copy link
Member

but for the resources, when you have duplicates, which one you end up with is undefined.

We detect page collisions (--printPathWarnings). Is there any way to detect resource collisions and warn or error, preferably without having to use a CLI flag?

What was bothersome was the inconsistent behavior from one build to the next; sometimes it finds a resource, sometimes it does not.

@bep
Copy link
Member Author

bep commented Jan 30, 2024

Is there any way to detect resource collisions and warn or error, preferably without having to use a CLI flag?

Maybe, but it's not in scope for this PR.

What was bothersome was the inconsistent behavior from one build to the next; sometimes it finds a resource, sometimes it does not.

I have never seen this issue in the wild. This very minor path normalisation is done with good intent: We already merge content from many files sources with different languages etc. Imagine when we add content from JSON, WordPress, SQL, whatnot. We need less ambiguity/complexity to prepare for ... more complexity.

@jmooring
Copy link
Member

I've tested what I can think of. My only remaining question is...

I was expecting that I'd be able to do {{ .Dates.Lastmod }}. Is that incorrect?

@bep
Copy link
Member Author

bep commented Jan 30, 2024

I was expecting that I'd be able to do {{ .Dates.Lastmod }}. Is that incorrect?

That is incorrect. The PageConfig struct is internal and not accessible directly in the templates.

@bep bep merged commit 80595bb into gohugoio:master Jan 30, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants