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.

Sign up for GitHub

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

Jump to bottom

Make .Site.GetPage work with 1 parameter #4652

Closed
bep opened this issue Apr 20, 2018 · 6 comments
Closed

Make .Site.GetPage work with 1 parameter #4652

bep opened this issue Apr 20, 2018 · 6 comments
Labels
Enhancement Outdated
Milestone
v0.45

Comments

@bep
Copy link
Member

bep commented Apr 20, 2018

Not sure why I insisted on the kind as a first param ... I don't see any ambiguous cases, and it should be possible to do in a non-breaking way, I think ...
@bep bep added this to the v0.40 milestone Apr 20, 2018
@kaushalmodi
Copy link
Contributor

What is your use case for this?

This will just confuse the users and more so add a maintenance burden on your shoulders.

At the moment, the syntax is quite simple, first the Kind, then the Page name.

If the first argument becomes (Kind or Page name), helping people understand that and debug that will be a pain.

Also the current syntax where the Kind is mandated is very clean.. someone reading the code knows instantly of the section page is getting fetched or the "page" page, or ..

@kaushalmodi
Copy link
Contributor

If the motivation is that people don't have to understand Page Kinds, then that would be bad because knowing the Kind is useful in many places like output formats, and mainly grasping the layout lookup order.

@bep
Copy link
Member Author

bep commented Apr 20, 2018

I was writing some test-shortcodes for my work on the shortcodes, and if you want to write a generic "get-page-content" shortcode ... you either have to know what it is or try several.

So: {{< get-page-content "section" "foo/mydirectory" >}} would fail for no good reason if I change that dir into a ... leaf bundle.

@bep bep added Proposal and removed Enhancement labels Apr 20, 2018
@bep
Copy link
Member Author

bep commented Apr 20, 2018

But I'm not totally convinced...

@bep bep modified the milestones: v0.40, v0.41 Apr 20, 2018
@bep bep modified the milestones: v0.41, v0.42 May 4, 2018
@kaushalmodi kaushalmodi mentioned this issue May 15, 2018
Closed
@vassudanagunta
Copy link
Contributor

vassudanagunta commented May 15, 2018 •
edited
Loading

@bep, I'm still a Go newb, but I don't think it is possible to remove the typ param from:

