Skip to content
This repository has been archived by the owner on May 11, 2021. It is now read-only.

Commit

Permalink
Fix $_ in local i18n.js so that Perseus can use it when running locally
Browse files Browse the repository at this point in the history
Summary:
/local-only/i18n.js defined window.$_ using an unspecified reference to
createFragment, which didn't cause an error when testing locally because
khan-exercises doesn't include any React code. However, the same local
javscript file is used when running Perseus locally - and Perseus uses
$_ all over the place.

Test Plan:
I ran ./local-only/update-local.sh as part of this commit. Since the
newest i18n.js no longer exports $._ I grepped to make sure it was no
longer used in any exercise/util:
grep -E '(jQuery|\$)\._\(|(jQuery|\$)\.ngettext|(jQuery|\$)\.i18nDoNotTranslate' -r .

make lint
python build/lint_i18n_strings_test.py

make pack
Checked that both [unpacked](http://localhost:8000/exercises/decimals_on_the_number_line_1.html) and [packed](http://localhost:8000/exercises-packed/decimals_on_the_number_line_1.html?lang=es) exercises work

Updated the khan-exercises submodule in the Perseus repo and verified
that [widgets using $_](http://localhost:9000/test.html#content=%7B%22question%22%3A%7B%22content%22%3A%22%5B%5B%E2%98%83%20radio%201%5D%5D%22%2C%22images%22%3A%7B%7D%2C%22widgets%22%3A%7B%22radio%201%22%3A%7B%22type%22%3A%22radio%22%2C%22alignment%22%3A%22default%22%2C%22static%22%3Afalse%2C%22graded%22%3Atrue%2C%22options%22%3A%7B%22choices%22%3A%5B%7B%7D%2C%7B%7D%5D%2C%22randomize%22%3Afalse%2C%22multipleSelect%22%3Afalse%2C%22displayCount%22%3Anull%2C%22hasNoneOfTheAbove%22%3Afalse%2C%22onePerLine%22%3Atrue%2C%22deselectEnabled%22%3Afalse%7D%2C%22version%22%3A%7B%22major%22%3A1%2C%22minor%22%3A0%7D%7D%7D%7D%2C%22answerArea%22%3A%7B%22type%22%3A%22multiple%22%2C%22options%22%3A%7B%22content%22%3A%22%22%2C%22images%22%3A%7B%7D%2C%22widgets%22%3A%7B%7D%7D%2C%22calculator%22%3Afalse%2C%22periodicTable%22%3Afalse%7D%2C%22itemDataVersion%22%3A%7B%22major%22%3A0%2C%22minor%22%3A1%7D%2C%22hints%22%3A%5B%5D%7D) render properly.

Reviewers: csilvers

Reviewed By: csilvers

Differential Revision: https://phabricator.khanacademy.org/D22830
  • Loading branch information
Alex Lopatin committed Oct 29, 2015
1 parent 3ca2d5e commit 95e2c88
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 11 deletions.
6 changes: 3 additions & 3 deletions build/lint_i18n_strings.py
Original file line number Diff line number Diff line change
Expand Up @@ -1490,12 +1490,12 @@ def filter_var(self, match, var_node):

class StringInVarFilter(BaseFilter):
"""Detect instances of i18n._ inside of <var>s and report an error."""
# Matches i18n._(...) and (the obsolete) $._(...)
_regex = re.compile(r'(\$|i18n)\._\s*\((.*?)\)', re.DOTALL)
# Matches i18n._(...)
_regex = re.compile(r'(i18n)\._\s*\((.*?)\)', re.DOTALL)

def find_fixable_vars(self, node):
"""Return the node if it has i18n._ in it"""
if 'i18n._' in node.text or '$._' in node.text:
if 'i18n._' in node.text:
return [node]
else:
return []
Expand Down
4 changes: 2 additions & 2 deletions exercises/decimals_on_the_number_line_1.html
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,12 @@

<p data-if="isSingular(TENTHS)">
Starting at <code><var>LOWER_BOUND</var></code>, we move a length of
<code>\blue{<var>commafy(MARK_INCREMENT)</var> \text{ <var>cardinalThrough20(TENTHS)</var> time}}</code>
<code>\blue{<var>commafy(MARK_INCREMENT)</var>}</code> <span class="hint_blue"><var>cardinalThrough20(TENTHS)</var> time</span>
to get to <code>\pink{<var>SOLN_TXT</var>}</code>.
</p>
<p data-else="">
Starting at <code><var>LOWER_BOUND</var></code>, we move a length of
<code>\blue{<var>commafy(MARK_INCREMENT)</var> \text{ <var>cardinalThrough20(TENTHS)</var> times}}</code>
<code>\blue{<var>commafy(MARK_INCREMENT)</var>}</code> <span class="hint_blue"><var>cardinalThrough20(TENTHS)</var> times</span>
to get to <code>\pink{<var>SOLN_TXT</var>}</code>.
</p>

Expand Down
11 changes: 5 additions & 6 deletions local-only/i18n.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
(function() {
// Perseus running in local mode depends on $_, which is defined here
if (typeof React !== 'undefined') {
var createFragment = React.__internalAddons.createFragment;
}

/* TODO(csilvers): fix these lint errors (http://eslint.org/docs/rules): */
/* eslint-disable camelcase, comma-dangle, max-len, no-undef, prefer-template */
/* To fix, remove an entry above, run "make linc", and fix errors. */
Expand Down Expand Up @@ -367,10 +372,4 @@ window.i18n = {
ngetpos: ngetpos };


// TODO(csilvers): remove this once everyone has moved from $._ to i18n._
// Then, we can get rid of the i18n.js entry in third_party_js.py
jQuery._ = _;
jQuery.ngettext = ngettext;
jQuery.i18nDoNotTranslate = i18nDoNotTranslate;

})();
4 changes: 4 additions & 0 deletions local-only/update_local.sh
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ cp -f "$srcdir"/moment-khansrc/moment.js "$destdir"
# system doesn't like. We also need to wrap this in an IIFE to avoid
# leaking internal vars.
( echo "(function() {"
echo "// Perseus running in local mode depends on \$_, which is defined here"
echo "if (typeof React !== 'undefined') {"
echo " var createFragment = React.__internalAddons.createFragment;"
echo "}\n"
sed /module.exports/q "$webapp_root"/genfiles/compiled_es6/en/javascript/shared-package/i18n.js | grep -v -e '= require("' -e 'module.exports ='
echo "})();"
) > "$destdir"/i18n.js
Expand Down

0 comments on commit 95e2c88

Please sign in to comment.