Skip to content
This repository has been archived by the owner on Jun 29, 2019. It is now read-only.

Plugin: Math - Support European-style decimal separator #268

Merged
merged 2 commits into from
Jun 10, 2018

Conversation

dannya
Copy link
Member

@dannya dannya commented Jun 5, 2018

  • Autodetect input mode - UK-style decimal separator "." (current) or European-style decimal separator "," (new) - and ensure the calculation and output matches what the user expects.

Follows the suggested implementation from the MathJS library documentation: http://mathjs.org/examples/browser/custom_separators.html.html

Implements #246

hain-calc

* Autodetect input mode - UK-style decimal separator "." (current) or European-style decimal separator "," (new) - and ensure the calculation and output matches what the user expects.

Follows the suggested implementation from the MathJS library documentation: http://mathjs.org/examples/browser/custom_separators.html.html
@dannya
Copy link
Member Author

dannya commented Jun 5, 2018

I also bolded the calculation result!

try {
let query = rawQuery;

// determine input mode by the presence of European-style decimal separators
Copy link
Member

Choose a reason for hiding this comment

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

Why not use Intl.NumberFormat?

console.log((new Intl.NumberFormat()).resolvedOptions());
Shows this on my system (I believe it gets it from the system)

{ locale: 'und',
numberingSystem: 'latn',
style: 'decimal',
useGrouping: true,
minimumIntegerDigits: 1,
minimumFractionDigits: 0,
maximumFractionDigits: 3 }

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/NumberFormat

Copy link
Member Author

Choose a reason for hiding this comment

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

@cpriest it's an interesting suggestion, but I prefer my solution as:

  • it is simpler, and more likely to work across platforms
  • it allows users to utilise either decimal point or comma independently of whatever the system locale might be set to

Copy link
Member Author

Choose a reason for hiding this comment

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

@cpriest are you happy for this to be merged as-is?

Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer it be more internationally useful, but I don't have a lot of conviction for it, so I'm fine with it. (I have no personal experience with Internationalization, but a lot of other people do care.)

@appetizermonster
Copy link
Collaborator

looks good to me 👍

@dannya dannya merged commit 2a289bd into develop Jun 10, 2018
@dannya dannya deleted the feature/calculator-l10n branch June 10, 2018 14:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants