Support for Arabic numerals and decimal separator #5
Conversation
Added two options: useHindiNumerals: defaults to true. If false uses the Arabic (modern) numerals (0, 1, 2...) useArabicDecimalSeparator: defaults to true. Uses the comma instead of the point as the decimal separator Require proper documentation
|
@SamirElkhatib thank you so much for the contribution! I'll take a look at it in a week or so. Please excuse my limited availability, as I've never expected anyone to contribute 😄 |
There was a problem hiding this comment.
Thanks @SamirElkhatib for the updates!
We're close! I've added a couple of other notes.
| '6.': '\u0666\u066b', | ||
| '7.': '\u0667\u066b', | ||
| '8.': '\u0668\u066b', | ||
| '9.': '\u0669\u066b', |
There was a problem hiding this comment.
@SamirElkhatib please use literal values like ٩ instead of \u0669. There's a build step to do that conversion. Let's keep the work human friendly.
| }, | ||
|
|
||
| // support for arabic (modern) numerals | ||
| arabicNumbersMap: { |
There was a problem hiding this comment.
I don't see why this is needed. We can disable the numbersMap instead of doing this. Right?
|
|
||
| // Default options | ||
| if (useHindiNumeral == null) useHindiNumeral = true | ||
| if (useArabicDecimalSeparator == null) useArabicDecimalSeparator = true |
There was a problem hiding this comment.
I suggest that we identify a better pattern than this e.g. use falsey variables like: disableArabicDecimalSeparator and disableHinduArabicNumbers
| var englishNumbersRegExp = /[0-9]/g; | ||
| if(useHindiNumeral) var numbersMap = arConfig.numbersMap; | ||
| else var numbersMap = arConfig.arabicNumbersMap | ||
| } |
There was a problem hiding this comment.
All the section above needs refactoring:
- The for looop isn't protected with
hasOwnProperty - Avoid for-loops and use RegExp replace functions like
var mapped = text.replace(englishNumbersRegExp, replaceNumber);
| @@ -57,6 +57,34 @@ MathJax.Extension.Arabic = { | |||
| '8': '٨', | |||
| '9': '٩' | |||
| }, | |||
| // numbers with arabic decimal separator ',' | |||
| arabicDecimalSeparator: { | |||
There was a problem hiding this comment.
I don't think we need this map, we can use RegExp instead without the need to repeat what's in numbersMap. I can add more details if you'd like.
|
@SamirElkhatib it would be great if you can update the README as we go. You're adding cool features to the plugin, let's make them visible! |
|
@SamirElkhatib #7 have added the support for Arabic decimal separator, so there's no need to do the same work in this PR again. Could you please refactor it to focus on the optional Arabic numerals only? Thanks! |
|
@shadinaif do we still need this pull request? |
Added two options:
useHindiNumerals: defaults to true. If false uses the Arabic (modern) numerals (0, 1, 2...)
useArabicDecimalSeparator: defaults to true. Uses the comma instead of the point as the decimal separator
Requires proper documentation and building...