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

Localization API #96

Closed
catamphetamine opened this issue Feb 13, 2019 · 16 comments
Closed

Localization API #96

catamphetamine opened this issue Feb 13, 2019 · 16 comments
Assignees

Comments

@catamphetamine
Copy link

catamphetamine commented Feb 13, 2019

I can see that your library supports some localization via separator, symbols and fullforms.
I guess, for Russian it would be something like:

filesize(..., {
  separator: ',',
  symbols: {
    B: "Б",
    KB: "кБ",
    MB: "мБ",
    ...
  },
  fullforms: [
    '',
    'кило',
    'мега',
    ...
  ]
})

You could make a directory called locales where people could submit pull requests with their languages.

import ru from 'filesize/locales/ru'
filesize(..., { locale: ru })

A similar API:
https://github.com/sindresorhus/pretty-bytes#readme

// Localized output using German locale
prettyBytes(1337, {locale: 'de'});
//=> '1,34 kB'

Notice how they also use , in that example.
That's because they use number.toLocaleString() instead of separator.
They should have also checked for number.toLocaleString existence before using it and then fall back to a default separator.
They also don't have kB translated.

These are localization API ideas you might want to review in some future if you decide on adding localization to this library.
I don't need localization, I'm using en-US.

@avoidwork
Copy link
Owner

That would be a much better solution to #93, which is more or less a hack.

@catamphetamine
Copy link
Author

It's still not a proper localization because it doesn't translate messages.

@avoidwork
Copy link
Owner

messages?

@avoidwork avoidwork self-assigned this Feb 13, 2019
@catamphetamine
Copy link
Author

A user passing locale: de property would expect the units to be translated into German.

@catamphetamine
Copy link
Author

Also your code will break when there's no .toLocaleString().

@avoidwork
Copy link
Owner

those are separate features; a language pack of custom symbols or full form words can be loaded into the instance. providing all variations is something i wouldn't attempt for free.

@catamphetamine
Copy link
Author

Then locale doesn't actually localize things.

@avoidwork
Copy link
Owner

@catamphetamine
Copy link
Author

That table only shows the latest verions of browsers.
The method couldn't "always exist" because it's not possible.
Your code will still break in older browsers.

@avoidwork
Copy link
Owner

locale actually does it's job in regards to the number, e.g. 1000000.24' in 'de' would be 1.000.000,24` which is only something you'd get for an exceptionally large number; majority will be under 3 significant digits.

@catamphetamine
Copy link
Author

locale actually does it's job in regards to the number

locale only does it's job in regards to the number.
You should state that it doesn't localize the units.

@avoidwork
Copy link
Owner

locale actually does it's job in regards to the number

locale only does it's job in regards to the number.
You should state that it doesn't localize the units.

are you grasping what you're suggesting? a language pack for all permutations is unmanageable.

as per your claim of support... https://caniuse.com/#feat=internationalization

@catamphetamine
Copy link
Author

catamphetamine commented Feb 13, 2019

a language pack for all permutations is unmanageable.

Nothing "unmanageable" in that.

as per your claim of support... https://caniuse.com/#feat=internationalization

As per your claim of support: the link clearly shows that it's not supported in all browsers.
So your code will break.

@avoidwork
Copy link
Owner

i don't care if my code breaks in a deprecated browser that only runs in an OS that's 10+ years out dated. I just don't care. I don't care if you want this feature to work on modern browsers and netscape 1.0.

You're not paying me money, so I don't care.

@catamphetamine
Copy link
Author

I don't care about what you care and don't care.
You're a low-skill develoepr.

@avoidwork
Copy link
Owner

And yet you opened this ticket, and it gets over 4 million installs a week.

I don't care if you care or not. I did you a favor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants