From 92a60f8bce2b79087e4f7c3579167a0fcc33a0f4 Mon Sep 17 00:00:00 2001 From: Oliver Eyton-Williams Date: Mon, 28 Sep 2020 15:13:18 +0200 Subject: [PATCH] fix(tools): update translation parser Since we're adding more validation we can simplify the parser and make sure it does catch all the comments. Rather than worry about a load of edge cases that do not appear in our challenges. --- curriculum/getChallenges.js | 12 ++--- .../translation-parser/translation-parser.js | 51 ++++++++++--------- .../translation-parser.test.js | 39 ++++++++++---- 3 files changed, 63 insertions(+), 39 deletions(-) diff --git a/curriculum/getChallenges.js b/curriculum/getChallenges.js index e9b0c4f3ea..9e506c4e1a 100644 --- a/curriculum/getChallenges.js +++ b/curriculum/getChallenges.js @@ -201,12 +201,12 @@ async function parseTranslation(engPath, transPath, dict, lang) { const engChal = await parseMarkdown(engPath); const translatedChal = await parseMarkdown(transPath); - const engWithTranslatedComments = translateCommentsInChallenge( - engChal, - lang, - dict - ); - + // challengeType 11 is for video challenges, which have no seeds, so we skip + // them. + const engWithTranslatedComments = + engChal.challengeType !== 11 + ? translateCommentsInChallenge(engChal, lang, dict) + : engChal; return mergeChallenges(engWithTranslatedComments, translatedChal); } diff --git a/tools/challenge-md-parser/translation-parser/translation-parser.js b/tools/challenge-md-parser/translation-parser/translation-parser.js index 82bee5d5a8..c5dd7e525f 100644 --- a/tools/challenge-md-parser/translation-parser/translation-parser.js +++ b/tools/challenge-md-parser/translation-parser/translation-parser.js @@ -6,11 +6,10 @@ exports.translateComments = (text, lang, dict, codeLang) => { const config = { knownComments, dict, lang }; switch (codeLang) { case 'js': - return transMultiline(transInline(text, config), config); case 'jsx': - return transJSX(text, config); + return transMultiline(transInline(text, config), config); case 'html': - return transHTML(transCSS(text, config), config); + return transScript(transHTML(transCSS(text, config), config), config); default: return text; } @@ -18,18 +17,20 @@ exports.translateComments = (text, lang, dict, codeLang) => { exports.translateCommentsInChallenge = (challenge, lang, dict) => { const challClone = clone(challenge); - - Object.keys(challClone.files).forEach(key => { - if (challClone.files[key].contents) { - challClone.files[key].contents = this.translateComments( - challenge.files[key].contents, - lang, - dict, - challClone.files[key].ext - ); - } - }); - + if (!challClone.files) { + console.warn(`Challenge ${challClone.title} has no comments to translate`); + } else { + Object.keys(challClone.files).forEach(key => { + if (challClone.files[key].contents) { + challClone.files[key].contents = this.translateComments( + challenge.files[key].contents, + lang, + dict, + challClone.files[key].ext + ); + } + }); + } return challClone; }; @@ -93,12 +94,7 @@ exports.mergeChallenges = (engChal, transChal) => { // bare urls could be interpreted as comments, so we have to lookbehind for // http:// or https:// function transInline(text, config) { - return translateGeneric( - text, - config, - '(^[^\'"`]*?(?.*?<\/script>/gms; + const matches = text.matchAll(regex); + + for (const [match] of matches) { + text = text.replace( + match, + transMultiline(transInline(match, config), config) + ); + } + return text; } function transHTML(text, config) { diff --git a/tools/challenge-md-parser/translation-parser/translation-parser.test.js b/tools/challenge-md-parser/translation-parser/translation-parser.test.js index 46866db76d..c642c90bea 100644 --- a/tools/challenge-md-parser/translation-parser/translation-parser.test.js +++ b/tools/challenge-md-parser/translation-parser/translation-parser.test.js @@ -205,15 +205,6 @@ describe('translation parser', () => { ); }); - // If a comment follows '"` it could be inside a string, so we should - // not try and translate it - erring on the side of caution. - it('should ignore comments if they follow an open quote', () => { - const seed = `var str = '// Add your code below this line' - var str2 = '// Add your code above this line`; - expect(translateComments(seed, 'chinese', SIMPLE_TRANSLATION, 'js')).toBe( - seed - ); - }); it('replaces multiple English comments with their translations', () => { const seed = `inline comment // Add your code below this line // Add your code below this line `; @@ -330,6 +321,32 @@ describe('translation parser', () => { ).toBe(transSeed); }); + it('replaces English script comments with their translations', () => { + const seed = ``; + const transSeed = ``; + expect( + translateComments(seed, 'chinese', SIMPLE_TRANSLATION, 'html') + ).toBe(transSeed); + }); + + it('replaces multiple script comments with their translations', () => { + const seed = ``; + const transSeed = ``; + expect( + translateComments(seed, 'chinese', SIMPLE_TRANSLATION, 'html') + ).toBe(transSeed); + }); + it('ignores html comments inside JavaScript', () => { const seed = `
change code below this line `; @@ -391,6 +408,7 @@ describe('translation parser', () => { /* this is not a comment */ `; const seedHTML = `
`; + const seedScript = ``; translateComments(seedJSX, 'chinese', SIMPLE_TRANSLATION, 'jsx'); expect(logSpy).toBeCalledTimes(1); @@ -402,7 +420,8 @@ describe('translation parser', () => { expect(logSpy).toBeCalledTimes(4); translateComments(seedHTML, 'chinese', SIMPLE_TRANSLATION, 'html'); expect(logSpy).toBeCalledTimes(5); - logSpy.mockRestore(); + translateComments(seedScript, 'chinese', SIMPLE_TRANSLATION, 'html'); + expect(logSpy).toBeCalledTimes(6); }); }); });