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

x/text: plural.Zero doesn't work as expected #27724

Closed
olivere opened this issue Sep 18, 2018 · 8 comments
Closed

x/text: plural.Zero doesn't work as expected #27724

olivere opened this issue Sep 18, 2018 · 8 comments

Comments

@olivere
Copy link

olivere commented Sep 18, 2018

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.11 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/oliver/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/oliver"
GOPROXY=""
GORACE=""
GOROOT="/Users/oliver/go"
GOTMPDIR=""
GOTOOLDIR="/Users/oliver/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/vx/tzlnk7sx0bsckdbj2j_0t0k00000gn/T/go-build015279889=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

If possible, provide a recipe for reproducing the error.
A complete runnable program is good.
A link on play.golang.org is best.

$ cd feature/plural
$ go test -run=Example -v
=== RUN   ExampleSelect
--- PASS: ExampleSelect (0.00s)
PASS
ok  	golang.org/x/text/feature/plural	0.005s

# Edit example_test.go and replace "=0" with plural.Zero

$ git diff
diff --git a/feature/plural/example_test.go b/feature/plural/example_test.go
index c75408c..2938c4c 100644
--- a/feature/plural/example_test.go
+++ b/feature/plural/example_test.go
@@ -14,7 +14,7 @@ func ExampleSelect() {
        // Manually set some translations. This is typically done programmatically.
        message.Set(language.English, "%d files remaining",
                plural.Selectf(1, "%d",
-                       "=0", "done!",
+                       plural.Zero, "done!",
                        plural.One, "one file remaining",
                        plural.Other, "%[1]d files remaining",
                ))

$ go test -run=Example -v
=== RUN   ExampleSelect
--- FAIL: ExampleSelect (0.00s)
got:
nog één bestand te gaan
klaar!
want:
5 files remaining
one file remaining
nog één bestand te gaan
klaar!
FAIL
exit status 1
FAIL	golang.org/x/text/feature/plural	0.004s

What did you expect to see?

I expect to see the same results regardless if using "=0" or plural.Zero.

What did you see instead?

I see the example code silently ignoring pluralization cases.

@gopherbot gopherbot added this to the Unreleased milestone Sep 18, 2018
@odeke-em
Copy link
Member

Thank you for this report @olivere! Kindly paging @mpvl

@vikramcse
Copy link

@odeke-em Can I work on this issue?

@odeke-em
Copy link
Member

@vikramcse, all yours and thank you!

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 22, 2018
@vikramcse
Copy link

vikramcse commented Sep 22, 2018

@bcmills @odeke-em Hello there
Here are my findings

  1. I think in file gen_common.go, the ordering of the constants is wrong. The ordering should be like this.
const (
	Zero Form = iota
	One
	Two
	Few
	Many
        Other
)

I did try changing above constants structure and run go generate but it did not solved problem.

  1. Let's consider below test case
message.Set(language.English, "%d files remaining",
		plural.Selectf(1, "%d",
			plural.Two, "2 files remaining",
		))

p := message.NewPrinter(language.English)
p.Printf("%d files remaining", 2)

In above case we are passing plural.Two as a case which has byte value 3 (because of the wrong order).
The selector type 3 does not match the valid cardinal rules list in this case which is [0, 2] which is returned by validForms function; further the valid list is used by this code but it's throwing error s:"plural: form '\x03' not supported for language "en"" as the value 3 does not match with [0, 2]

I am not sure how to take this further.

EDIT:
Hi @mpvl, i need some help to solve this issue.

Thanks

@bcmills
Copy link
Contributor

bcmills commented Sep 22, 2018

I need some help to solve this issue.

@mpvl is the right person to help. (He's the primary owner of x/text.)

@mpvl
Copy link
Contributor

mpvl commented Oct 29, 2018

Other has special meaning in CLDR and really should be 0. But I will look in to this.

@mpvl
Copy link
Contributor

mpvl commented Oct 29, 2018

This issue is the following: linguistically, English doesn't have a "Zero" plural form, only "One" and "Other" (singular and anything else). Like with ICU, the Go implementation of plurals allows matching to specific numeric values, like =0, but this is not the same as plural.Zero, which will never match in English or, in the example, Dutch.

In general, the linguistic plural categories are just names and may often not correspond one-to-one to a particular number. For instance, in Croatian the numbers 1, 21, and 101 are all considered to be plural.One. So "one" and plural.One are equivalent, but "=1" and plural.One, in general, are not.

So this is partly a documentation and naming issue. Using plural.Singular instead of plural.One, for instance, would have been clearer, in English. However, that approach wouldn't scale. The numeric names only loosely match a myriad of plural categories in different languages.

The documentation states that the plural categories do not correspond one-to-one with numbers. The naming is also the standard naming from ICU/CLDR. But I'm open to suggestions as to how to make this clearer.

@mpvl mpvl removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 29, 2018
@mpvl
Copy link
Contributor

mpvl commented Oct 29, 2018

working as intended.

@mpvl mpvl closed this as completed Oct 29, 2018
@golang golang locked and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants