-
Notifications
You must be signed in to change notification settings - Fork 458
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
Plenty chinese i18n ts files and code modification #898
base: master
Are you sure you want to change the base?
Conversation
I haven't reviewed in detail, but in principle this looks great! I'd be mostly concerned about unintended side effects, so maybe we could find a way to generate to packages from this branch that people can test thoroughly. |
I rebased and re-organize the changes. I will add comment directly on each commit |
A lot of the changes have been reviewed, fixed and integrated. Including support for Qt5. Remaining changes to review are the one in this PR |
// do the internationalization setup for ButtonBox buttons | ||
this->ButtonBox->button(QDialogButtonBox::Ok)->setText(tr("OK")); | ||
this->ButtonBox->button(QDialogButtonBox::Cancel)->setText(tr("Cancel")); | ||
this->ButtonBox->button(QDialogButtonBox::Reset)->setText(tr("Reset")); |
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.
@u8621011 This will work for these two occurrences of dialog (qSlicerDataDialog
and qSlicerSaveDataDialog
) but I think we should look for a more general approaches.
807a686
to
b2145ea
Compare
Co-authored-by: Jean-Christophe Fillion-Robin <[email protected]>
This commit translates: * SlicerApp * QTApp, QTCLI, QTCore, QTGUI and Modules/Core * MRML libraries: qMRMLWidgets * Modules: Data, Markups, Models, Segmentation, SlicerWelcome, SubjectHierarchy, Transforms, VolumeRendering and Volumes Co-authored-by: Jean-Christophe Fillion-Robin <[email protected]>
@@ -667,6 +668,9 @@ QWidget* qSlicerCLIModuleUIHelperPrivate::createImageTagWidget(const ModuleParam | |||
QString imageLabel = QString::fromStdString(moduleParameter.GetLabel()); | |||
QString imageName = QString::fromStdString(moduleParameter.GetName()); | |||
|
|||
imageLabel = QCoreApplication::translate( | |||
this->CLIModuleWidget->moduleName().toLatin1(), imageLabel.toLatin1()); |
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.
@u8621011 It would be nice to add translation for all the other tags.
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.
OK, I will add translation for all the other tags later.
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.
It would be also better not to use the CLI module name as translation context but indicate that it is a CLI module translation, by using a prefix, such as "CLI.". Something like this:
CLINodeTranslationContextPrefix = "CLI."
...
imageLabel = QCoreApplication::translate((CLINodeTranslationContextPrefix+this->CLIModuleWidget->moduleName()).toLatin1(), imageLabel.toLatin1());
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.
@lassoan why better not to use the CLI module name as translation context?
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 the module name is not specific enough. We need to know that a particular context belongs to a CLI module.
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.
@jcfr the CLI Module we translate likes steps below.
- modify our version xml file, partial BRAINSFit.xml for example
<parameters advanced="false">
<label>Output Settings (At least one output must be specified)</label>
<transform fileExtensions=".h5,.hdf5,.mat,.txt" type="linear" reference="movingVolume">
<name>linearTransform</name>
<longflag>linearTransform</longflag>
<label>QT_TRANSLATE_NOOP("BRAINSFit","Slicer Linear Transform")</label>
<description>(optional) Output estimated transform - in case the computed transform is not BSpline. NOTE: You must set at least one output object (transform and/or output volume).</description>
<channel>output</channel>
</transform>
<transform fileExtensions=".h5,.hdf5,.mat,.txt" type="bspline" reference="movingVolume">
<name>bsplineTransform</name>
<longflag>bsplineTransform</longflag>
<label>QT_TRANSLATE_NOOP("BRAINSFit","Slicer BSpline Transform")</label>
<description>(optional) Output estimated transform - in case the computed transform is BSpline. NOTE: You must set at least one output object (transform and/or output volume).</description>
<channel>output</channel>
</transform>
<image>
<name>outputVolume</name>
<longflag>outputVolume</longflag>
<label>QT_TRANSLATE_NOOP("BRAINSFit","Output Image Volume")</label>
<description>(optional) Output image: the moving image warped to the fixed image space. NOTE: You must set at least one output object (transform and/or output volume).</description>
<channel>output</channel>
</image>
</parameters>
- run qt lupdate over our xml
C:\D\Support\qt-4.8.7-64-vs2013-deb\bin\lupdate BRAINSFit.xml -ts BRAINSFit_zh_tw.ts
- translated the generated ts file. below are how lupdate generated for us.
<?xml version="1.0" encoding="utf-8"?>
<!DOCTYPE TS>
<TS version="2.0" language="zh_TW">
<context>
<name>BRAINSFit</name>
<message>
<location filename="BRAINSFit.xml" line="52"/>
<source>Slicer Linear Transform</source>
<translation type="unfinished"></translation>
</message>
<message>
<location filename="BRAINSFit.xml" line="59"/>
<source>Slicer BSpline Transform</source>
<translation type="unfinished"></translation>
</message>
<message>
<location filename="BRAINSFit.xml" line="66"/>
<source>Output Image Volume</source>
<translation type="unfinished"></translation>
</message>
</context>
</TS>
- copy translated cli ts context to some cmake generated ts file by hand. (qSlicerBaseQTApp_zh_tw.ts for example)
the step 2 & 4 should possibly be done in cmake but we don't know how to script it.
Any ideas?
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.
Thanks, now you provided the step it will be easy to automate the generation.
For step 2, we could ensure that an intermediate file named BRAINSFit.xml.tr
is generated based on the original xml file. This file could then be given as input to lupdate
.
Here are two possible approaches:
- (1) all strings in the same context (e.g
BRAINSFit
) - (2) label/description associated with their corresponding parameter group context (e.g
BRAINSFit.Input Images ...
import os
import xml.etree.ElementTree as ET
def tr(context, value):
return 'QT_TRANSLATE_NOOP("%s", "%s")' % (context, value)
def generate_lupdate_input_v1(xmlFile, candidates=["label"]):
"""Process CLI XML description file and generate an new XML file
including `QT_TRANSLATE_NOOP()` for each `candidates` xml tags.
"""
moduleName = os.path.splitext(xmlFile)[0]
tree = ET.parse(xmlFile)
root = tree.getroot()
for candidate in candidates:
for label in root.iter(candidate):
label.text = tr(moduleName, label.text)
tree.write(xmlFile + ".v1.tr")
def generate_lupdate_input_v2(xmlFile, candidates=["label"]):
"""Process CLI XML description file and generate an new XML file
including `QT_TRANSLATE_NOOP()` for each `candidates` xml tags.
"""
moduleName = os.path.splitext(xmlFile)[0]
tree = ET.parse(xmlFile)
root = tree.getroot()
for parameters in root.findall("./parameters"):
context = moduleName + "." + parameters.find("label").text
labels = parameters.findall(".//label")
for label in labels:
label.text = tr(context, label.text)
tree.write(xmlFile + ".v2.tr")
generate_lupdate_input_v1("BRAINSFit.xml")
generate_lupdate_input_v2("BRAINSFit.xml")
... and corresponding screenshot for the linguist
application:
Without parameter group context
With parameter group context
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.
I think approach (2) with context will make translation work easier. What do you think ?
After we agree on an approach, it will be easy to integrate it with the CLI build system.
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.
I feel if the CLI cmake script can parse and compile ts into qm for every cli module directory, approach should be ok.(eg, each CLI module has its own ts/qm file)
Approach 2 looks too long the name but if we have to put every cli module sentences into same/single ts file, approach 2 is necessary.
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.
Need to be careful with enumerated strings. While the values should be translated in the GUI, the original text should be sent on the command-line.
For example:
User will see "outside", "below", "above" translated on GUI, but when the selection is made, the CLI module must receive the original "outside", "below", "above" values.
This goes the other direction, too. Returning strings is rare, but eventually, any strings returned by the CLI should be possible to translate.
Few runtime issues:
Few more things we need to address:
Otherwise, I was able to start Slicer built with Qt5: |
Below is what this PR includes