Make --doctool locale aware
#55930
Make --doctool locale aware
#55930
Conversation
timothyqiu
force-pushed
the
doctool-i18n
branch
2 times, most recently
from
f871f85
to
21e11da
Compare
|
Updated the PR, and now it only translates the XMLs when It seems that the clearing of static |
core/string/ustring.cpp
Outdated
| return indent; | ||
| } | ||
|
|
||
| String String::indent(const String &p_indent) const { |
There was a problem hiding this comment.
For the record we seem to have
core/io/json.h
72: static String _make_indent(const String &p_indent, int p_size);
which could maybe be modified to use this instead if we're adding it to String.
And there's possibly more script editor code that might also reimplement indentation logic.
There was a problem hiding this comment.
This one is just making the correct level of indent string instead of indenting. Can be probably replaced by p_indent.repeat(p_size) :)
|
Looks good overall and seems to work well 👍 I'd split the XML fixes to a dedicated PR so that they can be merged now while we keep iterating on the core and translation API changes. |
timothyqiu
force-pushed
the
doctool-i18n
branch
from
21e11da
to
e220049
Compare
There was a problem hiding this comment.
Looks good to me for the editor/translation/main changes.
For the added String API, could use some additional review to make sure that it's a useful API to have here (and which should then likely be exposed to scripting). Otherwise it could be kept local to where it's needed since it's only needed by DocTools so far.
timothyqiu
force-pushed
the
doctool-i18n
branch
from
e220049
to
48d8be4
Compare
|
Rebased so CI won't complain about changed XMLs. Also made |
| static String _translate_doc_string(const String &p_text) { | ||
| const String indent = _get_indent(p_text); | ||
| const String message = p_text.dedent().strip_edges(); | ||
| const String translated = TranslationServer::get_singleton()->doc_translate(message, ""); | ||
| // No need to restore stripped edges because they'll be stripped again later. | ||
| return translated.indent(indent); | ||
| } |
There was a problem hiding this comment.
In general I feel like we could be doing a better job at handling the XML tab indentation of these strings. The only part which we need to preserve is the manual 4-space indentation in code blocks, but everything else should be something that we could simply ignore, and add manually in _write_string (as already done at least one the first line).
Currently we dedent, strip edges, indent again, strip edges again (so it removes indentation on the first line again?), and then escape. A lot of steps which could be done better IMO.
But that's not blocking for this PR.
timothyqiu
force-pushed
the
doctool-i18n
branch
from
48d8be4
to
d8c66ba
Compare
This should be good to backport now 👍 |
* Adds `indent(str)` to `String`:
* Indent the (multiline) string with the given indentation.
* This method is added in order to keep the translated XML correctly
indented.
* Moves the loading of tool/doc translation into
`editor/editor_translation.{h,cpp}`.
* This will be used from both `EditorSettings` and the doc tool from
`main`.
* Makes use of doc translation when generating XML class references, and
setup the translation locale based on `-l LOCALE` CLI parameter.
The XML class reference won't be translated if `-l LOCALE` parameter is
not given, or when it's `-l en`.
timothyqiu
force-pushed
the
doctool-i18n
branch
from
d8c66ba
to
e4e4e47
Compare
|
Thanks! |
This makes i18n of the online class reference easier. One can then generate RST based on the localized XML (although the generator script needs further tweaks in order to work with non-latin content).
indent(str)toString:editor/editor_translation.{h,cpp}.EditorSettingsand the doc tool frommain.-l LOCALECLI parameter.The XML class reference won't be translated if
-l LOCALEis not given, or when it's-l en. So it's backward compatible I think.I'll backport this PR to
3.xif it is OK.