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

Avoid warning 'calling IsSet with unsupported type "invalid" (<nil>)' #1465

Merged
merged 3 commits into from
Mar 8, 2023

Conversation

deining
Copy link
Collaborator

@deining deining commented Mar 8, 2023

This PR addresses #1464.

As pointed out in #608 already, the root cause of the warning is the wrong use of isset.
This PR replaces isset and uses with clauses instead, thus avoiding the confusing warn messages.

This PR closes #1464.

Copy link
Collaborator

@chalin chalin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking into this.

There's a simpler way to resolve such expressions, for example:

{{ $sidebarCacheLimit := and .Site.Params.ui .Site.Params.ui.sidebar_cache_limit | default 2000 -}}

It's might be a good idea to also include a test of .Site.Params since that section is absent when, for example, hugo creates a new site.

{{ $sidebarCacheLimit := and .Site.Params .Site.Params.ui .Site.Params.ui.sidebar_cache_limit | default 2000 -}}

Could you change all assignment expressions to be of this form?

@deining
Copy link
Collaborator Author

deining commented Mar 8, 2023

Thanks for looking into this.

Thanks for reviewing this promptly.

There's a simpler way to resolve such expressions, for example:

{{ $sidebarCacheLimit := and .Site.Params.ui .Site.Params.ui.sidebar_cache_limit | default 2000 -}}

Great idea. I don't see, however, why we additionally should evaluate .Site.Params.ui in this context. .Site.Params.ui is a map, so if .Site.Params.ui.sidebar_cache_limit is set, .Site.Params.ui is set, too. I think we can safely reduce this to:

{{ $sidebarCacheLimit := .Site.Params.ui.sidebar_cache_limit | default 2000 -}}

This is also what the official documentation suggests.

Could you change all assignment expressions to be of this form?

Done.

Meanwhile, I fixed a few unwanted warnings with taxonomies, too.

@deining deining requested a review from chalin March 8, 2023 14:02
@chalin
Copy link
Collaborator

chalin commented Mar 8, 2023

Have you tested your solution with the params.ui section of the Hugo config commented out?

@deining
Copy link
Collaborator Author

deining commented Mar 8, 2023

Have you tested your solution with the params.ui section of the Hugo config commented out?

Yes, I did, an it returned the default value, as expected.

Copy link
Collaborator

@chalin chalin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange. It is as if Hugo's . is acting as a safe selector (like .? in some other languages). I don't think it was always like that, but I'm happy that it works now. Thanks for fixing this.

@chalin
Copy link
Collaborator

chalin commented Mar 8, 2023

It might be worth fixing the other occurrences in the use of "isset"?

@chalin chalin merged commit 01eafd3 into google:main Mar 8, 2023
@deining deining deleted the warning-isset branch March 9, 2023 07:31
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.

Confusing warning 'calling IsSet with unsupported type "invalid" (<nil>)'
2 participants