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/message: better documentation for catalog #26767

Open
belimawr opened this issue Aug 2, 2018 · 11 comments
Open

x/text/message: better documentation for catalog #26767

belimawr opened this issue Aug 2, 2018 · 11 comments

Comments

@belimawr
Copy link

belimawr commented Aug 2, 2018

The documentation for this package could be clearer with regard to how catalogs should be used.
Specifically:

  • the default catalog is empty.
  • how to populate the default catalog (is there a standard way of doing this?)
  • document that the default fallback language is English.

Repo commit: b0f49b06765e38644f97dff880167326a7ab0391

@gopherbot gopherbot added this to the Unreleased milestone Aug 2, 2018
@belimawr
Copy link
Author

belimawr commented Aug 2, 2018

I'm already working on this documentation improvement.

@belimawr
Copy link
Author

belimawr commented Aug 2, 2018

There is a bit of the discussion on #26766

After more reading of the documentation/code on the package and also the point raised by @kaskavalci, I believe that the best way of making this package example work is to pass the language directly to message.NewPrinter.

@gopherbot
Copy link

Change https://golang.org/cl/127598 mentions this issue: message: improve documentation

@kaskavalci
Copy link

I think MatchLanguage behavior and its interaction with Catalog is also worth noting. A short example with adding number of languages to Catalog then calling MatchLanguage as in the catalog_test.go could be helpful. Using pre-configured language is straightforward but matching a language on the fly is more interesting use-case.

@belimawr
Copy link
Author

belimawr commented Aug 7, 2018

@kaskavalci I totally agree with you. I'm planning to extend the http example there to:

  • Add a few expressions on the catalog
  • Get the language using a request parameter and MatchLanguage
  • Return the translated text (not number formatting) on the response.

This way it will be more clear of how all those functions interact with each other.

I hope to get it done by the end of this week.

@beono
Copy link

beono commented Sep 11, 2018

@belimawr Hey buddy :)
I don't know your use case, but this package looks unreliable.
Recently I faced a major issue that had been in the master for 6 months.
https://go-review.googlesource.com/c/text/+/128795

Take a look at this repo https://github.com/nicksnyder/go-i18n

@belimawr
Copy link
Author

Hey @beono !

Actually my use case is to contribute with the Golang project. So far I haven't used this package in production yet.

@belimawr
Copy link
Author

@kaskavalci I finally had time to update the examples \0/

@kaskavalci
Copy link

Thanks @belimawr !

@kaskavalci
Copy link

Hey @belimawr

I'm working with message again and hit another problem. This is not from catalog documentation but from message. https://godoc.org/golang.org/x/text/message#example-Printer--MVerb

When you run the example it complains about %m type:

nl     U heeft ervoor gekozen om %!m(string=soccer) te spelen.

Changing it to string type %s resulted in no translation. See soccer is not converted into "voetbal".

nl     U heeft ervoor gekozen om soccer te spelen.

Another weird issue I noticed is in printer. See the following example. Is that the expected behavior?

	message.SetString(language.Dutch, "soccer", "voetbal")
	p := message.NewPrinter(language.Make("nl"))
	p.Printf("soccer\n")             // prints soccer
	fmt.Println(p.Sprint("soccer"))  // prints soccer
	fmt.Println(p.Sprintf("soccer")) // prints voetball

@kaskavalci
Copy link

Noticed one more problem. Dutch decimal separator is comma. When NewPrinter is called by MatchLanguage it prints wrong output but with language.Dutch it works as it should.

p := message.NewPrinter(message.MatchLanguage("nl"))
p.Printf("Hoogte: %.1f meter\n", 1244.9) // Prints Hoogte: 1,244.9 meter

m := message.NewPrinter(language.Dutch)
m.Printf("Hoogte: %.1f meter\n", 1244.9) // Prints Hoogte: 1.244,9 meter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants