From 0ea58017a57e63141ec476b21521d1c100f66f83 Mon Sep 17 00:00:00 2001
From: Chris Sangwin <C.J.Sangwin@ed.ac.uk>
Date: Wed, 23 Aug 2023 18:53:10 +0100
Subject: [PATCH] Fix to issue #789.

---
 backup/moodle2/backup_qtype_stack_plugin.class.php  |  2 +-
 db/install.xml                                      |  1 +
 db/upgrade.php                                      | 13 +++++++++++++
 doc/en/Authoring/Options.md                         | 11 +++++++++++
 doc/en/Authoring/Testing.md                         |  2 ++
 edit_stack_form.php                                 |  6 ++++++
 lang/en/qtype_stack.php                             |  9 ++++++---
 questiontype.php                                    |  5 +++++
 samplequestions/equivalence_reasoning_example.xml   |  1 +
 samplequestions/file_download.xml                   |  1 +
 .../stack-js-issue-specific-example-938.xml         |  1 +
 settings.php                                        |  5 +++++
 stack/cas/ast.container.class.php                   |  3 +++
 stack/maxima/stackmaxima.mac                        |  2 +-
 stack/options.class.php                             | 12 +++++++++++-
 stack/questiontest.php                              |  5 +++++
 tests/helper.php                                    |  4 ++++
 tests/questiontype_test.php                         |  3 +++
 version.php                                         |  2 +-
 19 files changed, 81 insertions(+), 7 deletions(-)

diff --git a/backup/moodle2/backup_qtype_stack_plugin.class.php b/backup/moodle2/backup_qtype_stack_plugin.class.php
index 210ba97af..0d17d5596 100644
--- a/backup/moodle2/backup_qtype_stack_plugin.class.php
+++ b/backup/moodle2/backup_qtype_stack_plugin.class.php
@@ -48,7 +48,7 @@ class backup_qtype_stack_plugin extends backup_qtype_plugin {
                 array('stackversion', 'questionvariables', 'specificfeedback', 'specificfeedbackformat',
                       'questionnote', 'questionsimplify', 'assumepositive', 'assumereal',
                       'prtcorrect', 'prtcorrectformat', 'prtpartiallycorrect', 'prtpartiallycorrectformat',
-                      'prtincorrect', 'prtincorrectformat', 'multiplicationsign', 'sqrtsign',
+                      'prtincorrect', 'prtincorrectformat', 'decimals', 'multiplicationsign', 'sqrtsign',
                       'complexno', 'inversetrig', 'logicsymbol', 'matrixparens', 'variantsselectionseed'));
 
         $stackinputs = new backup_nested_element('stackinputs');
diff --git a/db/install.xml b/db/install.xml
index 399f5ff5f..33b769b9e 100644
--- a/db/install.xml
+++ b/db/install.xml
@@ -24,6 +24,7 @@
         <FIELD NAME="prtpartiallycorrectformat" TYPE="int" LENGTH="2" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="Text format for the prtpartiallycorrect field."/>
         <FIELD NAME="prtincorrect" TYPE="text" NOTNULL="true" SEQUENCE="false" COMMENT="Standard feedback displayed for any PRT that returns a score of 0."/>
         <FIELD NAME="prtincorrectformat" TYPE="int" LENGTH="2" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="Text format for the prtincorrect field."/>
+        <FIELD NAME="decimals" TYPE="char" LENGTH="8" NOTNULL="true" DEFAULT="." SEQUENCE="false" COMMENT="The symbol to use for the decimal separator."/>
         <FIELD NAME="multiplicationsign" TYPE="char" LENGTH="8" NOTNULL="true" DEFAULT="dot" SEQUENCE="false" COMMENT="The symbol to use for multiplication."/>
         <FIELD NAME="sqrtsign" TYPE="int" LENGTH="4" NOTNULL="true" DEFAULT="1" SEQUENCE="false" COMMENT="Whether to display square roots as surds."/>
         <FIELD NAME="complexno" TYPE="char" LENGTH="8" NOTNULL="true" DEFAULT="i" SEQUENCE="false" COMMENT="How complex numbers should be displayed and represented."/>
