Internationalization of kiwix-serve's backend #679
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
veloman-yunkan
force-pushed
the
kiwix-serve-i18n
branch
2 times, most recently
from
cb659d9
to
ffb5373
Compare
Codecov Report
@@ Coverage Diff @@
## master #679 +/- ##
==========================================
+ Coverage 60.55% 61.09% +0.54%
==========================================
Files 56 58 +2
Lines 3732 3789 +57
Branches 2060 2080 +20
==========================================
+ Hits 2260 2315 +55
- Misses 1471 1473 +2
Partials 1 1
Continue to review full report at Codecov.
|
veloman-yunkan
force-pushed
the
kiwix-serve-i18n
branch
2 times, most recently
from
0a6b256
to
7f9f562
Compare
veloman-yunkan
changed the title
veloman-yunkan
force-pushed
the
kiwix-serve-i18n
branch
from
7f9f562
to
f82bd4c
Compare
|
@mgautierfr This PR is becoming pretty big. I think that it makes sense for you to throw a quick glance at it lest I move too far ahead with an approach that you may criticize in the end. Please note that intermediate commits contain some temporary solutions that get replaced in the end. The current state reflects my vision of how error messages and templates should be translated in the C++ code. |
veloman-yunkan
force-pushed
the
kiwix-serve-i18n
branch
from
99c18eb
to
04d0f46
Compare
|
@veloman-yunkan Thx for this PR trying to tackling the complex problem of internationalisation. Unfortunately @mgautierfr is pretty busy these weeks and it might take a two weeks before you get a feedback from him. That said, I believe 5os PR desserves an explication comment, 5is is really too big to be obvious. |
|
I am planning to split this PR into at least two parts
|
veloman-yunkan
force-pushed
the
kiwix-serve-i18n
branch
from
04d0f46
to
6c5d6c8
Compare
veloman-yunkan
changed the base branch from
master
to
unittest_for_suggestions
veloman-yunkan
force-pushed
the
kiwix-serve-i18n
branch
from
6c5d6c8
to
79c0b36
Compare
veloman-yunkan
changed the base branch from
unittest_for_suggestions
to
unittests_for_404_html_page
mgautierfr
force-pushed
the
unittests_for_404_html_page
branch
2 times, most recently
from
80ecc77
to
34d069e
Compare
veloman-yunkan
force-pushed
the
kiwix-serve-i18n
branch
from
79c0b36
to
dbe9108
Compare
veloman-yunkan
force-pushed
the
kiwix-serve-i18n
branch
2 times, most recently
from
e8d8b63
to
3d576df
Compare
veloman-yunkan
changed the base branch from
master
to
safer_testing_of_html_responses
veloman-yunkan
force-pushed
the
kiwix-serve-i18n
branch
from
3d576df
to
f8b3d2b
Compare
veloman-yunkan
changed the base branch from
master
to
testing_of_ft_search_unavailable_page
The (failing) tests now demonstrate that some text in the taskbar is not translated. Will fix in the next commit.
The new test fails since the "Go to the main page" button is not yet internationalized.
The new test fails since the "Go to random page" button is not yet internationalized.
At this point a potential issue has been revealed. Now we produce
the final HTML via 2-level template expansion
1. Render parameterized messages
2. Render the HTML template
In which templates we should use double mustache "{{}}" (HTML-escaping)
tags and where we may use triple mustache "{{{}}}" (non-escaping) tags?
In the absence of the "userlang" query parameter in the URL, the value of the "Accept-Language" header is used. However, it is assumed that "Accept-Language" specifies a single language (rather than a comma separated list of languages possibly weighted with quality values). Example: Accept-Language: fr // should work Accept-Language: fr-CH, fr;q=0.9, en;q=0.8, de;q=0.7, *;q=0.5 // The requested language will be considered to be // "fr-CH, fr;q=0.9, en;q=0.8, de;q=0.7, *;q=0.5". // The i18n code will fail to find resources for such a language // and will use the default "en" instead.
mgautierfr
force-pushed
the
kiwix-serve-i18n
branch
from
4d230c7
to
927c125
Compare
|
Rebased-fixup on master. I agree with the PR as it is now. I've few concern/improvement on it but it can be handle later, I will open issues for them. |
mgautierfr
approved these changes
Apr 13, 2022
This was referenced Apr 13, 2022
veloman-yunkan
added a commit
that referenced
this pull request
Apr 14, 2022
The story of search_results.css static/skin/search_results.css was extracted from static/templates/no_search_result.html before the latter was dropped. static/templates/no_search_result.html in turn seems to be a copied and edited version of static/templates/search_result.html. In the context of exploratory work on the internationalization of kiwix-serve (PR #679) I noticed duplication of inline CSS across those two templates and intended to eliminated it. That goal was not fully accomplished (static/templates/search_result.html remained untouched) because by that time PR #679 grew too big and the efforts were diverted into splitting it into smaller ones. Thus search_results.css slipped into one of those small PRs, without making much sense because nothing really justifies preserving custom CSS in the "Fulltext search unavailable" error page. At the same time, it served as the only case where a link to a cacheable resource is generated in C++ code (rather than found in a template). This poses certain problems to the handling of cache-ids. A workaround is to expel the URL into a template so that it is processed by `kiwix-resources`. This commit merely demonstrates that solution. But whether it should be preserved (or rather the "Fulltext search unavailable" page should be deprived of CSS) is questionable.
Yes I think we should be able to ingest it. I've created this task to track the work. Can you confirm that the translatewiki bot will have access to commit to this repository? |
|
@Abijeet Thank you! |
mgautierfr
pushed a commit
that referenced
this pull request
Apr 26, 2022
The story of search_results.css static/skin/search_results.css was extracted from static/templates/no_search_result.html before the latter was dropped. static/templates/no_search_result.html in turn seems to be a copied and edited version of static/templates/search_result.html. In the context of exploratory work on the internationalization of kiwix-serve (PR #679) I noticed duplication of inline CSS across those two templates and intended to eliminated it. That goal was not fully accomplished (static/templates/search_result.html remained untouched) because by that time PR #679 grew too big and the efforts were diverted into splitting it into smaller ones. Thus search_results.css slipped into one of those small PRs, without making much sense because nothing really justifies preserving custom CSS in the "Fulltext search unavailable" error page. At the same time, it served as the only case where a link to a cacheable resource is generated in C++ code (rather than found in a template). This poses certain problems to the handling of cache-ids. A workaround is to expel the URL into a template so that it is processed by `kiwix-resources`. This commit merely demonstrates that solution. But whether it should be preserved (or rather the "Fulltext search unavailable" page should be deprived of CSS) is questionable.
veloman-yunkan
added a commit
that referenced
this pull request
May 2, 2022
The story of search_results.css static/skin/search_results.css was extracted from static/templates/no_search_result.html before the latter was dropped. static/templates/no_search_result.html in turn seems to be a copied and edited version of static/templates/search_result.html. In the context of exploratory work on the internationalization of kiwix-serve (PR #679) I noticed duplication of inline CSS across those two templates and intended to eliminated it. That goal was not fully accomplished (static/templates/search_result.html remained untouched) because by that time PR #679 grew too big and the efforts were diverted into splitting it into smaller ones. Thus search_results.css slipped into one of those small PRs, without making much sense because nothing really justifies preserving custom CSS in the "Fulltext search unavailable" error page. At the same time, it served as the only case where a link to a cacheable resource is generated in C++ code (rather than found in a template). This poses certain problems to the handling of cache-ids. A workaround is to expel the URL into a template so that it is processed by `kiwix-resources`. This commit merely demonstrates that solution. But whether it should be preserved (or rather the "Fulltext search unavailable" page should be deprived of CSS) is questionable.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses the C++ part of kiwix/kiwix-tools#59
This PR introduces facilities for supporting internationalized textual output from the kiwix-serve backend (C++ code).
static/i18n/(one file per language). Those JSON files are compiled into C++ code by a new scriptkiwix-compile-i18n.kiwix::ParameterizedMessageclass.ParameterizedMessage(msgId, [msgParams])creates a message object with the specified id and (optional) parameters. The text of the message for the desired language must be obtained via theParameterizedMessage::getText(const std::string& language)method.userlangquery parameter. Adding support for user language selection via cookies and/or the Accept-Language header should be easy.