leonelquinteros / gotext

Go (Golang) GNU gettext utilities package
Other
434 stars 58 forks source link

Issue with Arabic localization not returning any strings #39

Closed chrisvaughn closed 4 years ago

chrisvaughn commented 4 years ago

Please describe your issue

First off, thanks for all the work on this project. It's exactly what I need for my use case.

I get no string returned when localizing Arabic. I've tracked it down to the complex Pluralization rules of Arabic. In this case, I'm not trying to use plurals.

Is this a bug, an improvement, a proposal or something else? Describe it.

Bug ..

What's the expected behaviour, the current behaviour and the steps to reproduce it?

I've created a sample project that shows the behavior: https://github.com/chrisvaughn/ar_gotext The key code being

enLocale := gotext.NewLocale("locale", "en_US")
enLocale.AddDomain("messages")

arLocale := gotext.NewLocale("locale", "ar_SA")
arLocale.AddDomain("messages")

po := new(gotext.Po)
po.ParseFile("locale/ar_SA/LC_MESSAGES/messages.po")

fmt.Printf("English With Just a Get: %s\n", enLocale.Get("This is a sample string"))
fmt.Printf("Arabic With Just a Get: %s\n", arLocale.Get("This is a sample string"))
fmt.Printf("Arabic Plural To 0: %s\n", arLocale.GetN("This is a sample string", "This is a sample string", 0))
fmt.Printf("Arabic Using po.Get: %s\n", po.Get("This is a sample string"))

the output of running this code is

English With Just a Get: This is a sample string
Arabic With Just a Get: 
Arabic Plural To 0: هذه سلسلة عينة
Arabic Using po.Get: هذه سلسلة عينة

I would expect arLocale.Get("This is a sample string") and po.Get("This is a sample string") to return the same thing in this case. ...

Comments

I don't think it's safe to assume you can always call GetND under the hood with a default plural of 1. I'm happy to submit an MR to attempt to address this.

I see that there is a test for Arabic from a previous issue but in that test the plural rules used are "Plural-Forms: nplurals=2; plural=(n != 1);\n" the actual plural rules for Arabic are "Plural-Forms: nplurals=6; plural=(n==0 ? 0 : n==1 ? 1 : n==2 ? 2 : n%100>=3 && n%100<=10 ? 3 : n%100>=11 && n%100<=99 ? 4 : 5);\n"

leonelquinteros commented 4 years ago

Hi @chrisvaughn,

Thank you so much for checking this and reporting it. Quickly looking at the code, it seems the problem is in the translation.go file, Translation.GetN(n int) method:


// GetN returns the string of the plural translation
func (t *Translation) GetN(n int) string {
    // Look for Translation index
    if _, ok := t.Trs[n]; ok {
        if t.Trs[n] != "" {
            return t.Trs[n]
        }
    }

    // Return untranslated singular if corresponding
    if n == 0 {
        return t.ID
    }

    // Return untranslated plural by default
    return t.PluralID
}

For your case, I'm pretty sure that t.PluralID is empty, because isn't defined in your PO file. Maybe a simple:

        // Return untranslated singular if corresponding
    if n == 0 || t.PluralID == "" {
        return t.ID
    }

does the trick.

But I prefer you to follow up the case and figure out if we're not missing any other use cases here. I'd be happy to get a PR from you to review with its corresponding tests.

Thanks for your contribution!

chrisvaughn commented 4 years ago

Thanks for the quick response, @leonelquinteros. I'll follow up on your tip and see if that works for me. I'm in the process of porting a project from Python that was using Pybabel to Go. This project has localizations in ~70 languages so I have a lot of cases to test against. I'll report back once I have a branch that's working for Arabic and then I'll run through the other languages to see if anything else with complex plural rules (like Russian) has problems.

Thanks again.

chrisvaughn commented 4 years ago

@leonelquinteros - Once I changed the pluralform string on the existing Arabic test to the correct string the existing test started failing in the same way that my sample code is failing. The call to Get returned an empty string. I tried your suggested change and that improved the behavior but it only returned the original English string and the existing test still failed. The changes submitted above made the existing tests pass again. Please take a look and let me know what you thing. Thanks

leonelquinteros commented 4 years ago

This got fixed by https://github.com/leonelquinteros/gotext/pull/40