diff --git a/db/upgrade.php b/db/upgrade.php
index 154f13b69..8337778f0 100644
--- a/db/upgrade.php
+++ b/db/upgrade.php
@@ -933,6 +933,19 @@ function xmldb_qtype_stack_upgrade($oldversion) {
         // Stack savepoint reached.
         upgrade_plugin_savepoint(true, 2023042800, 'qtype', 'stack');
     }
+
+    if ($oldversion < 2023082800) {
+        $table = new xmldb_table('qtype_stack_options');
+        $field = new xmldb_field('decimals', XMLDB_TYPE_CHAR, '8', null, XMLDB_NOTNULL, null, '.');
+
+        // Conditionally launch add field variantsselectionseed.
+        if (!$dbman->field_exists($table, $field)) {
+            $dbman->add_field($table, $field);
+        }
+
+        // STACK savepoint reached.
+        upgrade_plugin_savepoint(true, 2023082800, 'qtype', 'stack');
+    }
     // Add new upgrade blocks just above here.
 
     // Check the version of the Maxima library code that comes with this version
diff --git a/doc/en/Authoring/Options.md b/doc/en/Authoring/Options.md
index 85c545aec..6e0f43506 100644
--- a/doc/en/Authoring/Options.md
+++ b/doc/en/Authoring/Options.md
@@ -36,6 +36,17 @@ any penalty \(\ge 0.66\) and \(\le 0.67\) is changed to \(0.6666667\).
 
 The following options affect how mathematics is displayed.
 
+### Decimal separator ### {#decimals}
+
+Choose the symbol for the decimal separator used by student input.  There are currently two choices.
+
+* `.`, the British decimal point.
+* `,`, the comma, as used in much of Europe.
+
+The design of this option is discussed further in the [developer docs](../Developer/Syntax_numbers.md).
+
+Teachers must always use strict Maxima syntax, which requires `.`, including in test case construction.
+
 ### Multiplication Sign ### {#multiplication}
 
 * (none), e.g. \(x(x+1)\)
diff --git a/doc/en/Authoring/Testing.md b/doc/en/Authoring/Testing.md
index 00a2e6d53..9c52d0b80 100644
--- a/doc/en/Authoring/Testing.md
+++ b/doc/en/Authoring/Testing.md
@@ -56,6 +56,8 @@ Test cases can include a meaningful description of up to 255 characters.  This f
 
 Test cases are always written assuming `simp:false` regardless of the option set elsewhere.  If you want to construct a simplified test case then wrap this in `ev(... , simp)` to simplify the expression generating the test case.  This behaviour is required to enable construction of unsimplified test cases.
 
+Test cases are always written using the period `.` as the decimal separator.  This corresponds to strict Maxima syntax, which teachers should always use.
+
 You can (and should) constuct test cases based on invalid expressions.  If the raw testcase expression cannot be sent to the CAS, e.g. a missing bracket, then this invalidity will be tested.
 
 While test case construction uses `simp:false` Maxima must "evaluate" the expression prior to the result being used by an input as a test case.  This will replace variables by their values.  E.g. the typical case is to define a variable such as `ta` as the teacher's answer in the question variables field and use this throughout the question.  This answer will either be simplified, or not, when the question variables are evaluated.  To construct a test case using the teacher's answer use `ta` as the test case input.
