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 #2717

Merged
merged 1 commit into from
Nov 22, 2016
Merged

Fix case issues with Params #2717

merged 1 commit into from
Nov 22, 2016

Conversation

bep
Copy link
Member

@bep bep commented Nov 22, 2016

This replaces #2630 -- but it needs some more testing before we try again. There is a inifinite loop lurking in here ...

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
Copy link
Member Author

bep commented Nov 22, 2016

There seems to be a slight performance penalty with this:

benchmark           old ns/op      new ns/op       delta
BenchmarkHugo-4     9836140771     10273595307     +4.45%

benchmark           old allocs     new allocs     delta
BenchmarkHugo-4     41718655       42182280       +1.11%

benchmark           old bytes      new bytes      delta
BenchmarkHugo-4     8887030688     8895689976     +0.10%

Will have to look into this a little.

@bep
Copy link
Member Author

bep commented Nov 22, 2016

OK, I have removed a obvious ineffeciency, and this may be livable:

benchmark           old ns/op      new ns/op       delta
BenchmarkHugo-4     9908842113     10242229016     +3.36%

benchmark           old allocs     new allocs     delta
BenchmarkHugo-4     41717487       42137686       +1.01%

benchmark           old bytes      new bytes      delta
BenchmarkHugo-4     8865691320     8841285632     -0.28%

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 22, 2016

Now it is "fast enough":

benchmark           old ns/op      new ns/op      delta
BenchmarkHugo-4     9795843285     9700004476     -0.98%

benchmark           old allocs     new allocs     delta
BenchmarkHugo-4     41717550       42134335       +1.00%

benchmark           old bytes      new bytes      delta
BenchmarkHugo-4     8870693120     8899367200     +0.32%
´``

@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
1 participant