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

Plenty chinese i18n ts files and code modification #898

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

u8621011
Copy link

Below is what this PR includes

  1. traditional/simplified Chinese translation files
  2. i18n related cmake script for loadable module
  3. i18n related code fixes.

@pieper
Copy link
Member

pieper commented Feb 23, 2018

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.

@jcfr
Copy link
Member

jcfr commented Feb 23, 2018

I rebased and re-organize the changes.

I will add comment directly on each commit

@jcfr
Copy link
Member

jcfr commented Feb 23, 2018

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"));
Copy link
Member

@jcfr jcfr Feb 23, 2018

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.

  • Is there .ts files already available for Qt base classes ?

  • also wonder if using QT_TRANSLATE_NOOP as hinted here and here could be more general

@jcfr jcfr force-pushed the dtc18n2 branch 2 times, most recently from 807a686 to b2145ea Compare February 24, 2018 00:20
u8621011 and others added 4 commits February 23, 2018 19:25
Co-authored-by: Jean-Christophe Fillion-Robin <jchris.fillionr@kitware.com>
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 <jchris.fillionr@kitware.com>
@@ -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());
Copy link
Member

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.

Copy link
Author

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.

Copy link
Contributor

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());

Copy link
Author

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?

Copy link
Contributor

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.

Copy link
Author

@u8621011 u8621011 Mar 1, 2018

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.

  1. 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>
  1. 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
  1. 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>

  1. 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?

Copy link
Member

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

linguist_v1

With parameter group context

linguist_v2

Copy link
Member

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.

Copy link
Author

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.

Copy link
Contributor

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:

https://github.com/Slicer/Slicer/blob/aa07f6b274778f4cbab626b35228bea8768d38ce/Modules/CLI/ThresholdScalarVolume/ThresholdScalarVolume.xml#L31-L40

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.

@jcfr
Copy link
Member

jcfr commented Feb 24, 2018

Few runtime issues:

  • failed to initialize the dataprobe module: No Data Probe frame - cannot create DataProbe
  • And few file that failed to be loaded in the translator:
The File  "qSlicerBaseQTCore_zh_cn.qm"  hasn't been loaded in the translator
The File  "qSlicerBaseQTApp_zh_cn.qm"  hasn't been loaded in the translator
The File  "SubjectHierarchy_zh_cn.qm"  hasn't been loaded in the translator
The File  "VolumeRendering_zh_tw.qm"  hasn't been loaded in the translator

Few more things we need to address:

  • CLI module translation:
  • distribution of *.qm files within Slicer extension
  • translation of text from vtk and mrml objects. I suggest to investigate the use of QT_TRANSLATE_NOOP

Otherwise, I was able to start Slicer built with Qt5:

slicer-i18n

@jcfr jcfr added the wip label Feb 24, 2018
@jcfr jcfr changed the title Plenty chinese i18n ts files and code modification Plenty chinese i18n ts files and code modification [WIP] Feb 24, 2018
@jcfr jcfr changed the title Plenty chinese i18n ts files and code modification [WIP] Plenty chinese i18n ts files and code modification Feb 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 participants