Merge pull request #1372 from bjones1/lp-fix

Fix small bugs in the literate programming question type

Author: bnmnetp

runestone/lp/js/lp.js

@@ -54,14 +54,27 @@ class LP extends RunestoneBase {
             .siblings(".lp-feedback")
             .children("div");
         // Use a nice editor.
-        let that = this;
         this.textAreas = [];
-        $(".code_snippet").each(function (index, element) {
+        this.initTextAreas();
+        // Handle clicks to the "Save and run" button.
+        let that = this;
+        $(this.element).click((eventObject) =>
+            that.onSaveAndRun(eventObject).then(null)
+        );
+        this.checkServer("lp_build", true);
+    }
+
+    // Look for each code snippet (a textarea). Make it a nice CodeMirror editor.
+    initTextAreas(is_read_only = false) {
+        let that = this;
+        // Select only textareas that haven't been initialized with CodeMirror (which sets style="display:none" on the textarea).
+        $('textarea.code_snippet:not([style="display: none;"])').each(function (index, element) {
             let editor = CodeMirror.fromTextArea(element, {
                 lineNumbers: true,
                 mode: $(that.element).attr("data-lang"),
                 indentUnit: 4,
                 matchBrackets: true,
+                readOnly: is_read_only,
                 autoMatchParens: true,
                 extraKeys: { Tab: "indentMore", "Shift-Tab": "indentLess" },
             });
@@ -75,11 +88,6 @@ class LP extends RunestoneBase {
             // Keep track of it.
             that.textAreas.push(editor);
         });
-        // Handle clicks to the "Save and run" button.
-        $(this.element).click((eventObject) =>
-            that.onSaveAndRun(eventObject).then(null)
-        );
-        this.checkServer("lp_build", true);
     }
 
     // Data structures:
@@ -137,7 +145,7 @@ class LP extends RunestoneBase {
         // Store the answer as a string, since this is what goes in to / comes out from the database. We have to translate this back to a data structure when restoring from the db or local storage.
         let code_snippets = this.textareasToData();
         this.setLocalStorage({
-            answer: JSON.stringify({ code_snippets: code_snippets }),
+            answer: { code_snippets: code_snippets },
             timestamp: new Date(),
         });
         // Store the answer that the server returns, which includes additional data (correct/incorrect, feedback from the build, etc.).
@@ -170,8 +178,6 @@ class LP extends RunestoneBase {
         }
         serverAnswer.answer.code_snippets = code_snippets;
         this.displayAnswer(serverAnswer);
-        // JSON-encode the answer for storage.
-        serverAnswer.answer = JSON.stringify(serverAnswer.answer);
         this.setLocalStorage(serverAnswer);
     }
 
@@ -213,17 +219,29 @@ class LP extends RunestoneBase {
 
     // Store an array of strings in ``data.code_snippets`` into each textarea.
     dataToTextareas(data) {
-        // Find all code snippet textareas.
+        // For places where textareas aren't provided (instructor grading interface, assignment page), create them. However, these aren't runnable, so disable the "Save and run" button. Assume this is the case when there are no textareas. Reasoning: the page containing the problem contains code intended to be intermingled with the student's answer. If that's not present, we can't build.
+        if (!this.textAreas.length) {
+            // The unusual case -- a problem not on its original page. Create textareas to display the code, then initialize them. Note that we can't simply create these in the constructor (which would be convenient) since we don't yet know how many code snippets are associated with the problem.
+            $(this.element).before((data.answer.code_snippets || []).map(() => `<textarea class="code_snippet"></textarea><br />`));
+            // Put CodeMirror in read-only mode.
+            this.initTextAreas(true);
+            // Disable the "Save and run" button.
+            $(this.element).prop("disabled", true);
+            // Now that we have textareas, let the usual case code run.
+        }
+
+        // The usual case -- a problem on its original page. Find all code snippet textareas.
         $(this.textAreas).each(function (index, value) {
             // Silently ignore if ``data.answer.code_snippets`` or ``data.answer.code_snippets[index]`` isn't defined.
             value.setValue((data.answer.code_snippets || "")[index] || "");
         });
+        // Corner case TODO: if a problem is edited after students already submit code so that there are now fewer textareas, display these but don't save them when "Save and run" is pressed. This allows students to (hopefully) re-use code from now-discarded textareas.
     }
 
     // Restore answers from storage retrieval done in RunestoneBase.
     restoreAnswers(data) {
-        // We store the answer as a JSON-encoded string in the db / local storage. Restore the actual data structure from it.
-        data.answer = JSON.parse(data.answer);
+        // We store the answer as a JSON-encoded string in the db / local storage. Restore the actual data structure from it. Avoid exceptions if no data is available.
+        data.answer = JSON.parse(data.answer || "{}");
         this.dataToTextareas(data);
         this.displayAnswer(data);
     }
@@ -249,7 +267,10 @@ class LP extends RunestoneBase {
     }
 
     setLocalStorage(data) {
-        localStorage.setItem(this.localStorageKey(), JSON.stringify(data));
+        // Make a shallow copy, so we can JSON-encode the code snippets (matching what comes from the server) without changing the original object.
+        data_clone = object.assign({}, data);
+        data_clone.answer =  JSON.stringify(data.answer);
+        localStorage.setItem(this.localStorageKey(), JSON.stringify(data_clone));
     }
 }
 

runestone/lp/lp.py

@@ -195,7 +195,7 @@ class _LpBuildButtonDirective(RunestoneIdDirective):
     # No optional arguments.
     optional_arguments = 0
     # Per http://docutils.sourceforge.net/docs/howto/rst-directives.html, True if content is allowed. However, this isn't checked or enforced.
-    has_content = True
+    has_content = False
     # Options. Everything but language is currently ignored. This is based on activecode, so in the future similar support would be provided for these options.
     option_spec = RunestoneIdDirective.option_spec.copy()
     option_spec.update(
@@ -232,10 +232,6 @@ def run(self):
         lp_node["source"], lp_node["line"] = self.state_machine.get_source_and_line(
             self.lineno
         )
-        # Insert the question number.
-        self.content.append(self.options["qnumber"], "lp")
-        # Parse it, since the number may be a role.
-        self.state.nested_parse(self.content, self.content_offset, lp_node)
 
         return [lp_node]