Skip to content
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 ngettext #5257

Merged
merged 12 commits into from Aug 24, 2019

Conversation

@Frenzie
Copy link
Member

commented Aug 24, 2019

Fixes #5249.

See https://www.gnu.org/software/gettext/manual/html_node/Plural-forms.html and https://www.gnu.org/software/gettext/manual/html_node/Translating-plural-forms.html for more information.

Usage:

local T = ffiUtil.template
local _ = require("gettext")
local N_ = _.ngettext

local items_string = T(N_("1 item", "%1 items", num_items), num_items)

Looks a little something like this:
Screenshot_2019-08-24_18-58-11
Screenshot_2019-08-24_18-58-18

@Frenzie Frenzie added the enhancement label Aug 24, 2019

@Frenzie Frenzie added this to the 2019.09 milestone Aug 24, 2019

Frenzie added 3 commits Aug 24, 2019
Makefile Outdated
@@ -466,7 +466,8 @@ XGETTEXT_BIN=xgettext

pot:
mkdir -p $(TEMPLATE_DIR)
$(XGETTEXT_BIN) --from-code=utf-8 --keyword=C_:1c,2 \
$(XGETTEXT_BIN) --from-code=utf-8 \
--keyword=C_:1c,2 --keyword=N_:1,2 \

This comment has been minimized.

Copy link
@Frenzie

Frenzie Aug 24, 2019

Author Member

For npgettext, given that we're using C_ for pgettext, logically that should be NC_. I didn't really test that one yet but other than the lacking npgettext method it should be working.

This comment has been minimized.

Copy link
@poire-z

poire-z Aug 24, 2019

Contributor

But CN_ would state the order: first context, then the next thing will be N'ed :)
(if we don't need to care about the involved gettext function names - but you decide)

This comment has been minimized.

Copy link
@Frenzie

Frenzie Aug 24, 2019

Author Member

For reference, GTK also has an NC_ (although it means something completely different): https://developer.gnome.org/glib/stable/glib-I18N.html#NC-:CAPS

Anyway, I don't even know if we need npgettext. It's just with all the groundwork already there it seems silly not to add the method.

@Frenzie Frenzie changed the title [feat] Implement ngettext [feat, i18n] Implement ngettext Aug 24, 2019

local util = require("util")
headers = data.msgstr
local plural_forms = data.msgstr:match("Plural%-Forms: (.*);")
local nplurals = plural_forms:match("nplurals=([0-9]+);") or 1

This comment has been minimized.

Copy link
@Frenzie

Frenzie Aug 24, 2019

Author Member

2 actually seems to be right; I'll have to look at the docs a bit better.

This comment has been minimized.

Copy link
@Frenzie

Frenzie Aug 24, 2019

Author Member

Oh, I guess perhaps I can't check for sanity because a C boolean expression is 0 or 1.

Frenzie added 7 commits Aug 24, 2019
@@ -381,7 +382,7 @@ function ListMenuItem:update()
end
else
if pages then
pages_str = T(_("%1 pages"), pages)
pages_str = T(N_("1 page", "%1 pages", pages), pages)

This comment has been minimized.

Copy link
@Frenzie

Frenzie Aug 24, 2019

Author Member

@poire-z Like so:

Screenshot_2019-08-24_23-00-42

Screenshot_2019-08-24_22-58-43

@Frenzie Frenzie merged commit 2c55583 into koreader:master Aug 24, 2019

1 check passed

ci/circleci: build Your tests passed on CircleCI!
Details

@Frenzie Frenzie deleted the Frenzie:ngettext branch Aug 24, 2019

@Frenzie

This comment has been minimized.

Copy link
Member Author

commented Aug 24, 2019

Merging now so the translations can start rolling in; code should work as designed per-spec but we'll see if anything funny crops up in the wild. ;-)

@Frenzie

This comment has been minimized.

Copy link
Member Author

commented Aug 24, 2019

And it looks like this on Transifex:

Screenshot_2019-08-24_23-21-21

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.