Skip to content

Make fluent_number obey format specifiers for currencies #5

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

Merged
merged 2 commits into from
Nov 18, 2020

Conversation

spookylukey
Copy link
Contributor

@spookylukey
Copy link
Contributor Author

Regarding @Pike's comment on the other PR:

As for this aspect of number formatting, I'm wary of exposing the number of digits to localizers for currencies. That doesn't sound like something that should be language specific. @stasm ?

Still, might be good to have a way for the programmer to control this.

In fluent-compiler, we have ftl_arg_spec which allows control over what args are available to FTL files. However, we don't (yet) have the ability to say “make maximumFractionDigits allowed if its not a currency, but disallowed if is a currency", which I think is what would be needed.

@Sh4rK
Copy link

Sh4rK commented Nov 17, 2020

Thanks!

We are actually using Fluent with Django, and hence using your package, so my original goal was to get the PR accepted "upstream", then open a PR here as well, but it seems you have read my mind :)

What's your policy regarding these kind of things? Are you willing to merge it even if upstream doesn't? Can I help in any way?

@spookylukey
Copy link
Contributor Author

@Sh4rK I'm trying to keep compatibility as much as possible, so I opened this PR in anticipation of it getting merged there, and to see if it looks any different in this package. So thanks for opening the issue on python-fluent first, that really helps us keep in sync.

That said, I have been simply unable to use official python-fluent with its current feature set, especially due to a missing solution for HTML handling, which is implemented in fluent-compiler using the 'escapers' functionality. So I have been more ready to add things where it solves developer problems and makes the package usable now, rather than wait for 100% agreement on the path forward.

If you can cope with potential backwards incompatible changes in the future, I think this looks like a good change.

@spookylukey
Copy link
Contributor Author

We are actually using Fluent with Django, and hence using your package

Forgot to say - cool, django-ftl has a user apart from me 😄 🍾

@Sh4rK
Copy link

Sh4rK commented Nov 17, 2020

I opened this PR in anticipation of it getting merged there

I opened that one a while ago, and unfortunately there's not much activity, so I'm starting to give up on that :/

If you can cope with potential backwards incompatible changes in the future, I think this looks like a good change.

Well we haven't ever used the other package (and truthfully only did a quick test with yours, but we plan to go forward with it), so it wouldn't really be backwards incompatible for us specifically :)

Forgot to say - cool, django-ftl has a user apart from me 😄 🍾

Thanks for creating it, it's exactly what we needed!

@spookylukey spookylukey merged commit e73707f into master Nov 18, 2020
@spookylukey spookylukey deleted the currency-formatting-should-obey-format-specifiers branch November 18, 2020 18:18
@Sh4rK
Copy link

Sh4rK commented Nov 19, 2020

Awesome!

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.

2 participants