-
-
Notifications
You must be signed in to change notification settings - Fork 43
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
✨ feat(i18n): implement pluralization logic #277
Conversation
Yes, this b6aa219 took 51 minutes of debugging. it was always says the language is |
My bad! |
Let me know if you want any help or have any questions, by the way! Happy to help here. |
Now the macro works good with Arabic, what next? |
5a09ba3
to
62fc7de
Compare
to:
Or something like this. Not sure how feasible this is, though. Just an idea.
All of these are HTML strings, except for the search results. I'll take care of modifying the JavaScript at the end.
Once all of this is done, modify existing |
Great, I'll start on it tomorrow |
ChangesIntegrate
|
As I see is
I see Note This only for Arabic (for now) other languges will only have I'll work on it now |
Most languages use |
I don't see this is good, because for example in |
Also |
If you don't think so, I will add it in Arabic only, because in Arabic we can't say "0 minutes" or "0 results" it is against the laws of language |
It does sound better. My main concern is keeping the
Agreed. Alright, let's go with It makes me sad to realise we'll probably have to duplicate logic for the pluralisation, though: function updateResultText(count) {
const nResultsSpan = document.getElementById('n-results');
nResultsSpan.textContent = count.toString();
const singular = document.getElementById('result-text-singular');
const plural = document.getElementById('result-text-plural');
if (count === 1) {
singular.style.display = 'inline';
plural.style.display = 'none';
} else {
singular.style.display = 'none';
plural.style.display = 'inline';
}
} This is the current search JS. Oh, well. It's better to duplicate logic than to load JS just to do pluralisation, imo. |
I will work on translations now, and keep JS things for you |
You can do English and Arabic. I'll do the rest! |
- Make the default number `-1` - Make `zero_` and `one_` for all languages
@welpo Now should the remaining is JS and translating languages else than Arabic and English |
I think we can remove the |
ee5d269
to
3791ffc
Compare
Are we done like this? Do I prepare it for review? (Remove debugging and so on) |
btw, I really like the way you merge the PR since I don't really have to make the PR/branch log very clean (because it won't appear in the master, it just shows commit as the PR) |
Yeah, squash-and-merge pull requests are pretty cool! I like how you get to keep the atomic commits for comparison yet keep a clean
Nope! I need to add:
|
Almost done! Search now uses the new strings… s.movCan't test with Arabic, as Zola can't build an index for it. However, I replicated the logic for pluralization anyway; if it ever gets added, it should work without any changes. |
Added a new I needed to do this so that JavaScript could dynamically update the I think this is done! Tomorrow I will do a careful review of all changes + some more testing. If you can do some testing to ensure everything works well, I'd appreciate it! Off the top of my head:
|
I'll, in the morning |
Sorry for the delay, but I've been busy with university. I checked the following and they are all ok
|
I'll check soon as I get to my laptop |
All is good as expected |
Thank you! I will do some testing later. If everything is good, I will remove the debugging stuff and merge. |
I think it's done! I'll merge now. Thank you so much for suggesting this feature and working to make it a reality! It's been a pleasure collaborating~ |
Co-authored-by: welpo <welpo@users.noreply.github.com>
Ref #276
Proof of concept pluralization for testing.
Discussion: #273
The logic to get the strings is in the macro
pluralize.html
.To test:
templates/page.html
to change thecount
variable in theDebugging
section🚨 IMPORTANT: when you change the TOML strings in
i18n
, you need to restartzola serve
to use the updated strings.