-
Notifications
You must be signed in to change notification settings - Fork 9
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
Fix for string patterns that were erroneously translated. #18
Comments
Oops sorry, I'll repeat phetsims/rosetta#329 over here: Hello! I spent 15 minutes and wrote a script that should show every translation in which the current value of a translated template string key does not match the english version in master. I think it will at least be a good starting point, but it is worth noting that there may be a disconnect between published sims and master. Please also note that this does not look at any values of the babel translation "history", just the current value. the scriptconst fs = require( 'fs' );
const _ = require( 'lodash' );
const x = fs.readdirSync( '../babel' );
const templateVarRegex = /{\{?[\w\d]+\}\}?/g;
const openRegex = /{/g;
const closeRegex = /}/g;
function getTemplatedVars( string ) {
const matches = string.match( templateVarRegex ) || [];
// Don't match {HI}} for whatever reason that may be
return matches.filter( templateVar => templateVar.match( openRegex ).length === templateVar.match( closeRegex ).length );
}
// @returns map where string key is key, and list of template vars in it are the value.
function getTemplateVarsInEnglishStringFile( repo ) {
const englishStringFileName = `../${repo}/${repo}-strings_en.json`;
const englishStrings = JSON.parse( fs.readFileSync( englishStringFileName ).toString() );
// Record<stringKey, Array<templateVarString>>
const keyTemplateMap = {};
Object.keys( englishStrings ).forEach( key => {
const stringValue = englishStrings[ key ].value;
if ( stringValue ) {
const templateVars = getTemplatedVars( stringValue );
if ( templateVars.length > 0 ) {
keyTemplateMap[ key ] = templateVars;
}
}
} );
return keyTemplateMap;
}
x.forEach( dir => {
const babelRepoDir = `../babel/${dir}`;
if ( !dir.startsWith( '_' ) && !dir.startsWith( '.' ) && fs.statSync( babelRepoDir ).isDirectory() ) {
const repo = dir;
const templateVarsMap = getTemplateVarsInEnglishStringFile( repo );
fs.readdirSync( babelRepoDir ).forEach( translatedStringFileContents => {
const translatedStrings = JSON.parse( fs.readFileSync( `../babel/${dir}/${translatedStringFileContents}` ).toString() );
Object.keys( templateVarsMap ).forEach( stringKey => {
if ( translatedStrings[ stringKey ] ) {
const translatedStringValue = translatedStrings[ stringKey ].value;
const translatedTemplateVars = getTemplatedVars( translatedStringValue );
const englishTemplatedVars = templateVarsMap[ stringKey ];
// Set because order doesn't matter
if ( !_.isEqual( new Set( englishTemplatedVars ), new Set( translatedTemplateVars ) ) ) {
console.log( `${translatedStringFileContents}:\n`, 'english vars:', englishTemplatedVars, '\n translated vars:', translatedTemplateVars, '\n' );
}
}
} );
} );
}
} ); Resultsbeers-law-lab-strings_ru.json@pattern.0label: beers-law-lab-strings_ru.json@pattern.0value.1units: beers-law-lab-strings_ru.json@pattern.0percent: beers-law-lab-strings_ru.json@pattern.0formula.1name: charges-and-fields-strings_gu.json@pattern.0value.1units: circuit-construction-kit-common-strings_nb.json@resistanceOhmsSymbol: circuit-construction-kit-common-strings_ta.json@resistanceOhmsValuePattern: expression-exchange-strings_ko.json@numberCentsPattern: expression-exchange-strings_ko.json@levelNumberPattern: expression-exchange-strings_zh_CN.json@numberCentsPattern: expression-exchange-strings_zh_CN.json@levelNumberPattern: fluid-pressure-and-flow-strings_gu.json@readoutFeet: fluid-pressure-and-flow-strings_gu.json@massLabelPattern: fluid-pressure-and-flow-strings_gu.json@readoutMeters: fluid-pressure-and-flow-strings_gu.json@valueWithUnitsPattern: gene-expression-essentials-strings_es_ES.json@gene: gene-expression-essentials-strings_fa.json@gene: gene-expression-essentials-strings_ja.json@gene: gene-expression-essentials-strings_mk.json@gene: gene-expression-essentials-strings_ta.json@gene: joist-strings_nl.json@credits.qualityAssurance: masses-and-springs-strings_ig.json@dampingEqualsZero: masses-and-springs-strings_ig.json@gravityValue: masses-and-springs-strings_ig.json@massValue: molarity-strings_fa.json@pattern.parentheses.0text: molecule-polarity-strings_bs.json@pattern.dipoleDirection: molecule-polarity-strings_da.json@pattern.dipoleDirection: molecule-polarity-strings_fa.json@pattern.atomName: molecule-polarity-strings_ko.json@pattern.atomName: molecule-polarity-strings_ko.json@pattern.symbolName: molecule-polarity-strings_ko.json@pattern.dipoleDirection: molecule-polarity-strings_vi.json@pattern.dipoleDirection: molecule-polarity-strings_zh_CN.json@pattern.atomName: molecule-polarity-strings_zh_CN.json@pattern.symbolName: molecule-polarity-strings_zh_CN.json@pattern.dipoleDirection: number-play-strings_ht.json@wordLanguage: pendulum-lab-strings_nb.json@gravitationalAccelerationPattern: pendulum-lab-strings_nb.json@degreesPattern: pendulum-lab-strings_nb.json@secondsPattern: proportion-playground-strings_eu.json@pricePattern: proportion-playground-strings_ko.json@pricePattern: proportion-playground-strings_vi.json@pricePattern: proportion-playground-strings_zh_CN.json@pricePattern: scenery-phet-strings_ht.json@keyboardHelpDialog.grabOrReleaseHeadingPattern: scenery-phet-strings_ht.json@keyboardHelpDialog.grabOrReleaseLabelPattern: scenery-phet-strings_ht.json@measuringTapeReadoutPattern: scenery-phet-strings_tg.json@measuringTapeReadoutPattern: trig-tour-strings_iw.json@valueUnitPattern: trig-tour-strings_iw.json@numberPiPattern: vegas-strings_ht.json@pattern.0yourBest: vegas-strings_mt.json@label.level: |
I think it would be good to think of figuring out if/how all of these should be changed, as opposed to just handling for molecule polarity. Perhaps a more common issue? It wasn't clear to me if we should do this via rosetta interface, (which will likely help by triggering a rebuilt of the production version), or something else that is faster and more behind the scenes. Over to you @kathy-phet to see the scope of the issue (beyond molecule-polarity) and to recommend who should lead the assault! |
Transferring this issue to babel. It's not specific to molecule-polarity, it's a problem in many sims, as demonstrated by the "Results" in #18. |
@kathy-phet can you please prioritize this? |
@oliver-phet - If you can take care of Molecule Polarity by next Tuesday that would be great, and since I don't think it will take a lot of time just finish the list off before you leave for the holiday, and close this issue that would be ideal. Since we know Rosetta won't let this happen again, we can just fix these and be done. Thanks! |
@kathy-phet @oliver-phet and I spoke again today. We are most worried about phetsims/rosetta#329 and if translators can continue to make this mistake, but when it comes time to fix things, here is a more accurate script that compared the release branch en strings to babel, instead of using master. I also fixed a bug where non ascii chars were not being printed in the results: The script (run from `./perennial/script.js`)const fs = require( 'fs' );
const _ = require( 'lodash' );
const axios = require( 'axios' );
const gitCheckout = require( '../perennial/js/common/gitCheckout.js' );
let output = '';
// template vars with anything inside but template markers(i.e. supporting '{{名字}}')
const templateVarRegex = /{\{?[^{}]+\}\}?/g;
const openRegex = /{/g;
const closeRegex = /}/g;
function getTemplatedVars( string ) {
const matches = string.match( templateVarRegex ) || [];
// Don't match {HI}} for whatever reason that may be
return matches.filter( templateVar => templateVar.match( openRegex ).length === templateVar.match( closeRegex ).length );
}
let metadata = null;
const metadataURL = 'https://phet.colorado.edu/services/metadata/1.3/simulations?format=json&type=html&summary';
async function getMetadata() {
return ( await axios.get( metadataURL ) ).data.projects;
}
async function getLatestReleaseBranch( repo ) {
if ( !metadata ) {
metadata = await getMetadata();
}
const simProject = _.find( metadata, entry => entry.name === `html/${repo}` );
if ( !simProject ) {
return 'master';
}
const simProjectVersion = simProject.version;
return `${simProjectVersion.major}.${simProjectVersion.minor}`;
}
// @returns map where string key is key, and list of template vars in it are the value.
async function getTemplateVarsInEnglishStringFile( repo ) {
console.log( repo );
await gitCheckout( repo, await getLatestReleaseBranch( repo ) );
const englishStringFileName = `../${repo}/${repo}-strings_en.json`;
const englishStrings = JSON.parse( fs.readFileSync( englishStringFileName ).toString() );
await gitCheckout( repo, 'master' );
// Record<stringKey, Array<templateVarString>>
const keyTemplateMap = {};
Object.keys( englishStrings ).forEach( key => {
const stringValue = englishStrings[ key ].value;
if ( stringValue ) {
const templateVars = getTemplatedVars( stringValue );
if ( templateVars.length > 0 ) {
keyTemplateMap[ key ] = templateVars;
}
}
} );
return keyTemplateMap;
}
const babelRepoDir = '../babel';
async function checkForRepos( repos ) {
for ( const repo of repos ) {
const templateVarsMap = await getTemplateVarsInEnglishStringFile( repo );
fs.readdirSync( `${babelRepoDir}/${repo}` ).forEach( translatedStringFileContents => {
const translatedStrings = JSON.parse( fs.readFileSync( `../babel/${repo}/${translatedStringFileContents}` ).toString() );
Object.keys( templateVarsMap ).forEach( stringKey => {
if ( translatedStrings[ stringKey ] ) {
const translatedStringValue = translatedStrings[ stringKey ].value;
const translatedTemplateVars = getTemplatedVars( translatedStringValue );
const englishTemplatedVars = templateVarsMap[ stringKey ];
// Set because order doesn't matter
if ( !_.isEqual( new Set( englishTemplatedVars ), new Set( translatedTemplateVars ) ) ) {
output += `${translatedStringFileContents}:\n` + ' english vars: ' + JSON.stringify( englishTemplatedVars ) + '\n translated vars: ' +
JSON.stringify( translatedTemplateVars ) + '\n';
}
}
} );
} );
}
}
( async () => {
const babelRepos = fs.readdirSync( babelRepoDir ).filter( dir => {
return !dir.startsWith( '_' ) && !dir.startsWith( '.' ) && fs.statSync( `${babelRepoDir}/${dir}` ).isDirectory();
} );
await checkForRepos( babelRepos );
fs.writeFileSync( 'output.txt', output );
console.log( 'written to output.txt' );
} )(); Results
|
QA ran into this for the scenery-phet |
I don't have any updates on this. I'm still on hold to do any manual fixes until either
I can do the manual fixes whenever, but it seems that until the pattern validation is fixed this issue can continue to occur. |
@oliver-phet - I discussed with JB before break. I think you should do the manual fixes now. There will not likely be very many additional breaks, since we have not figured out how to even reproduce it. |
@oliver-phet, I see 8fb649d came in. Could you please give an update for this issue? Perhaps it was all covered over in phetsims/rosetta#329 (now closed?) |
Sorry, I forgot there were 2 issues about this. All the pattern fixes (identified by @jbphet 's script output) were made in phetsims/rosetta#329 If there's something else for me on this issue, let me know! |
Awesome! I confirmed my specific bug is fixed on master with phetsims/friction#315, and will be picked up in the next rc/production deploy. |
In issue phetsims/rosetta#329, we note that some translations have "translated" the strings that are within a code pattern. These strings need to remain as original strings, for the code to correctly work. They are basically variable names, not actual text that needs to be translated.
We need to find the translations where the code pattern was mistakenly translated, and fix it. We will do this by hand for the Molecule Polarity strings. Over in phetsims/rosetta#329, we are investigating if babel allows this error to happen now or whether it has a fix already in place.
@oliver-phet - Can you please go through the existing translations, and inspect this row
And if it reads anything differently from the English, replace it with the English version.
{{from}} → {{to}}
And do so without changing the credits?
@zepumph - I think you mentioned you might be able to make a quick script to see which translations needed this hand fix?
Here is a short list CM started:
The text was updated successfully, but these errors were encountered: