feat(pt-BR): add number parsing support and ordinal numeric formatting#1785
feat(pt-BR): add number parsing support and ordinal numeric formatting#1785Kaikygabriel wants to merge 3 commits into
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a token-map number parser under ChangesPortuguese-Brazil Locale Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05ffb8e3a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| noventa: 90 | ||
| cem: 100 | ||
| mil: 1000 | ||
| milhão: 1000000 |
There was a problem hiding this comment.
milhões token to pt-BR cardinal map
The new token-map parser for pt-BR only includes milhão but omits the plural milhões, even though this locale’s number-to-words output uses plural million forms (for example, values like 2,000,000 are rendered with milhões). TokenMapWordsToNumberConverter resolves scale words via exact token lookup, so dois milhões is treated as unrecognized and parsing fails for common million-range inputs.
Useful? React with 👍 / 👎.
| setenta: 70 | ||
| oitenta: 80 | ||
| noventa: 90 | ||
| cem: 100 |
There was a problem hiding this comment.
cento/hundreds tokens needed for canonical pt-BR parsing
This cardinal map defines cem but not cento (nor other hundred words), while the same locale’s number-to-words surface emits forms like cento e um and duzentos. With token-map parsing, those words must exist in cardinalMap to be recognized, so many standard pt-BR numbers in the 101–999 range will fail to parse even though they are canonical outputs of the locale.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Humanizer/Locales/pt-BR.yml`:
- Around line 342-378: Add the missing numeric word keys to the cardinalMap so
parsing handles plural and hundred forms: include plural large-number keys
("milhões", "trilhões", "quadrilhões", "quintilhões"), add hundreds entries
matching number.words.cardinal.hundredsMap ("duzentos", "trezentos",
"quatrocentos", "quinhentos", "seiscentos", "setecentos", "oitocentos",
"novecentos"), and ensure "milhões" maps to 1000000 while the plural
large-number keys map to their corresponding powers of 1,000; update the
cardinalMap block (the YAML mapping named cardinalMap) to contain these keys and
their numeric values to mirror the singular forms already present (e.g.,
"milhão":1000000).
- Around line 441-444: Add Portuguese articles for nextHour in the pt-BR clock
templates: define singularArticle and pluralArticle entries in the pt-BR clock
section (same keys used by other locales) and update min40, min45, min50 and
min55 to include the nextArticle token before nextHour (use "{nextArticle}
{nextHour}" pattern) so phrases like "vinte para as duas" are produced; modify
the existing min40/min45/min50/min55 templates and add
singularArticle/pluralArticle keys accordingly in the pt-BR locale block.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 51370ed1-4ba6-4b98-bdc2-fc343f3825e3
📒 Files selected for processing (1)
src/Humanizer/Locales/pt-BR.yml
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Humanizer/Locales/pt-BR.yml (1)
238-238: 💤 Low valueConsider using an abbreviated symbol for week.
Other time unit symbols use short abbreviations (
ms,s,min,h,d,m,a), but week uses the full wordsemana. While Portuguese lacks a universally accepted abbreviation for "week," usingsemorsem.would maintain consistency with other symbols. If the full word is intentional for clarity, feel free to disregard.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Humanizer/Locales/pt-BR.yml` at line 238, The pt-BR locale uses the full word for the week symbol ("symbol: 'semana'"); change that value to a short abbreviation (e.g., "sem" or "sem.") to match the other short unit symbols (ms, s, min, h, d, m, a) and keep consistency across time unit symbols—update the "symbol: 'semana'" entry accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/Humanizer/Locales/pt-BR.yml`:
- Line 238: The pt-BR locale uses the full word for the week symbol ("symbol:
'semana'"); change that value to a short abbreviation (e.g., "sem" or "sem.") to
match the other short unit symbols (ms, s, min, h, d, m, a) and keep consistency
across time unit symbols—update the "symbol: 'semana'" entry accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4603d172-ce6f-4c3a-a670-d162a4f5d93c
📒 Files selected for processing (1)
src/Humanizer/Locales/pt-BR.yml
1 participant
Here is a checklist you should tick through before submitting a pull request:
mainbranch (more info below)fixes #<the issue number>build.cmdorbuild.ps1and ensure there are no test failuresEnhances the pt-BR locale with improved localization and number parsing support.
Changes include:
No runtime code changes were made. This PR focuses only on locale improvements.
This brings the pt-BR locale closer to feature parity with the en locale while preserving natural Portuguese usage.