-
-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
tpl: Add float template function #3921
Conversation
@@ -30,6 +30,22 @@ type Namespace struct { | |||
|
|||
// ToInt converts the given value to an int. | |||
func (ns *Namespace) ToInt(v interface{}) (int, error) { | |||
v = convertTemplateToString(v) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't change exisiting methods. This doesn't look correct and I'm surprised there were not red tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't change exisiting methods
Why not? Should I just copy-and-paste the code that was used in toInt? I just took what was there before (the switch-case for the template types) and moved it into a separate helper function.
v interface{} | ||
expect interface{} | ||
}{ | ||
{"1", 1.0}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tests assumes that the input is strings only. I suspect your implementation will fail on 2, int64(2), float64(2)
etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've pushed additional additional test cases, they all pass.
PS: Never mind me, there seems to be a difference between running make check
and go test
in that directory. Investigating
PPS: I just commited a typo, nvm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, I see now -- I read your original code slightly wrong, but it helped with the test cases.
e995f75
to
2ed01bf
Compare
Add a template function that allows conversion to float. This is useful, for example, when passing aspect ratios into templates, which tend to not be integers. Fixes gohugoio#3307
2ed01bf
to
ef0463c
Compare
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. |
Add a template function that allows conversion to float. This is
useful, for example, when passing aspect ratios into templates,
which tend to not be integers.
Fixes #3307
Feedback very welcome, this is the first time I've tampered with the hugo source 🎉