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

Fix case issues with Params #2630

Merged
merged 1 commit into from Nov 22, 2016
Merged

Fix case issues with Params #2630

merged 1 commit into from Nov 22, 2016

Conversation

bep
Copy link
Member

@bep bep commented Oct 24, 2016

There are currently several Params and case related issues floating around in Hugo.

This is very confusing for users and one of the most common support questions on the forum.

And while there have been done some great leg work in Viper etc., this is of limited value since this and similar doesn't work:

Params.myCamelCasedParam

Hugo has control over all the template method invocations, and can take care of all the lower-casing of the map lookup keys.

But that doesn't help with direct template lookups of type Site.Params.TWITTER_CONFIG.USER_ID.

This commit solves that by doing some carefully crafted modifications of the templates' AST -- lowercasing the params keys.

This is low-level work, but it's not like the template API wil change -- and this is important enough to defend such "bit fiddling".

Tests are added for all the template engines: Go templates, Ace and Amber.

Fixes #2615
Fixes #1129
Fixes #2590

@bep bep force-pushed the fix-caseins branch 24 times, most recently from b5d4d5e to 48269a1 Compare October 27, 2016 19:28
@bep bep changed the title WIP: Fix case issues Fix case issues with Params Oct 27, 2016
@bep
Copy link
Member Author

bep commented Oct 27, 2016

/cc @moonytheloony @spf13 @tatsushid and friends.

@bep bep force-pushed the fix-caseins branch 2 times, most recently from 1ef5828 to 3eda2e7 Compare October 27, 2016 19:58
@rahulrai-in
Copy link
Contributor

Will this commit enforce that we can't have 2 variables with the same name but different casing? e.g. someVariable, somevariable and SomeVariable would ultimately mean the same thing?

@bep
Copy link
Member Author

bep commented Oct 27, 2016

Will this commit enforce that we can't have 2 variables with the same name but different casing?

The commit will not change anything in that department: The Params keys are case insensitive, i.e. Foo is the same as foo. This is how it is already, but if you ask for Foo, you don't get it. After this commit, you do.

@bep
Copy link
Member Author

bep commented Nov 4, 2016

@spf13 you out there?

@bep
Copy link
Member Author

bep commented Nov 21, 2016

No comments on this PR?

@moorereason
Copy link
Contributor

Interesting approach. I don't see anything wrong with it and can confirm that it fixes Params issues I've seen since 0.17.

LGTM 👍

PS - I was daydreaming the other day about having a go fix-type hugo sub-command that would rewrite template/layout ASTs to update deprecated features.

There are currently several Params and case related issues floating around in Hugo.

This is very confusing for users and one of the most common support questions on the forum.

And while there have been done some great leg work in Viper etc., this is of limited value since this and similar doesn't work:

`Params.myCamelCasedParam`

Hugo has control over all the template method invocations, and can take care of all the lower-casing of the map lookup keys.

But that doesn't help with direct template lookups of type `Site.Params.TWITTER_CONFIG.USER_ID`.

This commit solves that by doing some carefully crafted modifications of the templates' AST -- lowercasing the params keys.

This is low-level work, but it's not like the template API wil change -- and this is important enough to defend such "bit fiddling".

Tests are added for all the template engines: Go templates, Ace and Amber.

Fixes gohugoio#2615
Fixes gohugoio#1129
Fixes gohugoio#2590
@bep
Copy link
Member Author

bep commented Nov 27, 2016

@mdhender this is a PR. If you have an issue with params, please create a new issue for it.

@bep bep deleted the fix-caseins branch April 18, 2017 09:19
@github-actions
Copy link

This pull request 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 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants