gohugoio / hugoDocs

The source for https://gohugo.io/
Apache License 2.0
1.06k stars 1.49k forks source link

Clarify the difference in behavior of Site vs Page GetPage #546

Closed kaushalmodi closed 1 year ago

kaushalmodi commented 6 years ago

For a path like /foo/bar/abc.md, .Site.GetPage "abc" will work (assuming that the "abc" name is unique in the whole site).

But given that the /foo page context is stored in $foo, $foo.GetPage "abc" is not expected to always? work.

The Page version of GetPage expects either an absolute path (starting with /) or a valid relative path from the current context. It is not designed to do a "lookup" anywhere in the given context.

This distinction needs to be made explicit in the docs.

bep commented 6 years ago

But given that the /foo page context is stored in $foo, $foo.GetPage "abc" is not expected to always? work.

I'm not sure I follow (and please don't ask me to read the thred on the forum):

Should return the same Page. If abc is not found relative to $foo we currently print a warning about using /abc instead. But it should be found and returned?

kaushalmodi commented 6 years ago

@bep I have committed a very light weight reproducible example at https://gitlab.com/hugo-mwe/hugodocs-issue-546. See the "But this fails!" section in the layout file.

bep commented 6 years ago

OK, then I misunderstood the problem -- "post-1" is not unique.

Maybe in that page context, as you say it, but that is not how this works.

kaushalmodi commented 6 years ago

but that is not how this works

I now realize that it's not how this works. But it's not intuitive given that .Page.GetPage works on a Page object.. it shouldn't be confused with similar named pages outside that Page object context.

So it should be explicitly documented that the arg to .Page.GetPage should be relative references directly from the input page object argument or absolute links. While .Site.GetPage style lookups work, they are not guaranteed as the "ambiguity error" is triggered from the context of the whole site.

bep commented 6 years ago

But it's not intuitive given that .Page.GetPage works on a Page object.. it shouldn't be confused with similar named pages outside that Page object context.

That argument could be had and a possible improvement (possibly expensive): But I think it helps to think about Page.GetPage as also supporting relative paths. .GetPage "mypage.md" (assuming it is in the root folder of that branch bundle) is plenty more portable than.Site.GetPage "/mybundle/mypage.md"`.

kaushalmodi commented 6 years ago

But I think it helps to think about Page.GetPage as also supporting relative paths.

I am not denying the utility of the new function. Also I don't suggest adding this feature ASAP.. (it would be awesome if the long standing bugs like Next/Previous and NextInSection/PrevInSection get love before addition of more new features :)).

I opened this issue in HugoDocs just so that this nuance of the context when calculating the ambiguity is properly documented (i.e. the .Site context is used for ambiguity detection whether .Site.GetPage or .Page.GetPage is used).

kaushalmodi commented 6 years ago

@bep Actually turns out that the "Page context" association with .Page.GetPage is completely misleading..

This works! .. even though the post-2 is not inside /posts/bar at all:

├── content/
│  └── posts/
│     ├── bar/
│     │  ├── _index.md
│     │  └── x/
│     │     ├── _index.md
│     │     └── post-1.md
│     └── foo/
│        ├── _index.md
│        └── x/
│           ├── _index.md
│           ├── post-1.md
│           └── post-2.md
        {{ with .Site.GetPage "/posts/bar" }}
            {{ with .GetPage "post-2" }}
                {{ printf "Title of a post-2 post anywhere in Site: %s" .Title }}<br />
            {{ end }}
        {{ end }}

So.. for $page.GetPage "<path>":

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. The resources of the Hugo team are limited, and so we are asking for your help. If you still think this is important, please tell us why. This issue will automatically be closed in the near future if no further activity occurs. Thank you for all your contributions.