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

Consider making .File on .Page a zero value instead of nil #5781

Closed
bep opened this issue Mar 23, 2019 · 3 comments · Fixed by #5791
Closed

Consider making .File on .Page a zero value instead of nil #5781

bep opened this issue Mar 23, 2019 · 3 comments · Fixed by #5791
Assignees
Labels
Milestone

Comments

@bep
Copy link
Member

bep commented Mar 23, 2019

  • It's now a method that returns an interface
  • It's nil if no set
  • The embedded .File is deprecated so you need to say .File.Filename instead of .Filename (you will get a deprecation warning for the latter)
  • If you do .File.Filename on a Page without a File you will get a nilpointer (a nice error message pointing to the location in Go 1.12.1
  • With the latest work on if/with we could make it a zero value work better and create less noise in the wild, but I tend to think that it would be better for people to know that the File isn't there (this is what I see in the wild; I think getting the nilpointer will help fix actual bugs in the templates around where people assumes that every Page is backed by a file)

I create this issue as a "thinking issue". What do you think, @moorereason ?

@bep bep added this to the v0.55 milestone Mar 23, 2019
@bep
Copy link
Member Author

bep commented Mar 25, 2019

I have thought a little about this, and I suspect the current solution will create more noise than we'd care for. So I will do this:

  • Return a zero value when no file is set that is a proxy that prints a generic warning about wrapping the code in {{ with .File ...}}
  • That way we avoid the bulk of the noise and site breakage, but help people fix what in most cases is a logical bug

/cc @onedrawingperday

@bep bep self-assigned this Mar 25, 2019
@onedrawingperday
Copy link
Contributor

This looks like a great plan!

Once you make a commit let me know and I will take it for a spin.

bep added a commit to bep/hugo that referenced this issue Mar 25, 2019
bep added a commit to bep/hugo that referenced this issue Mar 25, 2019
bep added a commit to bep/hugo that referenced this issue Mar 25, 2019
bep added a commit to bep/hugo that referenced this issue Mar 25, 2019
@bep bep closed this as completed in #5791 Mar 26, 2019
bep added a commit that referenced this issue Mar 26, 2019
nguyenvanduocit pushed a commit to 12bitvn/hugo that referenced this issue Apr 5, 2019
@github-actions
Copy link

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 Feb 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants