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

Add TOML support for language files #2577

Closed
bep opened this issue Oct 14, 2016 · 11 comments · Fixed by #3257
Closed

Add TOML support for language files #2577

bep opened this issue Oct 14, 2016 · 11 comments · Fixed by #3257
Assignees
Milestone

Comments

@bep
Copy link
Member

bep commented Oct 14, 2016

Doing this in YAML is frustration galore.

EDIT: https://github.com/nicksnyder/go-i18n

@moorereason
Copy link
Contributor

Why is this an upstream issue? What's missing?

@bep
Copy link
Member Author

bep commented Oct 14, 2016

Why is this an upstream issue? What's missing?

It is maybe more clear now. The snyder lib does the parsing and currently only supports JSON and YAML.

@n10v
Copy link
Contributor

n10v commented Nov 19, 2016

The question is, which structures should be used. I mean, there are no anonymous tables in TOML and it should be implemented in another way. For example, how could be written following structures in TOML?

[
  {
    "id": "settings_title",
    "translation": "Settings"
  }
]
[
  {
    "id": "d_days",
    "translation": {
      "one": "{{.Count}} day",
      "other": "{{.Count}} days"
    }
  }
]

Like this?

[[Translations]]
id = "settings_title"
translation = "Settings"
[[Translations]]
id = "d_days"

[[Translations.translation]]
  one = "{{.Count}} day"
  other = "{{.Count}} days"

IMO YAML is more convenient for these things

@bep
Copy link
Member Author

bep commented Nov 19, 2016

[[d_days]]
one = "{{.Count}} day"
  other = "{{.Count}} days"

[[string_id_2]]
translation = "Some other translation"

go-i18n's file format is probably an attempt of future-proofing, making it more complex than it should.

If go-i18n could take the data structure directly (it should), Hugo could define whatever format it likes and create an adapter for it.

With that in mind, we could also maybe create some support for PO files (or whatever they are called).

My main issue about YAML is that it is soooo easy to get the indentation wrong, and the error messages are terrible.

Maybe @nicksnyder could chime in with his thoughts on this.

@n10v
Copy link
Contributor

n10v commented Nov 19, 2016

I think, the adapter is the best option for hugo now. I will try to implement it if there will be no more ideas

@bep bep removed the Upstream label Nov 19, 2016
@nicksnyder
Copy link

go-i18n's file format is probably an attempt of future-proofing, making it more complex than it should.

go-i18n's file format is an attempt to match the data that it actually needs. Each translation object has an id and either one translation (non-plural form) or multiple translations (plural form).

If go-i18n could take the data structure directly (it should)

It can, so you can choose any format and inject translations as you see fit:
https://godoc.org/github.com/nicksnyder/go-i18n/i18n#AddTranslation

If you prefer a flatter format I would recommend treating non-plural forms exactly the same:

[[d_days]]
one = "{{.Count}} day"
other = "{{.Count}} days"

[[string_id_2]]
other = "Some other translation"

If you need to differentiate between a plural and non-plural form (the goi18n command does), then the logic would be something like "if there is only other key and the value doesn't contain {{ then it is non-plural, else plural".

The goi18n command helps manage translation files so if you want to use it, then you would either need to be able to either (1) convert TOML to YAML or JSON, or (2) add support to go-i18n for TOML.

If there is general agreement that a flatter format would be better (even in JSON and YAML) then I am open to changing go-i18n.

@n10v
Copy link
Contributor

n10v commented Nov 19, 2016

@nicksnyder but support for TOML will break all existing API of go-i18n package. This moment should be discussed in separate issue at go-i18n's page. But for hugo at the moment the adapter should be implemented.
IMHO it looks not so good to hold only one other for non-plural form. It looks nicer as bep has offered

@bep
Copy link
Member Author

bep commented Nov 19, 2016

But for hugo at the moment the adapter should be implemented.

We have no real rush; @nicksnyder is right about the translation workflows, and we don't want to add additional glue code in Hugo if we don't really have to.

I'll create an issue on the go-i18n tomorrow where we can continue this discussion. A flattening of the data format would be breaking, but it should be fairly easy to support both in a grace period.

@nicksnyder
Copy link

but support for TOML will break all existing API of go-i18n package

It doesn't need to. We can support a new format without dropping support for the old format.

n10v added a commit to n10v/go-i18n that referenced this issue Mar 20, 2017
n10v added a commit to n10v/go-i18n that referenced this issue Mar 20, 2017
n10v added a commit to n10v/go-i18n that referenced this issue Mar 20, 2017
n10v added a commit to n10v/go-i18n that referenced this issue Mar 20, 2017
n10v added a commit to n10v/go-i18n that referenced this issue Mar 20, 2017
n10v added a commit to n10v/go-i18n that referenced this issue Mar 20, 2017
n10v added a commit to n10v/go-i18n that referenced this issue Mar 20, 2017
@n10v n10v self-assigned this Mar 21, 2017
n10v added a commit to n10v/go-i18n that referenced this issue Mar 21, 2017
n10v added a commit to n10v/go-i18n that referenced this issue Apr 1, 2017
nicksnyder pushed a commit to nicksnyder/go-i18n that referenced this issue Apr 1, 2017
@nicksnyder
Copy link

go-i18n 1.8 now supports TOML and a flatter file format
https://github.com/nicksnyder/go-i18n/releases/tag/v1.8.0

Thanks @BoGeM !

@n10v n10v added this to the v0.20 milestone Apr 2, 2017
n10v added a commit to n10v/hugo that referenced this issue Apr 2, 2017
@bep bep closed this as completed in #3257 Apr 2, 2017
bep pushed a commit that referenced this issue Apr 2, 2017
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants