-
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
Open
u8621011
wants to merge
5
commits into
Slicer:master
Choose a base branch
from
u8621011:dtc18n2
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
1e2c14a
ENH: i18n: SampleData: Translate file menu action
u8621011 d9e9bc9
ENH: i18n: Associate CLI Module with translation context
u8621011 303c564
ENH: i18n: Add some QTGUI i18n fixes
u8621011 a6545f5
STYLE: i18n: Remove incomplete french translation files (*_fr.ts)
jcfr 9470766
ENH: i18n: Add chinese translation files (*_zh_cn.ts, *_zh_tw.ts)
u8621011 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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.
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:
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.
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 tolupdate
.Here are two possible approaches:
BRAINSFit
)BRAINSFit.Input Images ...
... 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:
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.