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

Deprecate Page.IsNode #11574

Open
jmooring opened this issue Oct 18, 2023 · 6 comments
Open

Deprecate Page.IsNode #11574

jmooring opened this issue Oct 18, 2023 · 6 comments

Comments

@jmooring
Copy link
Member

Why?

  • It's meaning is unclear
  • It's superfluous (use if not .IsPage instead)
  • Not sure anyone actually uses it
@onedrawingperday
Copy link
Contributor

onedrawingperday commented Oct 18, 2023

Not in favour of this proposal as I am using it in my templating.

It is an older variable and there are still projects from 5 years ago that make use of it.

Please don’t make breaking changes because some things seem unclear.

@bep bep removed the NeedsTriage label Oct 18, 2023
@bep bep added this to the v0.120.0 milestone Oct 18, 2023
@bep
Copy link
Member

bep commented Oct 18, 2023

This needs to at least wait (and in any case, we can limit the change to the documentation to start with).

@bep bep modified the milestones: v0.120.0, v0.121.0 Oct 31, 2023
@jmooring
Copy link
Member Author

Updated documentation.

Copy link

github-actions bot commented Dec 3, 2023

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 Dec 3, 2023
@jmooring
Copy link
Member Author

jmooring commented Mar 20, 2024

I'm reopening this because we recently introduced the concept of logical paths, and the easiest way to describe a logical path is the path between two nodes on a node tree, either relative to each other or absolute from the root of the tree.

In a data structure, the term "node" is never limited to a parent.
https://en.wikipedia.org/wiki/Tree_(data_structure)

But our existing definition of node is limited to page kinds home, section, taxonomy, term, 404, and maybe some others, but the definition excludes page kind page.

I would like to deprecate .Page.IsNode recommending the use of not .IsPage instead. .IsPage returns false for page kinds home, section, taxonomy, term, 404, and maybe some others.

This is not a technical issue... .IsNode works fine. This is about the language of Hugo, the opportunity to clarify concepts in documentation, and to use terminology that is generally accepted.


Alternatively we could rename .Page.IsNode (and how we name it internally) to .Page.IsList per:
https://github.com/gohugoio/hugo/blob/master/resources/page/page.go#L214

IsNode returns whether this is an item of one of the list types in Hugo,

@jmooring jmooring reopened this Mar 20, 2024
@jmooring
Copy link
Member Author

jmooring commented Apr 8, 2024

@bep Please comment when you have a moment. Thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants