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 support for go-i18n v2 file #4323

Closed
hanzei opened this issue Aug 17, 2020 · 14 comments
Closed

Add support for go-i18n v2 file #4323

hanzei opened this issue Aug 17, 2020 · 14 comments
Assignees
Labels
backlog This is not on the Weblate roadmap for now. Can be prioritized by sponsorship. enhancement Adding or requesting a new feature. translate-toolkit Issues which need to be fixed in the translate-toolkit
Milestone

Comments

@hanzei
Copy link

hanzei commented Aug 17, 2020

Describe the bug

Loading a go-i18n file into a new component fails with 'str' object has no attribute 'get'

To Reproduce

Steps to reproduce the behavior:

  1. Open dialog to create new component with custom file type setting
  2. Use a repo that contains a go-i18n file e.g. https://github.com/mattermost/mattermost-plugin-jitsi
  3. Set the filemask to match the existing translation files e.g. assets/i18n/active.*.json
  4. Click Save
  5. Observe error

Expected behavior

A new component get created and the existing translation loaded.
Screenshots

Screenshot from 2020-08-17 12-33-20
Screenshot from 2020-08-17 12-33-32

Server configuration and status

Will provide this later. I'm using Weblate 4.1.1.

Weblate deploy checks

Will provide this later.

Exception traceback

None

Additional context

N/A

@sentry-io
Copy link

sentry-io bot commented Aug 17, 2020

Sentry issue: WEBLATE-4K3

@nijel
Copy link
Member

nijel commented Aug 17, 2020

Use "JSON file" as file format in this case. The go-i18n format needs structured data at least with id and translation attributes. See also #2558

@nijel nijel self-assigned this Aug 17, 2020
@nijel nijel added the question This is more a question for the support than an issue. label Aug 17, 2020
@github-actions
Copy link

This issue looks like a support question. We try to answer these reasonably fast, but in case you are looking for faster resolution, please consider purchasing support subscription and make Weblate stronger.

@hanzei
Copy link
Author

hanzei commented Aug 17, 2020

@nijel According to the source code of github.com/nicksnyder/go-i18n/v2 the go-i18n data doesn't contain a translation attribute. Hence, the test data at https://github.com/translate/translate/pull/4037/files#diff-5b954663f244c2e1d32bce2961aa1104R41 seems wrong. The data looks something like:

  "$ID": {
    "hash": "sha1-$SOME_SHA",
    "other": "I am a translation"
  }

@nijel
Copy link
Member

nijel commented Aug 17, 2020

See #2558 for the discussion on the format. The go-i18n consumes wide range of JSON variants and what Weblate calls go-i18n is only one of these. I have no knowledge whether it is the most widely used one or not, but nobody did comment on that in #2558 (comment)

@hanzei
Copy link
Author

hanzei commented Aug 18, 2020

Actually, I think the data structure go-i18n consumes is quite well defined. Currently, the format may vare. Please note that is issue is not a request to support more then JSON.

Maybe the confusion comes from the different formats v1 and v2 use:
v1:

  • Supports yaml and json
  • Uses an id and translation atribute
  • Sample data:
{
    "id": "your_unread_email_count",
    "translation": {
      "one": "You have {{.Count}} unread email.",
      "other": "You have {{.Count}} unread emails."
    }
}

v2:

  • Supports yaml, json and toml
  • Uses a map[id]translation style
  • Sample data:
{
  "your_unread_email_count": {
    "hash": "sha1-e23eeb209a3a322e1c1174d7816d2ccbca52080b",
    "one": "You have {{.Count}} unread email.",
    "other": "You have {{.Count}} unread emails."
  }
}

According to the docs (https://docs.weblate.org/en/latest/formats.html#go-i18n-json-files) only v2 is supported by weblate. Is the documentation correct here?

@nijel
Copy link
Member

nijel commented Aug 18, 2020

Is this documented somewhere? All I was able to find were tests, which seem parse pretty much any JSON file, see https://github.com/nicksnyder/go-i18n/blob/949485dac070df78fc9b06c9758facf6c41a6470/v2/i18n/parse_test.go#L21-L140

What is actually supported in Weblate is what you describe as v1 (see the test data: https://github.com/translate/translate/blob/441f8ff70e0a81b99d9f133ab7a040797d24974d/translate/storage/test_jsonl10n.py#L55-L70), but I was told this is v2 when implementing this.

@hanzei
Copy link
Author

hanzei commented Aug 21, 2020

The output weblate currently produces is correctly parsable by go-i18n v2. But weblates fails to read existing translation files from go-i18n v2.

You can find an example for toml at https://github.com/nicksnyder/go-i18n/blob/949485dac070df78fc9b06c9758facf6c41a6470/v2/example/active.es.toml.

@nijel
Copy link
Member

nijel commented Aug 21, 2020

Part of the problem is that go-i18n will happily parse nearly any JSON, what makes it hard to determine what is the actual current format. Especially when there is no documentation describing the formats itself besides "Supports message files of any format (e.g. JSON, TOML, YAML)".

I think we can rename existing format to v1 and implement support for v2. Is there some specification for the hash field?

@nijel nijel changed the title Fail to load go-i18n file Fail to load go-i18n v2 file Aug 21, 2020
@nijel nijel added backlog This is not on the Weblate roadmap for now. Can be prioritized by sponsorship. enhancement Adding or requesting a new feature. translate-toolkit Issues which need to be fixed in the translate-toolkit and removed question This is more a question for the support than an issue. labels Aug 21, 2020
nijel added a commit that referenced this issue Aug 21, 2020
Apparently, what we support is v1, see #4323
@github-actions
Copy link

This issue has been added to the backlog. It is not scheduled on our roadmap, but it eventually might be implemented. In case you desperately need this feature, please consider helping or funding the development.

@github-actions
Copy link

The issue you've reported needs to be addressed in the translate-toolkit. Please file the issue there and do not forget to include links to any relevant specifications about the formats (if applicable).

@hanzei
Copy link
Author

hanzei commented Aug 25, 2020

Thanks for updating the docs 👍

From the docs: Hash uniquely identifies the content of the message that this message was translated from.

@nijel
Copy link
Member

nijel commented Aug 25, 2020

That explains nothing. In case Weblate is supposed to update that, it needs to know how it is calculated. Yes, I can probably read the code to figure it out, but that's not what I prefer.

Honestly, go-i18n is terrible in this - the formats are not documented, the changes between v1 and v2 are not documented, basically there is no documentation besides describing the API. People should first look at existing formats (there are already few dozens of them) before inventing yet another one for good reason...

@nijel nijel changed the title Fail to load go-i18n v2 file Add support for go-i18n v2 file Nov 9, 2020
@nijel nijel removed their assignment Jan 13, 2021
@nijel nijel self-assigned this Feb 27, 2023
@nijel nijel added this to the 4.16 milestone Feb 27, 2023
@nijel nijel closed this as completed in 3601981 Feb 27, 2023
@github-actions
Copy link

Thank you for your report; the issue you have reported has just been fixed.

  • In case you see a problem with the fix, please comment on this issue.
  • In case you see a similar problem, please open a separate issue.
  • If you are happy with the outcome, don’t hesitate to support Weblate by making a donation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog This is not on the Weblate roadmap for now. Can be prioritized by sponsorship. enhancement Adding or requesting a new feature. translate-toolkit Issues which need to be fixed in the translate-toolkit
Projects
None yet
Development

No branches or pull requests

2 participants