-
-
Notifications
You must be signed in to change notification settings - Fork 297
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
implementing support for translations in bukuserver #782
Conversation
@@ -120,6 +120,7 @@ The following are os env config variables available for bukuserver. | |||
| REVERSE_PROXY_PATH | reverse proxy path | string | | |||
| THEME | [GUI theme](https://bootswatch.com/3) | string [default: `default`] (`slate` is a good pick for dark mode) | | |||
| LOCALE | GUI language (partial support) | string [default: `en`] | | |||
| DEBUG | debug mode (verbose logging etc.) | boolean [default: `false`] | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added an option to enable Flask debug mode (mostly for development purposes)
except Exception: | ||
return str(s) | ||
|
||
__all__ = ['_', '_p', '_l', '_lp', '_key', 'gettext', 'pgettext', 'ngettext', 'lazy_gettext', 'lazy_pgettext', 'LazyString'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to the messy i18n setup in Flask-Admin that needs to be matched, I ended up implementing a similar setup with a primitive fallback, and a few aliases for local usage.
_key()
directly replicates Flask-Admin logic for handling translation strings (and is meant to be used in similar circumstances).
IN_LIST = {'func': in_list_func, 'text': _l('in list')} | ||
NOT_IN_LIST = {'func': not_in_list_func, 'text': _l('not in list')} | ||
TOP_X = {'func': top_x_func, 'text': _l('top X')} | ||
BOTTOM_X = {'func': bottom_x_func, 'text': _l('bottom X')} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gettext()
/_()
requires Jinja context at the time of invocation, so I'm using lazy_gettext()
/_l()
here instead
try: | ||
self.index = ['name', 'usage_count'].index(name) | ||
except ValueError as e: | ||
raise ValueError(f'name: {name}') from e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A minor simplification
if _key(self.operation_text) in ('in list', 'not in list'): | ||
super().__init__(name, options, data_type='select2-tags') | ||
else: | ||
super().__init__(name, options, data_type) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixes list filters in Bookmarks list
FIELDS = ['index', 'url', 'title', 'description', 'tags'] | ||
|
||
def __init__(self, field, *args, **kwargs): | ||
self.field = field | ||
super().__init__('order', *args, options=self.DIR_LIST, **kwargs) | ||
|
||
def operation(self): | ||
return 'by ' + self.field | ||
return _l(f'by {self.field}') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leaving _l('by ' + self.field)
would result in pybabel misdetecting a "by "
translation key 😅
_(" - '#,' is the same but will match FULL tags only"), | ||
_(" - '*' will be searched for in all fields (this prefix can be omitted in the 1st keyword)"), | ||
_('Keywords need to be separated by placing spaces before the prefix.'), | ||
]))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Splitting these into separate lines and joining immediately at the time of printing is more convenient than trying to deal with the whole block of text as a single message key.
url = StringField(_l('URL'), name='link', validators=[InputRequired()]) | ||
title = StringField(_l('Title')) | ||
tags = StringField(_l('Tags')) | ||
description = TextAreaField(_l('Description')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The names need to be provided explicitly, both to detect them when generating translation files and to apply translation to the field name.
try: # as per Flask-Admin-1.6.1 | ||
try: | ||
from flask_babelex import Babel | ||
Babel(app).localeselector(lambda: app.config['BUKUSERVER_LOCALE']) | ||
except ImportError: | ||
from flask_babel import Babel | ||
Babel().init_app(app, locale_selector=lambda: app.config['BUKUSERVER_LOCALE']) | ||
app.context_processor(lambda: {'lang': app.config['BUKUSERVER_LOCALE'] or 'en', **context_processor()}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making lang
accessible from the template
except Exception as e: | ||
app.jinja_env.add_extension('jinja2.ext.i18n') | ||
app.jinja_env.install_gettext_callables(gettext, ngettext, newstyle=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Applying fallback functions so that the app still works without Flask-Babel(Ex)
|
||
|
||
def create_app(db_file=None): | ||
"""create app.""" | ||
app = FlaskAPI(__name__) | ||
os.environ.setdefault('FLASK_DEBUG', ('1' if get_bool_from_env_var('BUKUSERVER_DEBUG') else '0')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The normal way to enable debug mode doesn't seem to be accessible from this code, so I'm using the variable fallback here.
@@ -117,6 +122,7 @@ def shell_context(): | |||
return {'app': app, 'bukudb': bukudb} | |||
|
|||
app.jinja_env.filters.update(util.JINJA_FILTERS) | |||
app.jinja_env.globals.update(_p=_p) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exposing _p()
(pgettext()
aka translation with context) as a function in the templates
admin.add_view(views.StatisticView(bukudb, 'Statistic', endpoint='statistic')) | ||
admin.add_view(views.BookmarkModelView(bukudb, _l('Bookmarks'))) | ||
admin.add_view(views.TagModelView(bukudb, _l('Tags'))) | ||
admin.add_view(views.StatisticView(bukudb, _l('Statistic'), endpoint='statistic')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Navigation tabs
.filters .filter-op {width: 280px !important} | ||
.filters .filter-val {width: 595px !important} | ||
:root {--filters: 1130px; --filter-op: 20em} | ||
html[lang=ru] #filter_form[action^='/bookmark/'] {--filter-op: 25em} /* the last 'buku' filter has a rather long name */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some adjustment is required for localized sizes (which unfortunately cannot be done with flexbox due to how these filters were implemented in the first place
}); | ||
}).appendTo(this); | ||
}) | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Converted this into a macro, to enable l10n
@@ -3,6 +3,7 @@ | |||
|
|||
{% block tail %} | |||
{{ super() }} | |||
{{ buku.set_lang() }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sets the lang=
attribute of the main <html>
element.
{% endif %} | ||
{% endblock %} | ||
|
||
{% block tail %} | ||
{{ buku.fix_translations('bookmarks') }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to how filter groups are implemented in Flask-Admin, they have to be fixed in a very specific way to support l10n (even though the data model itself would allow for it)
<input type="hidden" name="markers" value="true"/> | ||
<input type="hidden" name="all_keywords" value="true"/> | ||
</div> | ||
<button type="submit" class="btn btn-default">Search</button> | ||
<button type="submit" class="btn btn-default">{{ _gettext('Search') }}</button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using Flask-Admin translation for this string
<dt>in Firefox:</dt> | ||
<dd>Open the bookmarks editor and set <code>@buku</code> in the Keyword field of the bookmarklet.</dd> | ||
<dt>{{ _('in Firefox:') }}</dt> | ||
<dd>{{ _('Open the bookmarks editor and set %(buku)s in the Keyword field of the bookmarklet.', buku='<code>@buku</code>'|safe) }}</dd> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Injecting nested HTML as string parameters
@@ -35,7 +35,7 @@ | |||
<script> | |||
$('.admin-form [name=fetch]').remove(); | |||
$('.admin-form').append( | |||
$(`<div class="form-group"><label style="display: block"><span class="col-md-2 text-right">{{ _gettext('Fetch') }} </span>` | |||
$(`<div class="form-group"><label style="display: block"><span class="col-md-2 text-right">{{ _('Fetch') }} </span>` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Flask-Admin doesn't have this translation key (or at least an appropriate translation for it); switching to using app translations.
'bookmarks': {'id': _p('bookmarks', 'index'), 'url': _p('bookmarks', 'url'), 'title': _p('bookmarks', 'title'), | ||
'tags': _p('bookmarks', 'tags'), 'order': _p('bookmarks', 'order')}} %} | ||
<script>{ | ||
$(`button[title="Delete record"]`).attr('title', {{ _('Delete record')|tojson }}); // see flask-admin issue #1974 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Delete action button is left untranslated in Flask-Admin for some reason
(Also, using namespaced strings for filter group names to avoid confusion)
let text = $(this).text(); | ||
$(this).text(FILTERS[text] || text); | ||
}); | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filter data is generated and printed out as raw JSON in Flask-Admin, then it's immediately used to initialize filters form. (And the add-filter dropdown is rendered separately.) More importantly, it has to be translated on the frontend, because backend supports translations for filters but not filter groups.
Thus, to translate the filters data I had to substitute values in the JSON immeadiately after they got rendered on page.
const UNITS = {day: 60*60*24, hour: 60*60, minute: 60, second: 1}; | ||
const AGO = {{ {'day': _('{} days ago'), 'hour': _('{} hours ago'), 'minute': _('{} minutes ago'), 'second': _('{} seconds ago')}|tojson }}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't quite how translations with numbers are meant to be used, but this code runs in the browser.
<script> | ||
{% set NO_NETLOC = '\u200B' + _('(no netloc)') + '\u200B' -%} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved into the <script>
since NO_TITLE
is already within it
``` | ||
Will run the `__main__.py` script (if running from a different folder, pass relative path to it instead of `.`) | ||
```sh | ||
python -m bukuserver.translations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In short, python -m bukuserver.translations
(when run within venv) does everything that is needed when working with translations (if you want to add new locales, just add them as parameters to the command)
f'^# FIRST AUTHOR <EMAIL@ADDRESS>, [0-9]+.{_EOL}': '', | ||
f'^#, fuzzy{_EOL}': '', | ||
r'(?<=^"POT-Creation-Date: ).*(?=\\n"$)': '2024-09-12 00:00+0000', # avoid git updates of unchanged translations | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Base substitutions for generated template
ctxt_re = ('' if _ctxt is None else f'msgctxt "{re.escape(_ctxt)}"{_EOL}') | ||
if m := re.search(f'^{ctxt_re}msgid "{re.escape(_id)}"{_EOL}msgstr "()"{_EOL}', text, re.MULTILINE): | ||
text = (text[:m.start(1)] + _str + text[m.end(1):]).replace(obsolete.group(0), '', 1) | ||
return text |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This saves us the hassle of dealing with those obsolete strings that can be handled automatically (the blank ones can be stripped, and re-added keys can be restored to their previous values).
def translations_generate(): | ||
'''Generates and patches the messages.pot template file''' | ||
pybabel().run(['', 'extract', '--no-wrap', f'--mapping-file={MAPPING}', | ||
'--keywords=_ _l _p:1c,2 _lp:1c,2 lazy_gettext', f'--output-file={TEMPLATE}', BUKUSERVER]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pybabel()
hardcodes argv handling without stripping first parameter in CLI, so when invoking from the code an extra string at the beginning is needed to represent the (unused) program name
|
||
def translations_compile(update=False, generate=True, domain=DOMAIN, new_locales=[], fuzzy=False): | ||
'''Compiles all existing translations''' | ||
update and translations_update(generate=generate, domain=DOMAIN, new_locales=new_locales, fuzzy=fuzzy) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With correct parameters, a single translations_compile()
call is sufficient to run all functionality of the script at once
@@ -0,0 +1,2 @@ | |||
[python: **.py] | |||
[jinja2: **/templates/**.html] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When generating translations template file, calls to gettext()
/pgettext()
/lazy_gettext()
/_()
/_p()
/_l()
/_lp()
are collected from Python and Jinja sources
@@ -0,0 +1 @@ | |||
/messages.pot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is always generated automatically, and isn't really necessary to be included
"POT-Creation-Date: 2024-09-12 00:00+0000\n" | ||
"PO-Revision-Date: 2024-09-08 19:42+0200\n" | ||
"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n" | ||
"Language: de\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
de
/fr
translation files were added but only a few strings were translated in them
@@ -0,0 +1,53 @@ | |||
msgid "Original and replacement tags are the same." | |||
msgstr "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These strings can't be detected so I'm appending them after template generation (e.g. exception messages or generated text)
"error", | ||
) | ||
msg = _('Failed to create record.') | ||
flash('%(msg)s %(error)s' % {'msg': msg, 'error': _(str(ex))}, 'error') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no point in using gettext()
to format a string that includes no actual words 😅
@@ -425,28 +415,32 @@ class TagModelView(BaseModelView, ApplyFiltersMixin): | |||
def _create_ajax_loader(self, name, options): | |||
pass | |||
|
|||
def _name_formatter(self, _, model, name): | |||
def _name_formatter(self, context, model, name): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This parameter actually has to be named context
for non-lazy gettext()
to work (…and the lazy version ignores escape()
somehow 😅)
"name": _name_formatter, | ||
} | ||
column_filters = ['name', 'usage_count'] | ||
column_labels = {'name': _lp('tag', 'Name'), 'usage_count': _lp('tag', 'Usage Count')} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using namespaced strings here to allow for tag-specific translations of these labels
list_template = 'bukuserver/tags_list.html' | ||
edit_template = "bukuserver/tag_edit.html" | ||
named_filter_urls = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enabled readable filter names in URLs
if (arrow.now() - self.refreshed).seconds > 60: # automatic refresh if more than a minute passed since last one | ||
self._refresh() | ||
else: | ||
app.logger.debug('Tags cache refreshed %ss ago', (arrow.now() - self.refreshed).seconds) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensured that tags data does not get more stale than 1 minute (with some logging when in debug mode)
reverse=sort_desc, | ||
) | ||
) | ||
tags = sorted(tags, reverse=sort_desc, key=lambda x: x[sort_field_dict[sort_field]]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorted()
returns a list anyway
@@ -525,19 +517,20 @@ def top_most_common_func(query, value, index): | |||
bs_filters.TagBaseFilter(name, filter_type=FilterType.NOT_EQUAL), | |||
bs_filters.TagBaseFilter(name, filter_type=FilterType.IN_LIST), | |||
bs_filters.TagBaseFilter(name, filter_type=FilterType.NOT_IN_LIST), | |||
bs_filters.TagBaseFilter(name, filter_type=FilterType.CONTAINS), | |||
bs_filters.TagBaseFilter(name, filter_type=FilterType.NOT_CONTAINS), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…Just noticed these were added to the usage_count
filter group as well (even though they expect strings and not numbers 😅)
else: | ||
raise ValueError | ||
finally: | ||
rmdb(bdb) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I'm fixing some testing regressions on Windows (or rather, errors that did not happen to pop up before some of the more recent tests got added)
@@ -28,7 +28,7 @@ def app(dbfile): | |||
yield app | |||
# clean up / reset resources here | |||
|
|||
def env_fixture(name, **kwargs): | |||
def env_fixture(name, **kwargs): # place this fixture BEFORE app or its dependencies |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When an env_fixture
is combined with some app
-related fixtures (like client
), it has to be placed before any of them to actually have any effect
@@ -287,7 +287,7 @@ def test_env_per_page(bukudb, app, client, total, per_page, pages, last_page): | |||
|
|||
response = client.get('/bookmark/last-page', follow_redirects=True) | |||
dom = assert_response(response, '/bookmark/', args={'page': str(pages - 1)}) | |||
cells = dom.xpath(f'//td{xpath_cls("col-Entry")}') | |||
cells = dom.xpath(f'//td{xpath_cls("col-entry")}') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed the column key to lowercase since it's displayed using the original translation string for rendering anyway
f'//td{xpath_cls("list-buttons-column")}/a/@title': ['Просмотр записи', 'Редактировать запись'], | ||
f'//td{xpath_cls("list-buttons-column")}/form/button/@title': ['Delete record']}, | ||
f'//td{xpath_cls("list-buttons-column")}/form/button/@title': ['Delete record']}, # ['Удалить запись']}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even though "Delete record" is a Flask-Admin string, it's not included in their translation files; I've added a fix for that while I was at it but it's replaced on the frontend due to potential complexity of trying to replace Flask-Admin code that renders it
strings = _DICT[locale or 'en'] | ||
_add_rec(bukudb, 'http://example.com') | ||
if locale: | ||
app.config.update({'BUKUSERVER_LOCALE': locale}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Turns out, this approach doesn't actually work with app translations (as opposed to Flask-Admin ones)
This was split off from #779 once I realized it's becoming quite a large changeset 😅
Since we have basic i18n support in Bukuserver, I've decided to try implementing l10n for all UI strings. (Also fixed a few minor bugs here and there.)
I've completed strings for the
ru
locale (mostly to make sure all these translation strings actually work properly); any others can be contributed by whoever is actually interested in having them (I've included a simple script to add/update/recompile translation files, minimizing the effort required to manage them).Screenshots (ru)
(list filters now work again)