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

Commit

Permalink
Fixed a few TODOs.
Browse files Browse the repository at this point in the history
Summary:
1) Moved jshint to a third-party directory

2) Moved calculator.js to genfiles.  This is because I've noticed
several commits over the years that have edited calculator.js
directly, only to have that edit overwritten the next time it was
auto-regenerated.  By putting it in genfiles, I hope to indicate to
people that they should change the source .jison file instead.  (I
still check in the genfiles/ file, so folks can use khan-exercises
without needing to install jison.)

While testing, I realized that we don't do a very good job of
installing deps for you, so I added deps-manifests for the three (!)
programming languages we use, and changed the Makefile to use them.

Small cleanups based on review.

Test Plan:
To test the jshint move, I ran:
   env LC_ALL=en_US.UTF-8 .bundle exec ruby build/pack.rb exercises/z_scores_3.html::exercises-packed/z_scores_3.html
without error.

To test the calculator move/change, I ran:
   python -mSimpleHTTPServer
and visited
   http://localhost:8000/exercises/law_of_cosines.html
and clicked '3+4=' and got 7, and clicked 'ln(-5)' and saw the bar
turn red because it's an error.  (It looks like the actual error
message is suppressed these days.)

(same)

Reviewers: eater, alpert

Reviewed By: alpert

Differential Revision: http://phabricator.khanacademy.org/D9356
  • Loading branch information
