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

feat: Plurals #58

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

feat: Plurals #58

wants to merge 7 commits into from

Conversation

sullenor
Copy link
Collaborator

@sullenor sullenor commented Aug 2, 2017

Plural and Parametrized keys

Adds support of plural and parametrized keys (issue #1).

The modern version of #42

@sullenor sullenor changed the title Plurals feat: Plurals Aug 2, 2017
@codecov
Copy link

codecov bot commented Aug 2, 2017

Codecov Report

Merging #58 into master will increase coverage by 0.92%.
The diff coverage is 96.52%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #58      +/-   ##
==========================================
+ Coverage   95.12%   96.04%   +0.92%     
==========================================
  Files           4       25      +21     
  Lines          82      354     +272     
  Branches       23      144     +121     
==========================================
+ Hits           78      340     +262     
- Misses          4       13       +9     
- Partials        0        1       +1
Impacted Files Coverage Δ
src/interpolate.js 0% <0%> (ø)
src/pluralRules/pluralRule02.js 100% <100%> (ø)
src/pluralRules/pluralRule04.js 100% <100%> (ø)
src/pluralRules/pluralRule07.js 100% <100%> (ø)
src/pluralRules/pluralRule12.js 100% <100%> (ø)
src/pluralRules/pluralRule08.js 100% <100%> (ø)
src/pluralRules/pluralRule13.js 100% <100%> (ø)
src/pluralRules/pluralRule00.js 100% <100%> (ø)
src/pluralRules/pluralRule11.js 100% <100%> (ø)
src/pluralRules/pluralRule15.js 100% <100%> (ø)
... and 33 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd32318...09504f7. Read the comment docs.

@sullenor
Copy link
Collaborator Author

sullenor commented Aug 2, 2017

smirks @d3viant0ne :)

Copy link
Member

@michael-ciniawsky michael-ciniawsky left a comment

Choose a reason for hiding this comment

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

All question are purely naive atm as I'm not really suited to review this :), but we should definitely push this forward

@@ -37,6 +37,8 @@ plugins: [
- `optionsObj.functionName`: the default value is `__`, you can change it to other function name.
- `optionsObj.failOnMissing`: the default value is `false`, which will show a warning message, if the mapping text cannot be found. If set to `true`, the message will be an error message.
- `optionsObj.hideMessage`: the default value is `false`, which will show the warning/error message. If set to `true`, the message will be hidden.
- `optionsObj.pluralIdentName`: the default value is `count`, which will be used to determine whether key is plural or not.
Copy link
Member

Choose a reason for hiding this comment

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

Possible to merge this a one option e.g options.plurals

options.plurals = {
   identName: 'count',
   ruleNumber:  1
}

Are identName && ruleNumber standardize names for plurals ?

Copy link
Collaborator Author

@sullenor sullenor Oct 21, 2017

Choose a reason for hiding this comment

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

I'm not certain about names. Just used something that is related to the stuff :)

Plural rule is used to choose the proper grammar form and there are plenty of them: https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_and_Plurals
I guess we can use pluralRule instead as in the guide.

And the identName is more related to the parsing stuff. I need the way to tell the difference between plural and singular keys. In our company we usually use the particular variable name for the plurals.

@@ -0,0 +1,6 @@
// Families: Asian (Chinese, Japanese, Korean), Persian, Turkic/Altaic (Turkish), Thai, Lao
Copy link
Member

Choose a reason for hiding this comment

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

For all these rules there isn't a sophisticated lib available ?

Copy link
Collaborator Author

@sullenor sullenor Oct 21, 2017

Choose a reason for hiding this comment

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

I haven't checked actually. The main idea was to have small functions, so it could be easier to inline them and to have a smaller resulting bundle.

return interpolate;
}

export default createInterpolateFn;
Copy link
Member

Choose a reason for hiding this comment

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

export default (fn) => interpolate

moduleJsPath = `./${moduleJsPath.replace(/\\/g, '/')}`;
}

return `require(${JSON.stringify(moduleJsPath)}).default`;
Copy link
Member

Choose a reason for hiding this comment

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

Is there a way to ES2015 Syntax here e.g hoisting import

`import module_${i} from ${moduleJsPath}`

...

`${module_[i]}`

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.

None yet

4 participants