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

Add Page.Markup with scope support #12759

Merged
merged 1 commit into from
Aug 29, 2024
Merged

Conversation

bep
Copy link
Member

@bep bep commented Aug 13, 2024

Edit in 2024-08-16. I have revised my take on this after talking to @jmooring and thinking a little. The (original) primary motivation behind this PR (for me, anyway) was to make it possible to render different versions of a given page's content on, say, the home page. But we also want to make room for further improvements in this area without blowing the scope of this PR out of proportions. I have always wanted to create a map of the rendered content, but that would only be practical for Goldmark (and hard even there, I guess). I think that and similar extensions should be fairly simple to fit in the below.

  • I have added ContentWithoutSummary, which I know many have asked for
  • I have reduced the amount of name changes; FuzzyWordCount has been around for a decade and has become a brand by now.
  • The content Summary type will have a Type discriminator that is either "auto" or "manual" or "frontmatter"
  • For now, I suggest we keep all old content methods as aliases on Page, e.g. .Content, .WordCount etc.

With this you can do:

{{ with .Markup }}
  {{ with .Render }}
    {{ .Content }} 
    {{ .WordCount }} // ... etc.
  {{ end }}
{{ end }}

You can also give it its own scope:

{{ range site.RegularPages | first 20 }}
  {{ with .Markup "home" }}
    {{ .Render.Content }}
  {{ end }}
{{ end }}
type Markup interface {
	Render(context.Context) (Content, error)
	RenderString(ctx context.Context, args ...any) (template.HTML, error)
	RenderShortcodes(context.Context) (template.HTML, error)
	Fragments(context.Context) *tableofcontents.Fragments
}

type Content interface {
        Content(context.Context) template.HTML
	ContentWithoutSummary(context.Context) template.HTML
	Summary(context.Context) Summary
	Plain(context.Context) string
	PlainWords(context.Context) []string
	WordCount(context.Context) int
	FuzzyWordCount(context.Context) int
	ReadingTime(context.Context) int
	Len(context.Context) int
}


type Summary struct {
	Text      template.HTML
	Type      string // "auto" or "manual".
	Truncated bool
}

Fixes #8680

@bep
Copy link
Member Author

bep commented Aug 14, 2024

Looking at the above, I like most things about it; we greatly reduce the amount of methods on the giga Page interface (making it easier to understand, I think) and the introduction of "scoped" content rendering can solve many hard-to-workaround issues reported by users.

But I don't like the main name ...

{{ .Contents.Render }}

The above does not read well, and I think .Contents is confusingly too similar with what we had. So, instead of trying to find a term that describes what this is I thought we could find a term that describes what this is about, and also use the same term we use about this in the configuration:

{{ .Markup.Render }}

Or ...

{{ with .Markup }}
   {{ .Render }}
{{ end }}

Which is what we do, we render the markup (Markdown, HTML, Pandoc ...) to one or more output formats.

Also, if we could somehow get the scope key above into the markup configuration, it would be useful for people to have some custom config for e.g. rendering of pages on the home page.

/cc @jmooring

@bep bep mentioned this pull request Aug 14, 2024
@bep bep force-pushed the feat/contents-8680 branch 2 times, most recently from 3d65276 to 412e1e3 Compare August 14, 2024 08:37
@bep bep changed the title Add Page.Contents with scope support Add Page.Markup with scope support Aug 14, 2024
@bep bep force-pushed the feat/contents-8680 branch 8 times, most recently from 6005ca2 to 92178b4 Compare August 18, 2024 10:49
@jmooring
Copy link
Member

I've tested this a bit. It works as expected except for ContentWithoutSummary which is expected at this point.

I did not play with the scope as described above because:

  1. I don't understand what it does
  2. I don't understand where I would want to use it

Both of these are documentation issues.

And while I appreciate grouping related items, I believe most users would prefer to do this:

{{ .Content }}
{{ .WordCount }}

Instead of this:

{{ .Markup.Render.Content }}
{{ .Markup.Render.WordCount }}

I also think the former is easier to understand.

So your intention to leave existing methods as-is makes sense to me.

@bep bep force-pushed the feat/contents-8680 branch 11 times, most recently from 7b9a8b5 to cbf0dfc Compare August 29, 2024 09:58
@bep
Copy link
Member Author

bep commented Aug 29, 2024

@jmooring I have now pushed a version that passes all tests, including ContentWithoutSummary.

I don't understand what it does

So, scope is a little specialised, but questions related to this pops up on the forum (and here) from time to time. From the top of my head I can mention 2:

  • There was one user asking how to render some post's .Content with different h2 etc. levels on the home page. He tried passing down some .Store.Set "isHome" true, but that obviously doesn't work because of all the caching. (https://discourse.gohugo.io/t/render-headings-differently-on-home-page/51136/5?u=bep)
  • There was one user asking how he could hide all images when rendering content in RSS. I told him to add an empty render hook (or shortcode) template for RSS, which works, but is rather cumbersome ....

There are others and better examples, I'm sure, but those are recent ones.

So, when doing something like this in the home page template:

{{ range site.RegularPages | first 20 }}
  {{ with .Markup "home" }}
    {{ .Render.Content }}
  {{ end }}
{{ end }}

The home page will get a fresh render of all of those 20 pages (home acts as a cache key); any shortcode/render hook template used doing that will have page.IsHome == true and hugo.MarkupScope == home.

There's a performance cost to the above, of course, so it should be ... used with care.

@bep bep force-pushed the feat/contents-8680 branch 3 times, most recently from 12f294d to 462f9e0 Compare August 29, 2024 10:38
@bep
Copy link
Member Author

bep commented Aug 29, 2024

And while I appreciate grouping related items, I believe most users would prefer to do this:

I will keep these top level Page content methods for now, but at some point it may be a question about the cost of maintaining these in relation to what "users prefer". The motivation behind this grouping isn't primarily cosmetic.

@bep bep force-pushed the feat/contents-8680 branch 3 times, most recently from f4d12ba to b030963 Compare August 29, 2024 13:53
Note that this also adds a new `.ContentWithoutSummary` method, and to do that we had to unify the different summary types:

Both `auto` and `manual` now returns HTML. Before this commit, `auto` would return plain text. This could be considered to be a slightly breaking change, but for the better: Now you can treat the `.Summary` the same without thinking about where it comes from, and if you want plain text, pipe it into `{{ .Summary | plainify }}`.

Fixes gohugoio#8680
Fixes gohugoio#12761
Fixes gohugoio#12778
Fixes gohugoio#716
@bep bep marked this pull request as ready for review August 29, 2024 14:43
@bep bep merged commit 3760926 into gohugoio:master Aug 29, 2024
6 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.

Add .Page.Contents
2 participants