diff --git a/edit_stack_form.php b/edit_stack_form.php
index 18c303cb2..ca891a30f 100644
--- a/edit_stack_form.php
+++ b/edit_stack_form.php
@@ -312,6 +312,11 @@ class qtype_stack_edit_form extends question_edit_form {
         $mform->getElement('prtincorrect')->setValue(array(
                         'text' => $this->stackconfig->prtincorrect));
 
+        $mform->addElement('select', 'decimals',
+            stack_string('decimals'), stack_options::get_decimals_sign_options());
+        $mform->setDefault('decimals', $this->stackconfig->decimals);
+        $mform->addHelpButton('decimals', 'decimals', 'qtype_stack');
+
         $mform->addElement('select', 'multiplicationsign',
                 stack_string('multiplicationsign'), stack_options::get_multiplication_sign_options());
         $mform->setDefault('multiplicationsign', $this->stackconfig->multiplicationsign);
@@ -662,6 +667,7 @@ class qtype_stack_edit_form extends question_edit_form {
                                             $opt->prtpartiallycorrect, $opt->prtpartiallycorrectformat, $question->id);
         $question->prtincorrect          = $this->prepare_text_field('prtincorrect',
                                             $opt->prtincorrect, $opt->prtincorrectformat, $question->id);
+        $question->decimals              = $opt->decimals;
         $question->multiplicationsign    = $opt->multiplicationsign;
         $question->complexno             = $opt->complexno;
         $question->inversetrig           = $opt->inversetrig;
diff --git a/lang/en/qtype_stack.php b/lang/en/qtype_stack.php
index e5bbda358..6b802256d 100644
--- a/lang/en/qtype_stack.php
+++ b/lang/en/qtype_stack.php
@@ -215,12 +215,15 @@ $string['insertstarsassumesinglechar'] = 'Insert stars assuming single-character
 $string['insertspaces'] = 'Insert stars for spaces only';
 $string['insertstarsspaces'] = 'Insert stars for implied multiplication and for spaces';
 $string['insertstarsspacessinglechar'] = 'Insert stars assuming single-character variables, implied and for spaces';
-$string['multiplicationsign'] = 'Multiplication sign';
-$string['multiplicationsign_help'] = 'Controls how multiplication signs are displayed.';
-$string['multiplicationsign_link'] = '%%WWWROOT%%/question/type/stack/doc/doc.php/Authoring/Options.md#multiplication';
+$string['decimals'] = 'Decimal separator';
+$string['decimals_help'] = 'Choose the symbol, and options, for the decimal separator.';
+$string['decimals_link'] = '%%WWWROOT%%/question/type/stack/doc/doc.php/Authoring/Options.md#decimals';
 $string['multcross'] = 'Cross';
 $string['multdot'] = 'Dot';
 $string['multonlynumbers'] = 'Only numbers';
+$string['multiplicationsign'] = 'Multiplication sign';
+$string['multiplicationsign_help'] = 'Controls how multiplication signs are displayed.';
+$string['multiplicationsign_link'] = '%%WWWROOT%%/question/type/stack/doc/doc.php/Authoring/Options.md#multiplication';
 $string['mustverify'] = 'Student must verify';
 $string['mustverify_help'] = 'Specifies whether the student\'s input is presented back to them as a forced two step process before this input is made available to the scoring mechanism.  Syntax errors are always reported back.';
 $string['mustverify_link'] = '%%WWWROOT%%/question/type/stack/doc/doc.php/Authoring/Inputs.md#Student_must_verify';
diff --git a/questiontype.php b/questiontype.php
index 5ca867b8e..cf5d0e18a 100644
--- a/questiontype.php
+++ b/questiontype.php
@@ -159,6 +159,7 @@ class qtype_stack extends question_type {
         $options->prtincorrect              = $this->import_or_save_files($fromform->prtincorrect,
                     $context, 'qtype_stack', 'prtincorrect', $fromform->id);
         $options->prtincorrectformat        = $fromform->prtincorrect['format'];
+        $options->decimals                  = $fromform->decimals;
         $options->multiplicationsign        = $fromform->multiplicationsign;
         $options->sqrtsign                  = $fromform->sqrtsign;
         $options->complexno                 = $fromform->complexno;
@@ -471,6 +472,7 @@ class qtype_stack extends question_type {
         }
 
         $question->options = new stack_options();
+        $question->options->set_option('decimals',           $questiondata->options->decimals);
         $question->options->set_option('multiplicationsign', $questiondata->options->multiplicationsign);
         $question->options->set_option('complexno',          $questiondata->options->complexno);
         $question->options->set_option('inversetrig',        $questiondata->options->inversetrig);
@@ -1157,6 +1159,7 @@ class qtype_stack extends question_type {
                         $options->prtpartiallycorrectformat, $contextid, 'prtpartiallycorrect', $questiondata->id);
         $output .= $this->export_xml_text($format, 'prtincorrect', $options->prtincorrect,
                         $options->prtincorrectformat, $contextid, 'prtincorrect', $questiondata->id);
+        $output .= "    <decimals>{$options->decimals}</decimals>\n";
         $output .= "    <multiplicationsign>{$options->multiplicationsign}</multiplicationsign>\n";
         $output .= "    <sqrtsign>{$options->sqrtsign}</sqrtsign>\n";
         $output .= "    <complexno>{$options->complexno}</complexno>\n";
@@ -1298,6 +1301,7 @@ class qtype_stack extends question_type {
         }
         $fromform->prtincorrect          = $this->import_xml_text($xml, 'prtincorrect', $format, $fformat);
         $fromform->penalty               = $format->getpath($xml, array('#', 'penalty', 0, '#'), 0.1);
+        $fromform->decimals              = $format->getpath($xml, array('#', 'decimals', 0, '#'), '.');
         $fromform->multiplicationsign    = $format->getpath($xml, array('#', 'multiplicationsign', 0, '#'), 'dot');
         $fromform->sqrtsign              = $format->getpath($xml, array('#', 'sqrtsign', 0, '#'), 1);
         $fromform->complexno             = $format->getpath($xml, array('#', 'complexno', 0, '#'), 'i');
@@ -1499,6 +1503,7 @@ class qtype_stack extends question_type {
         $fixingdollars = array_key_exists('fixdollars', $fromform);
 
         $this->options = new stack_options();
+        $this->options->set_option('decimals',           $fromform['decimals']);
         $this->options->set_option('multiplicationsign', $fromform['multiplicationsign']);
         $this->options->set_option('complexno',          $fromform['complexno']);
         $this->options->set_option('inversetrig',        $fromform['inversetrig']);
diff --git a/samplequestions/equivalence_reasoning_example.xml b/samplequestions/equivalence_reasoning_example.xml
index 22ed418bc..dc913d7d3 100644
--- a/samplequestions/equivalence_reasoning_example.xml
+++ b/samplequestions/equivalence_reasoning_example.xml
@@ -40,6 +40,7 @@
     <prtincorrect format="html">
       <text><![CDATA[<span style="font-size: 1.5em; color:red;"><i class="fa fa-times"></i></span> Incorrect answer.]]></text>
     </prtincorrect>
+    <decimals>.</decimals>
     <multiplicationsign>cross</multiplicationsign>
     <sqrtsign>1</sqrtsign>
     <complexno>i</complexno>
diff --git a/samplequestions/file_download.xml b/samplequestions/file_download.xml
index c5fadb472..208879572 100644
--- a/samplequestions/file_download.xml
+++ b/samplequestions/file_download.xml
@@ -49,6 +49,7 @@ taC: mean(map(third,data));
     <prtincorrect format="html">
       <text><![CDATA[<span style="font-size: 1.5em; color:red;"><i class="fa fa-times"></i></span> Incorrect answer.]]></text>
     </prtincorrect>
+    <decimals>.</decimals>
     <multiplicationsign>dot</multiplicationsign>
     <sqrtsign>1</sqrtsign>
     <complexno>i</complexno>
diff --git a/samplequestions/stack-js/stack-js-issue-specific-example-938.xml b/samplequestions/stack-js/stack-js-issue-specific-example-938.xml
index a56445a43..60906e289 100644
--- a/samplequestions/stack-js/stack-js-issue-specific-example-938.xml
+++ b/samplequestions/stack-js/stack-js-issue-specific-example-938.xml
@@ -115,6 +115,7 @@ simp:true;
     <prtincorrect format="html">
       <text></text>
     </prtincorrect>
+    <decimals>.</decimals>
     <multiplicationsign>dot</multiplicationsign>
     <sqrtsign>0</sqrtsign>
     <complexno>i</complexno>
diff --git a/settings.php b/settings.php
index 18c2e342b..a30278180 100644
--- a/settings.php
+++ b/settings.php
@@ -243,6 +243,11 @@ $settings->add(new admin_setting_configtextarea('qtype_stack/prtincorrect',
         get_string('symbolicprtincorrectfeedback', 'qtype_stack') . ' ' .
             get_string('defaultprtincorrectfeedback', 'qtype_stack'), PARAM_RAW, 60, 3));
 
+$settings->add(new admin_setting_configselect('qtype_stack/decimals',
+        get_string('decimals', 'qtype_stack'),
+        get_string('decimals_help', 'qtype_stack'), '.',
+        stack_options::get_decimals_sign_options()));
+
 $settings->add(new admin_setting_configselect('qtype_stack/multiplicationsign',
         get_string('multiplicationsign', 'qtype_stack'),
         get_string('multiplicationsign_help', 'qtype_stack'), 'dot',
diff --git a/stack/cas/ast.container.class.php b/stack/cas/ast.container.class.php
index 0643507fc..b4b0ab3a1 100644
--- a/stack/cas/ast.container.class.php
+++ b/stack/cas/ast.container.class.php
@@ -216,6 +216,9 @@ class stack_ast_container extends stack_ast_container_silent implements cas_late
          * (3) we want ? characters, and no semicolons.
          * (4) we want +- and not #pm#.
          * (5) ntuples have to be stripped off.
+         *
+         * Note we do not construct test cases using the decimal comma.
+         * The only place a comma is accepted is when a student really types it!
          */
 
         $dispval = $this->displayvalue;
diff --git a/stack/maxima/stackmaxima.mac b/stack/maxima/stackmaxima.mac
index ad814be08..79e852b18 100644
--- a/stack/maxima/stackmaxima.mac
+++ b/stack/maxima/stackmaxima.mac
@@ -3313,4 +3313,4 @@ is_lang(code):=ev(is(%_STACK_LANG=code),simp=true)$
 
 /* Stack expects some output with the version number the output happens at */
 /* maximalocal.mac after additional library loading */
-stackmaximaversion:2023072102$
+stackmaximaversion:2023082800$
diff --git a/stack/options.class.php b/stack/options.class.php
index 3db99be5d..81eedda36 100644
--- a/stack/options.class.php
+++ b/stack/options.class.php
@@ -37,7 +37,6 @@ class stack_options {
                 'caskey'     => 'OPT_OUTPUT',
                 'castype'    => 'string',
              ),
-            // Currently no way for users to set this option.
             'decimals'       => array(
                 'type'       => 'list',
                 'value'      => '.',
@@ -137,6 +136,7 @@ class stack_options {
     public function set_site_defaults() {
         $stackconfig = stack_utils::get_config();
         // Display option does not match up to $stackconfig->mathsdisplay).
+        $this->set_option('decimals', $stackconfig->decimals);
         $this->set_option('multiplicationsign', $stackconfig->multiplicationsign);
         $this->set_option('complexno', $stackconfig->complexno);
         $this->set_option('inversetrig', $stackconfig->inversetrig);
@@ -254,6 +254,16 @@ class stack_options {
         );
     }
 
+    /**
+     * @return array of choices for the decimal sign select menu.
+     */
+    public static function get_decimals_sign_options() {
+        return array(
+            '.'    => '.',
+            ','    => ',',
+        );
+    }
+
     /**
      * @return array of choices for the multiplication sign select menu.
      */
diff --git a/stack/questiontest.php b/stack/questiontest.php
index 7eaae6db7..6785752dd 100644
--- a/stack/questiontest.php
+++ b/stack/questiontest.php
@@ -81,6 +81,11 @@ class stack_question_test {
         // Create a completely clean version of the question usage we will use.
         // Evaluated state is stored in question variables etc.
         $question = question_bank::load_question($questionid);
+        // Hard-wire testing to use the decimal point.
+        // Teachers must use strict Maxima syntax, including in test case construction.
+        // I appreciate teachers will, reasonably, want to test the input mechanism but the added internal complexity here is serious.
+        // This complexity includes things like matrix input types which need a valid Maxima expression as the value of the input.
+        $question->options->set_option('decimals', '.');
         if (!is_null($seed)) {
             $question->seed = (int) $seed;
         }
diff --git a/tests/helper.php b/tests/helper.php
index dd31c538d..b81839e52 100644
--- a/tests/helper.php
+++ b/tests/helper.php
@@ -302,6 +302,7 @@ class qtype_stack_test_helper extends question_test_helper {
         $formform->prtcorrect = array('text' => 'Correct answer, well done!', 'format' => '1', 'itemid' => 0);
         $formform->prtpartiallycorrect = array('text' => 'Your answer is partially correct!', 'format' => '1', 'itemid' => 0);
         $formform->prtincorrect = array('text' => 'Incorrect answer :-(', 'format' => '1', 'itemid' => 0);
+        $formform->decimals = '.';
         $formform->multiplicationsign = 'dot';
         $formform->sqrtsign = '1';
         $formform->complexno = 'i';
@@ -1966,6 +1967,7 @@ class qtype_stack_test_helper extends question_test_helper {
         $qdata->options->prtpartiallycorrectformat = FORMAT_HTML;
         $qdata->options->prtincorrect              = self::DEFAULT_INCORRECT_FEEDBACK;
         $qdata->options->prtincorrectformat        = FORMAT_HTML;
+        $qdata->options->decimals                  = '.';
         $qdata->options->multiplicationsign        = 'dot';
         $qdata->options->sqrtsign                  = 1;
         $qdata->options->complexno                 = 'i';
@@ -2095,6 +2097,7 @@ class qtype_stack_test_helper extends question_test_helper {
         $qdata->options->prtpartiallycorrectformat = FORMAT_HTML;
         $qdata->options->prtincorrect              = self::DEFAULT_INCORRECT_FEEDBACK;
         $qdata->options->prtincorrectformat        = FORMAT_HTML;
+        $qdata->options->decimals                  = '.';
         $qdata->options->multiplicationsign        = 'dot';
         $qdata->options->sqrtsign                  = 1;
         $qdata->options->complexno                 = 'i';
@@ -2721,6 +2724,7 @@ class qtype_stack_test_helper extends question_test_helper {
                 'text' => 'Incorrect answer :-(',
                 'format' => '1',
                 'itemid' => 56111684);
+        $formform->decimals = '.';
         $formform->multiplicationsign = 'dot';
         $formform->sqrtsign = '1';
         $formform->complexno = 'i';
diff --git a/tests/questiontype_test.php b/tests/questiontype_test.php
index 22e7153ce..c80ffacae 100644
--- a/tests/questiontype_test.php
+++ b/tests/questiontype_test.php
@@ -218,6 +218,7 @@ class questiontype_test extends qtype_stack_walkthrough_test_base {
     <prtincorrect format="html">
       <text><![CDATA[<p>Incorrect answer.</p>]]></text>
     </prtincorrect>
+    <decimals>.</decimals>
     <multiplicationsign>dot</multiplicationsign>
     <sqrtsign>1</sqrtsign>
     <complexno>i</complexno>
@@ -339,6 +340,7 @@ class questiontype_test extends qtype_stack_walkthrough_test_base {
     <prtincorrect format="html">
       <text><![CDATA[<p>Incorrect answer.</p>]]></text>
     </prtincorrect>
+    <decimals>.</decimals>
     <multiplicationsign>dot</multiplicationsign>
     <sqrtsign>1</sqrtsign>
     <complexno>i</complexno>
@@ -441,6 +443,7 @@ class questiontype_test extends qtype_stack_walkthrough_test_base {
                                                     'format' => FORMAT_HTML, 'files' => array());;
         $expectedq->prtincorrect          = array('text' => '<p>Incorrect answer.</p>',
                                                     'format' => FORMAT_HTML, 'files' => array());;
+        $expectedq->decimals              = '.';
         $expectedq->multiplicationsign    = 'dot';
         $expectedq->sqrtsign              = 1;
         $expectedq->complexno             = 'i';
diff --git a/version.php b/version.php
index 14b84fef3..78dc02b71 100644
--- a/version.php
+++ b/version.php
@@ -24,7 +24,7 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$plugin->version   = 2023072102;
+$plugin->version   = 2023082800;
 $plugin->requires  = 2020061500;
 $plugin->cron      = 0;
 $plugin->component = 'qtype_stack';
-- 
GitLab