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

Fix and improve i18n support #1258

Merged

Conversation

benoit-pierre
Copy link
Member

@benoit-pierre benoit-pierre commented Apr 15, 2021

Summary of changes

The bad news is that our i18n support is broken, particularly for plugins that would want to use it. The good news is that since nobody is going to be using it, we can fix without fear of regressions!

See individual commits messages for a detail of the change, as well as the new documentation.

Pull Request Checklist

  • Changes have tests
  • News fragment added in news.d. See documentation for details

@benoit-pierre benoit-pierre force-pushed the fix_and_improve_i18n_support branch from c85dca5 to dbe8424 Compare April 15, 2021 01:10
@benoit-pierre
Copy link
Member Author

And I broke all the tests... Because setup.py will import plover, which will try to setup thei18n support and that will fail because the .egg-info directory is not present.

@benoit-pierre
Copy link
Member Author

I added a workaround:

  • use exec in setup.py
  • detect exec is being used in plover, and disable i18n support

Note: in this case ("import" from setup.py), we don't want i18n support anyway, since some of the strings are used for package metadata.

@benoit-pierre benoit-pierre force-pushed the fix_and_improve_i18n_support branch 2 times, most recently from 2bd4211 to 1a44c72 Compare April 15, 2021 01:53
@benoit-pierre
Copy link
Member Author

Fixed the Linux build, as well as some is_release checks in the packaging job.

doc/i18n.md Outdated Show resolved Hide resolved
doc/i18n.md Outdated Show resolved Hide resolved
doc/i18n.md Outdated Show resolved Hide resolved
doc/i18n.md Outdated Show resolved Hide resolved
doc/i18n.md Outdated Show resolved Hide resolved
doc/i18n.md Outdated Show resolved Hide resolved
doc/i18n.md Show resolved Hide resolved
doc/i18n.md Outdated Show resolved Hide resolved
Look for localizations data in $CONFIG_DIR/messages.
As it turns out, there's no need to manually register Babel'
setuptools commands.

Add a `babel_options` helper to `plover_build_utils` for use
by plugins to easily support translating their UI.
- Don't use a global `_` builtin (which might conflict with other
  packages): assume the top package will provide a gettext like
  `_` method.
- Fix tool plugins using the wrong language (because the i18n init
  would happen after importing those modules).
- Move the `i18n` support module as well as `messages` directory one
  level up so it's possible to translate parts of the core too.
- Improve the `gettext` `build_ui` hook: automatically add
  comments giving translators a little more context about each
  translation.
- Configure Babel to extract comments preceding a translation
  (when they are tagged with `# i18n:`).
@benoit-pierre benoit-pierre force-pushed the fix_and_improve_i18n_support branch from 42718b4 to b9da326 Compare April 15, 2021 02:15
@benoit-pierre benoit-pierre force-pushed the fix_and_improve_i18n_support branch from 5aaa058 to b1f6005 Compare April 15, 2021 22:34
@benoit-pierre
Copy link
Member Author

Of course this broke towncrier too... Anyway I think this is finally ready now.

- use `exec` in `setup.py`
- detect `exec` is being used in `plover`, and disable i18n support

Note: in this case, we don't want i18n support anyway, since some
of the strings we normally translate are used for package metadata.
Can't import `plover` without the necessary dependencies anymore
because of i18n support: use the same workaround as for `setup.py`.
Work around towncrier trying to import the `plover` package: fill-in
empty `name` and `version` values in the configuration. We don't use
the project name in the template, and the version can be specified
with `--version` when we do actually need it.
- there's no need to specify the version when drafting
- it's better to use `python -m towncrier` (to make sure
  we're using the correct version / Python interpreter)
@benoit-pierre benoit-pierre force-pushed the fix_and_improve_i18n_support branch from b1f6005 to 7bd6194 Compare April 16, 2021 03:46
@benoit-pierre
Copy link
Member Author

And I broke the dictionaries file picker:

diff --git a/plover/gui_qt/dictionaries_widget.py b/plover/gui_qt/dictionaries_widget.py
index 74c04688..4ffaf346 100644
--- a/plover/gui_qt/dictionaries_widget.py
+++ b/plover/gui_qt/dictionaries_widget.py
@@ -40,7 +40,7 @@ def _dictionary_filters(include_readonly=True):
     formats = sorted(_dictionary_formats(include_readonly=include_readonly))
     filters = ['*.' + ext for ext in formats]
     # i18n: Widget: “DictionariesWidget”, file picker.
-    filters = [_('Dictionaries ({extensions}').format(extensions=' '.join(filters))]
+    filters = [_('Dictionaries ({extensions})').format(extensions=' '.join(filters))]
     filters.extend(
         # i18n: Widget: “DictionariesWidget”, file picker.
         _('{format} dictionaries ({extensions})').format(

All good now! I think...

@benoit-pierre benoit-pierre merged commit 3c66ce2 into openstenoproject:master Apr 16, 2021
@benoit-pierre benoit-pierre deleted the fix_and_improve_i18n_support branch April 16, 2021 03:49
@benoit-pierre
Copy link
Member Author

@Sillabix, @nvdaes: could you update the Italian / Spanish translations? This should be the last translations update before the final release. Here is the latest release, and do check the new i18n documentation. Pretty much everything should be translatable now, except for plugins (plugins manager included).

@nvdaes
Copy link
Contributor

nvdaes commented Apr 16, 2021

Í will update the spanish translation this weekend, thanks.

@Sillabix
Copy link
Contributor

Ok, same for me

nvdaes added a commit to nvdaes/plover that referenced this pull request Apr 17, 2021
Requested by @benoit-pierre in PR openstenoproject#1258

News fragment added in news.d/feature/1165.ui.md
news.d/feature/1165.ui.md:
@nvdaes nvdaes mentioned this pull request Apr 17, 2021
2 tasks
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.

4 participants