csilvers committed May 27, 2014
1 parent 8a73c5e commit 1915034
Show file tree
Hide file tree
Showing 12 changed files with 81 additions and 40 deletions.
5 changes: 2 additions & 3 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
*.pyc
_hashed-*.js
.bundle
node_modules
exercises-packed
exercises-packed.zip
genfiles/**
build/lxml
6 changes: 6 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
source 'https://rubygems.org'

gem 'json', '1.8.0'
gem 'nokogiri', '1.5.10'
gem 'therubyracer', '0.12.0'
gem 'uglifier', '2.3.1'
27 changes: 20 additions & 7 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,15 +1,28 @@
utils/calculator.js: build/calculator/calculator.jison build/calculator/calculator-tail.js
jison -m js build/calculator/calculator.jison -o utils/Calculator.js
cat build/calculator/calculator-tail.js >>utils/Calculator.js
mv utils/Calculator.js utils/calculator.js
.FAKE: pack packed lint fix_lint

# This needs 'npm' on your system, to install jison.
# The output file has to be named 'Calculator' because that's how jison
# determines the variable name.
genfiles/calculator.js: build/calculator/calculator.jison build/calculator/calculator-tail.js
npm install
mkdir -p genfiles
node_modules/.bin/jison -m js build/calculator/calculator.jison -o genfiles/Calculator.js
cat build/calculator/calculator-tail.js >>genfiles/Calculator.js
mv genfiles/Calculator.js genfiles/calculator.js


# Pack all files in exercises/ into exercises-packed/, unless the one
# in exercises-packed is newer.
pack packed:
args=`cd exercises && find * -name '*.html' | while read infile; do outfile="../exercises-packed/$$infile"; echo "$$outfile" | xargs dirname | xargs mkdir -p; [ "$$outfile" -nt "$$infile" ] || echo "exercises/$$infile::exercises-packed/$$infile"; done`; echo "$$args" | tr ' ' '\012'; [ -z "$$args" ] || ruby build/pack.rb $$args
# This needs 'bundle' on your system, to install needed ruby modules.
pack packed: deps
args=`cd exercises && find * -name '*.html' | while read infile; do outfile="../exercises-packed/$$infile"; echo "$$outfile" | xargs dirname | xargs mkdir -p; [ "$$outfile" -nt "$$infile" ] || echo "exercises/$$infile::exercises-packed/$$infile"; done`; echo "$$args" | tr ' ' '\012'; [ -z "$$args" ] || env LC_ALL=en_US.UTF-8 bundle exec ruby build/pack.rb $$args


# These need 'pip' on your system, to install lxml/etc.
lint:
pip install -r requirements.txt
python build/lint_i18n_strings.py exercises/*html

fix_lint:
python build/lint_i18n_strings.py --fix exercises/*html
pip install -r requirements.txt
python build/lint_i18n_strings.py --fix exercises/*html
15 changes: 8 additions & 7 deletions build/calculator/calculator-tail.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ window.Calculator = (function(parser) {
if (ans !== undefined) {
return ans;
} else {
throw new CalculatorError("Invalid variable ans");
throw new CalculatorError($._("Invalid variable ans"));
}
} else if (tree === "pi") {
return Math.PI;
Expand All @@ -61,35 +61,35 @@ window.Calculator = (function(parser) {
tan: function(a) {
var ans = Math.tan(toRad(a));
if (isNaN(ans) || Math.abs(ans) > Math.pow(2, 53)) {
throw new CalculatorError("undefined");
throw new CalculatorError($._("undefined"));
}
return ans;
},
asin: function(a) {
var ans = fromRad(Math.asin(a));
if (isNaN(ans)) {
throw new CalculatorError("undefined");
throw new CalculatorError($._("undefined"));
}
return ans;
},
acos: function(a) {
var ans = fromRad(Math.acos(a));
if (isNaN(ans)) {
throw new CalculatorError("undefined");
throw new CalculatorError($._("undefined"));
}
return ans;
},
atan: function(a) {
var ans = fromRad(Math.atan(a));
if (isNaN(ans)) {
throw new CalculatorError("undefined");
throw new CalculatorError($._("undefined"));
}
return ans;
},
ln: function(a) {
var ans = Math.log(a);
if (isNaN(ans) || !isFinite(ans)) {
throw new CalculatorError("undefined");
throw new CalculatorError($._("undefined"));
}
return ans;
}
Expand All @@ -105,7 +105,8 @@ window.Calculator = (function(parser) {
}
} else {
throw new CalculatorError(
"Invalid type " + Object.prototype.toString.call(tree));
$._("Invalid type %(type)s",
{type: Object.prototype.toString.call(tree)}));
}
},

Expand Down
2 changes: 1 addition & 1 deletion build/pack.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ def uglify(js, options)
Dir.chdir(File.join(File.dirname(__FILE__), ".."))

@uglifier = Uglifier.new(:copyright => false) # Discard all comments
@jshint = ExecJS.compile(File.read("build/jshint.js"))
@jshint = ExecJS.compile(File.read("build/third_party/jshint.js"))

def uglifier_insane(output)
$stderr.puts "-- unexpected uglifier output: " + output.inspect
Expand Down
File renamed without changes.
15 changes: 8 additions & 7 deletions utils/calculator.js → genfiles/calculator.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 4 additions & 6 deletions khan-exercise.js
Original file line number Diff line number Diff line change
Expand Up @@ -260,14 +260,12 @@ var Khan = {
// TODO(alpert): This doesn't need to be in the Khan object.
getBaseModules: function() {
var mods = [];
// Base modules required for every problem
// MathJax is here because Perseus wants it loaded regardless of if
// we load a khan-exercises problem that needs it. Previously it
// was a dependency of 'math' so this isn't really any different.
// Base modules required for every problem. These are specified
// as filenames (minus the .js extension) relative to util/.
// subhints is here to support the intervention experiment.
mods.push(
"answer-types", "tmpl", "tex", "jquery.adhesion",
"calculator", "scratchpad", "subhints");
"scratchpad", "subhints");

return mods;
},
Expand Down Expand Up @@ -1804,7 +1802,7 @@ function prepareSite() {
$(".calculator-decimal").html(separator);
}

require(["./utils/calculator.js"], initializeCalculator);
require(["./genfiles/calculator.js"], initializeCalculator);
Khan.initReportIssueLink("#extras .report-issue-link");

$("#answer_area").delegate("input.button, select", "keydown", function(e) {
Expand Down
7 changes: 1 addition & 6 deletions lint_blacklist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,8 @@
local-only/**
genfiles/**
third_party/**
build/third_party/**
test/**

# Config files that don't (and shouldn't) end in a semicolon
requirejs.config.js

# Third-party code in build/. TODO(csilvers): move to third_party/?
build/jshint.js

# TODO(csilvers): fix these
utils/calculator.js
26 changes: 26 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
{
"name": "khan-exercises",
"version": "1.0.0",
"author": "Khan Academy",
"keywords": "education exercises khanacademy khan math exercises",
"description": "(Some) exercises for Khan Academy",
"licenses": [
{
"type": "MIT",
"url": "http://www.opensource.org/licenses/mit-license.php"
},
{
"type": "CC Attribution/Non-Commercial/Share-Alike",
"url": "http://creativecommons.org/licenses/by-nc-sa/3.0/"
}
],

"repository": {
"type": "git",
"url": "http://github.com/Khan/khan-exercises/"
},

"devDependencies": {
"jison" : "0.4.13"
}
}
6 changes: 4 additions & 2 deletions requirejs.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@

// Base modules used by every problem (matches Khan.getBaseModules())
"utils/answer-types.js",
"utils/calculator.js",
"utils/tex.js",
"utils/tmpl.js",
"utils/jquery.adhesion.js",

// Loaded separately.
"genfiles/calculator.js",

// Utils required asynchronously by khan-exercise.js (when files are
// required without a callback, r.js will include them automatically)
"utils/scratchpad.js",
Expand Down Expand Up @@ -66,7 +68,7 @@
out: "genfiles/exercise-content-bundle.js",
shim: {
// Wrap these with a define() wrapper to defer execution until required
"utils/calculator.js": true,
"genfiles/calculator.js": true,
"utils/jquery.adhesion.js": true,
"third_party/jquery.cursor-position.js": true,
"third_party/jquery.mobile.vmouse.js": true
Expand Down
2 changes: 1 addition & 1 deletion test/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
<script>
require([
"../utils/answer-types.js",
"../utils/calculator.js",
"../genfiles/calculator.js",
"../utils/expressions.js",
"../utils/kline.js",
"../utils/kmatrix.js",
Expand Down

0 comments on commit 1915034

Please sign in to comment.