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

tpl/lang: Add NumFmt function #2851

Merged
merged 1 commit into from
May 18, 2017
Merged

tpl/lang: Add NumFmt function #2851

merged 1 commit into from
May 18, 2017

Conversation

moorereason
Copy link
Contributor

numFormat formats a number with a given precision using the requested decimal,
grouping, and negative characters.

Fixes #1444

@moorereason
Copy link
Contributor Author

Replaces #1450.

This should be simpler than the previous attempt. Formats the number using strconv and then inserts/replaces characters where needed. Should be easier to maintain.

@moorereason moorereason added this to the v0.19 milestone Dec 29, 2016
@bep bep removed this from the v0.19 milestone Feb 21, 2017
@bep bep added this to the v0.21 milestone Apr 26, 2017
@bep
Copy link
Member

bep commented Apr 26, 2017

@moorereason if you rebase this, I will merge it. I see many people ask for it, and once we get some aliasing and namespaces in order, I'm not too worried about having "many" funcs. But double check the arg order (and example) so it fits nicely into a pipe.

@moorereason
Copy link
Contributor Author

Added a pipe example and rebased.

{{ numFormat "," "." "-" 2 12345.6789 }} → 12.345,68
{{ numFormat "." "" "-" 6 -12345.6789 }} → -12345.678900
{{ numFormat "." "," "-" 0 -12345.6789 }} → -12,346
{{ -12345.6789 | numFormat "." "," "-" 2 }} → -12,346.68
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the result be -12,345.68

Copy link
Member

Choose a reason for hiding this comment

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

While you're in that file, you may also specify what rounding rule used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doh! typo

@moorereason
Copy link
Contributor Author

Pushed a new commit. Please review.

@moorereason moorereason changed the title tpl: Add numFormat func tpl/lang: Add NumFmt function Apr 30, 2017
@moorereason
Copy link
Contributor Author

@bep, we can merge this after #3415, but I made some changes from my original PR:

  • Moved this function to the lang namespace since this is really a localization concern.
  • Removed shorthand numFormat from funcMap. Can only access via namespace.
  • Renamed from numFormat to NumFmt.

@moorereason
Copy link
Contributor Author

@bep, this is ready for review.

So, with precision set to 0, 1.5 becomes `2`, and 1.4 becomes `1`.

```
{{ lang.NumFmt "," "." "-" 2 12345.6789 }} → 12.345,68
Copy link
Member

Choose a reason for hiding this comment

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

Thinking about this, the signature looks kind of klunky for a template func.

  • I kind of want to put the format into my site config, so people can set their own.
  • I doubt this will work: myNumFormat = [",", ".", "-"]
  • Suspect this will be better and then use strings.Fields: myNumFormat = ", . _"

Other than that it looks fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good thoughts. How about we build it with the intention that it will work with l10n later. Like i18n, the options are defined in the site config.

func NumFmt(precision int, n float64, opts ...interface{}) string
// Assume that they've defined locale number formatting in the site config.
{{ lang.NumFmt 2 123.456 }}
{{ 123.456 | lang.NumFmt 2 }}

// Given multiple locales defined in config, allow to specify which one to use.
{{ lang.NumFmt 2 123.456 "loc:en_US" }}

// Allow use without a locale definition?
{{ lang.NumFmt 2 123.456 "dec:." "grp:," "neg:-" }}

We don't have to support all of those options, but the func signature would allow us flexibility in the implementation as we move toward l10n.

If we need to think through l10n before merging this, I'm okay with that.

Copy link
Member

Choose a reason for hiding this comment

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

We have no concept of locale or "locale formatting". What you are saying is that we should wait until we see how that looks like before we revisit this one.

Copy link
Member

Choose a reason for hiding this comment

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

But I think we should just drop the locale debate, and just pack the three arguments into one string. It should be possible to do some magic locale stuff later if needed.

NumFmt formats a number with a given precision using the requested
decimal, grouping, and negative characters.

Fixes gohugoio#1444
@moorereason
Copy link
Contributor Author

@bep, how's this look? I still need to add site config support.

@bep
Copy link
Member

bep commented May 18, 2017

This looks great. Don't bother with the site config support; we will have to revisit this at some point. With this, people can easily put that config in site params.

@bep bep merged commit 93b3b13 into gohugoio:master May 18, 2017
@moorereason moorereason deleted the iss1444 branch December 17, 2020 18:20
@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 Jan 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Outputting numbers with thousands separators
3 participants