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

Allow custom key separators for JSON based file formats and exporters. #11390

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

mkarol-neurosys
Copy link
Contributor

Proposed changes

During discussion in translate/translate#5179 I realized that Weblate during export is losing key structure by not escaping dots in the natural language keys. Instead of escaping and unescaping separators throughout whole application I've implemented support for custom key separator allowing for simple import and export of files containing non-default key separators.

Checklist

  • Lint and unit tests pass locally with my changes.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added documentation to describe my feature.
  • I have squashed my commits into logic units.
  • I have described the changes in the commit messages.

Other information

Setting up key separator when creating component from scratch. Option is also available when creating component from other sources as well it is possible to change key separator in component settings.
image

Have a great day!
Michał Karol

@mkarol-neurosys mkarol-neurosys changed the title Feature/custom key separators Allow custom key separators for JSON based file formats and exporters. Apr 11, 2024
Copy link
Member

@nijel nijel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution, unfortunately I see several issues with it:

  • The setting is done at the component level, but doesn't affect file loading when parsing files from the repository. That is confusing.
  • When introducing parameters for file formats, we would rather need something more generic, not adding arbitrary parameters for few formats. See Parameters for File formats #1897
  • Hard-coding separator to . would break YAML as it historically uses ->, see https://github.com/translate/translate/blob/fea03126d2421454bb6d7f029eff2342c681dc30/translate/storage/yaml.py#L32 (I see you've limited this to JSON subclasses only, but this makes the setting behavior inconsistent)
  • Generally, Weblate preserves whatever is in the file in the round-trip. What is the motivation to add this specific exporter and not just save changed files within Weblate?

weblate/templates/component.html Outdated Show resolved Hide resolved
nijel pushed a commit that referenced this pull request Apr 11, 2024
It was supposed to be moved in cc260c2, but it was copied.

See #11390
@mkarol-neurosys
Copy link
Contributor Author

Ad 1) When loading from vsc, after setting the source, there should be and option to specify key separator on ComponentCreateForm. Later, when celery task is performing weblate/trans/models/component.py:2448 check_sync function it uses weblate/trans/models/translation.py:268 self.component.file_format_cls which uses wrapped file format class from component.

Ad 2) This is currently rather quick fix, I am planning to add changes to translate to allow for key separator customization. If you want I can start with changes to the translate and utilizing those changes in weblate later on.

Ad 3) Like I said, it is first step.

Ad 4) Unfortunately when some edits are made to the translations, there is no possibility to export changed i18nextv4 translations. JSON and JSON nested formats are stripping out pluras, and i18nextv4 exporter alone will nest keys in resulting JSON regardless whether the dot was inside the key eg. { "Natural language key. Will be nested": "some translation" }.

@nijel
Copy link
Member

nijel commented Apr 23, 2024

Unfortunately when some edits are made to the translations, there is no possibility to export changed i18nextv4 translations.

You don't need to convert the format via exporters when changing existing i18nextv4 translation. Just download the file as it is. The format exporting is there for interoperability with a few formats, and not as a generic framework for converting anything to anything.

@mkarol-neurosys
Copy link
Contributor Author

Unfortunately there is no other way to download only reviewed and accepted translations. Exporters allow for selecting subset of translations without downloading whole file.

@nijel
Copy link
Member

nijel commented May 7, 2024

I understand your need, but I see this more like a workaround than a solution.

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

Successfully merging this pull request may close these issues.

None yet

2 participants