func (s *SiteInfo) GetPage(typ string, path ...string)`

because the first param ("page", "section", etc) in existing calls to .Site.GetPage would then come across as part of the variadic path arg, right?

I was going to propose a combined solution to this and #4728 by adding .GetPage (i.e. method on Page) and deprecating .Site.GetPage. The new method would be able to support relative paths since it would have a Page context.

func (p *Page) GetPage(path ...string)

Though I was wondering if the new method can dispense support for passing the path broken up into separate strings:

func (p *Page) GetPage(path string)

The nice thing about this is that .GetPage would take the same arg and have the same semantics as the ref/relref shortcodes. Consistency makes for a less confusing API.

@vassudanagunta vassudanagunta mentioned this issue May 18, 2018
Closed
@vassudanagunta vassudanagunta mentioned this issue May 30, 2018
Closed
@bep bep modified the milestones: v0.42, v0.43 Jun 5, 2018
@bep bep modified the milestones: v0.43, v0.44 Jun 30, 2018
@bep bep modified the milestones: v0.44, v0.45 Jul 10, 2018
bep pushed a commit to bep/hugo that referenced this issue Jul 17, 2018
@vassudanagunta @bep
Unify page look ups
930e320 
This commit unifies the core internal page index for all page kinds.

This enables the `ref` and `relref` shortcodes to support all pages kinds, and adds a new page-relative  `.GetPage` method with simplified signature.

Fixes gohugoio#4147
Fixes gohugoio#4727
Fixes gohugoio#4728
Fixes gohugoio#4728
Fixes gohugoio#4726
Fixes gohugoio#4652
bep pushed a commit to bep/hugo that referenced this issue Jul 17, 2018
@vassudanagunta @bep
Unify page lookups
058f20e 
This commit unifies the core internal page index for all page kinds.

This enables the `ref` and `relref` shortcodes to support all pages kinds, and adds a new page-relative  `.GetPage` method with simplified signature.

Fixes gohugoio#4147
Fixes gohugoio#4727
Fixes gohugoio#4728
Fixes gohugoio#4728
Fixes gohugoio#4726
Fixes gohugoio#4652
@bep bep modified the milestones: v0.45, v0.46 Jul 17, 2018
@bep bep added Enhancement and removed Proposal labels Jul 17, 2018
@bep bep modified the milestones: v0.46, v0.45 Jul 17, 2018
bep pushed a commit to bep/hugo that referenced this issue Jul 17, 2018
@vassudanagunta @bep
Unify page lookups
2d9da6e 
This commit unifies the core internal page index for all page kinds.

This enables the `ref` and `relref` shortcodes to support all pages kinds, and adds a new page-relative  `.GetPage` method with simplified signature.

See gohugoio#4147
See gohugoio#4727
See gohugoio#4728
See gohugoio#4728
See gohugoio#4726
See gohugoio#4652
bep added a commit to bep/hugo that referenced this issue Jul 17, 2018
@bep
Simplify .Site.GetPage etc.
a07eb71 
This commit is a follow up to a recent overhaul of the GetPage/ref/relref implemenation.

The most important change in this commit is the update to `.Site.GetPage`:

* To reduce the amount of breakage in the wild to its minimum, I have reworked .Site.GetPage with some rules:

* We cannot support more than 2 arguments, i.e. .Site.GetPage "page" "posts" "mypage.md" will now throw an error. I think this is the most uncommon syntax and should be OK. It is an easy fix to change the above to .Site.GetPage "/posts/mypage.md" or similar.
* .Site.GetPage "home", .Site.GetPage "home" "" and .Site.GetPage "home" "/" will give you the home page. This means that if you have page in root with the name home.md you need to do .Site.GetPage "/home.md" or similar

This commit also fixes some multilingual issues, most notable it is now possible to do cross-language ref/relref lookups by prepending the language code to the path, e.g. `/jp/posts/mypage.md`.

This commit also reverts the site building tests related to this to "Hugo 0.44 state", to get better control of the changes made.

Closes gohugoio#4147
Closes gohugoio#4727
Closes gohugoio#4728
Closes gohugoio#4728
Closes gohugoio#4726
Closes gohugoio#4652
bep added a commit to bep/hugo that referenced this issue Jul 17, 2018
@bep
Simplify .Site.GetPage etc.
e4de990 
This commit is a follow up to a recent overhaul of the GetPage/ref/relref implemenation.

The most important change in this commit is the update to `.Site.GetPage`:

* To reduce the amount of breakage in the wild to its minimum, I have reworked .Site.GetPage with some rules:

* We cannot support more than 2 arguments, i.e. .Site.GetPage "page" "posts" "mypage.md" will now throw an error. I think this is the most uncommon syntax and should be OK. It is an easy fix to change the above to .Site.GetPage "/posts/mypage.md" or similar.
* .Site.GetPage "home", .Site.GetPage "home" "" and .Site.GetPage "home" "/" will give you the home page. This means that if you have page in root with the name home.md you need to do .Site.GetPage "/home.md" or similar

This commit also fixes some multilingual issues, most notable it is now possible to do cross-language ref/relref lookups by prepending the language code to the path, e.g. `/jp/posts/mypage.md`.

This commit also reverts the site building tests related to this to "Hugo 0.44 state", to get better control of the changes made.

Closes gohugoio#4147
Closes gohugoio#4727
Closes gohugoio#4728
Closes gohugoio#4728
Closes gohugoio#4726
Closes gohugoio#4652
bep added a commit to bep/hugo that referenced this issue Jul 17, 2018
@bep
Simplify .Site.GetPage etc.
2701749 
This commit is a follow up to a recent overhaul of the GetPage/ref/relref implemenation.

The most important change in this commit is the update to `.Site.GetPage`:

* To reduce the amount of breakage in the wild to its minimum, I have reworked .Site.GetPage with some rules:

* We cannot support more than 2 arguments, i.e. .Site.GetPage "page" "posts" "mypage.md" will now throw an error. I think this is the most uncommon syntax and should be OK. It is an easy fix to change the above to .Site.GetPage "/posts/mypage.md" or similar.
* .Site.GetPage "home", .Site.GetPage "home" "" and .Site.GetPage "home" "/" will give you the home page. This means that if you have page in root with the name home.md you need to do .Site.GetPage "/home.md" or similar

This commit also fixes some multilingual issues, most notable it is now possible to do cross-language ref/relref lookups by prepending the language code to the path, e.g. `/jp/posts/mypage.md`.

This commit also reverts the site building tests related to this to "Hugo 0.44 state", to get better control of the changes made.

Closes gohugoio#4147
Closes gohugoio#4727
Closes gohugoio#4728
Closes gohugoio#4728
Closes gohugoio#4726
Closes gohugoio#4652
bep added a commit to bep/hugo that referenced this issue Jul 17, 2018
@bep
Simplify .Site.GetPage etc.
c48074b 
This commit is a follow up to a recent overhaul of the GetPage/ref/relref implemenation.

The most important change in this commit is the update to `.Site.GetPage`:

* To reduce the amount of breakage in the wild to its minimum, I have reworked .Site.GetPage with some rules:

* We cannot support more than 2 arguments, i.e. .Site.GetPage "page" "posts" "mypage.md" will now throw an error. I think this is the most uncommon syntax and should be OK. It is an easy fix to change the above to .Site.GetPage "/posts/mypage.md" or similar.
* .Site.GetPage "home", .Site.GetPage "home" "" and .Site.GetPage "home" "/" will give you the home page. This means that if you have page in root with the name home.md you need to do .Site.GetPage "/home.md" or similar

This commit also fixes some multilingual issues, most notable it is now possible to do cross-language ref/relref lookups by prepending the language code to the path, e.g. `/jp/posts/mypage.md`.

This commit also reverts the site building tests related to this to "Hugo 0.44 state", to get better control of the changes made.

Closes gohugoio#4147
Closes gohugoio#4727
Closes gohugoio#4728
Closes gohugoio#4728
Closes gohugoio#4726
Closes gohugoio#4652
bep pushed a commit that referenced this issue Jul 17, 2018
@vassudanagunta @bep
Unify page lookups
b93417a 
This commit unifies the core internal page index for all page kinds.

This enables the `ref` and `relref` shortcodes to support all pages kinds, and adds a new page-relative  `.GetPage` method with simplified signature.

See #4147
See #4727
See #4728
See #4728
See #4726
See #4652
@bep bep closed this as completed in 3eb313f Jul 17, 2018
@dependencies dependencies bot mentioned this issue Jul 22, 2018
Closed
@github-actions
Copy link

github-actions bot commented Mar 1, 2022

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

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Enhancement Outdated
Projects
None yet
Milestone
v0.45
Development

No branches or pull requests
3 participants
@bep @vassudanagunta @kaushalmodi