Skip to content

Commit

Permalink
handle various variable errors better
Browse files Browse the repository at this point in the history
If a variable has an empty definition, that doesn't break the rest of them.
The error message for a variable is now also shown beneath the definition, in addition to the "generated value" column.
Variables with duplicate names are highlighted in red.
  • Loading branch information
christianp committed Apr 1, 2015
1 parent 233d1f7 commit 42e31d2
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 10 deletions.
14 changes: 13 additions & 1 deletion editor/static/css/question/edit.css
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,13 @@ input.recalculate-checkbox {
font-weight: bold;
}

.variable-selector .name-error {
text-decoration: underline;
color: red;
font-style: italic;
font-weight: bold;
}

.variable-selector.noname .name {
font-family: inherit;
font-weight: bold;
Expand All @@ -287,9 +294,10 @@ input.recalculate-checkbox {
overflow-x: hidden;
}

.variable-selector .value.error {
.variable-selector.error .value {
color: red;
font-weight: bold;
max-height: none;
}

.variable {
Expand All @@ -312,6 +320,10 @@ input.recalculate-checkbox {
font-style: italic;
}

.variable .value-error {
margin-top: 1em;
}

.variable .inline-names {
margin: 1em 0;
}
Expand Down
3 changes: 3 additions & 0 deletions editor/static/js/numbas/jme-variables.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,9 @@ jme.variables = /** @lends Numbas.jme.variables */ {
}
}

if(!v.tree) {
throw(new Numbas.Error('jme.variables.empty definition',name));
}
try {
scope.variables[name] = jme.evaluate(v.tree,scope);
} catch(e) {
Expand Down
30 changes: 24 additions & 6 deletions editor/static/js/question/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -729,7 +729,12 @@ $(document).ready(function() {
var prep = this.prepareVariables();

this.variables().map(function(v) {
v.dependencies(prep.todo[v.name().toLowerCase()].vars);
var name = v.name().toLowerCase();
if(prep.todo[name]) {
v.dependencies(prep.todo[name].vars);
} else {
v.dependencies([]);
}
});

var results = this.computeVariables(prep);
Expand Down Expand Up @@ -759,6 +764,11 @@ $(document).ready(function() {
this.variables().map(function(v) {
var name = v.name().toLowerCase();
var result = results.variables[name];
if(!result) {
v.value(null);
v.error('');
return;
}
if('value' in result) {
v.value(result.value);
}
Expand Down Expand Up @@ -819,8 +829,18 @@ $(document).ready(function() {
//make structure of variables to evaluate
var todo = {}
this.variables().map(function(v) {
if(!v.name() || !v.definition())
var name = v.name().toLowerCase();
if(!v.name()) {
return;
}
if(!v.definition()) {
todo[name] = {
v: v,
tree: null,
vars: []
};
return;
}
try {
var tree = jme.compile(v.definition(),scope,true);
var vars = jme.findvars(tree);
Expand All @@ -829,7 +849,7 @@ $(document).ready(function() {
v.error(e.message);
return;
}
todo[v.name().toLowerCase()] = {
todo[name] = {
v: v,
tree: tree,
vars: vars
Expand Down Expand Up @@ -1320,9 +1340,7 @@ $(document).ready(function() {
var variables = q.variables();
for(var i=0;i<variables.length;i++) {
var v = variables[i];
if(v==this)
break;
else if(v.name().toLowerCase()==name.toLowerCase())
if(v!=this && v.name().toLowerCase()==name.toLowerCase())
return 'There\'s already a variable with this name.';
}

Expand Down
8 changes: 5 additions & 3 deletions editor/templates/question/editable.html
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,10 @@ <h4 data-bind="text: displayName,visible: fixed || $root.editVariableGroup()!=$d
</div>
</div>
<script type="text/html" id="variable-template">
<tr data-bind="click: $root.currentVariable, css: {noname: !name(), current: $data==$root.currentVariable(), dependency: isDependency}" class="variable-selector">
<td class="name" data-bind="text: name() ? name() : 'Unnamed'"></td>
<tr data-bind="click: $root.currentVariable, css: {noname: !name(), current: $data==$root.currentVariable(), dependency: isDependency, error: error().length>0}" class="variable-selector">
<td class="name" data-bind="text: name() ? name() : 'Unnamed', css: {'name-error': nameError()!=''}"></td>
<td class="value-cell">
<div class="value" data-bind="css: {error: error().length>0}, html: display"></div>
<div class="value" data-bind="html: display"></div>
</td>
<td class="delete-cell">
<button type="button" title="Remove this variable" class="delete" data-bind="click:remove"></button>
Expand Down Expand Up @@ -221,6 +221,8 @@ <h4 data-bind="text: displayName,visible: fixed || $root.editVariableGroup()!=$d
</ul>
(text strings)
</div>
<div class="value-error alert alert-danger" data-bind="visible: error().length>0, html: error">
</div>
</div>
</div>
<div class="control-group">
Expand Down

0 comments on commit 42e31d2

Please sign in to comment.