From 5bd5fed98535c6d02d800276e63eea48c3758d60 Mon Sep 17 00:00:00 2001 From: Michael Gage Date: Sun, 21 May 2017 17:32:56 -0400 Subject: [PATCH 1/5] white space changes --- lib/PGcore.pm | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/PGcore.pm b/lib/PGcore.pm index fc4794fea9..cc041d9fe1 100755 --- a/lib/PGcore.pm +++ b/lib/PGcore.pm @@ -491,6 +491,8 @@ sub record_array_name { # currently the same as record ans name $label; } + + sub extend_ans_group { # modifies the group type my $self = shift; my $label = shift; @@ -507,6 +509,7 @@ sub extend_ans_group { # modifies the group type } $label; } + sub record_unlabeled_ans_name { my $self = shift; $self->{unlabeled_answer_blank_count}++; From d92b1673aa8fcf95a57998750a18b89789d19334 Mon Sep 17 00:00:00 2001 From: Michael Gage Date: Sun, 21 May 2017 17:35:23 -0400 Subject: [PATCH 2/5] Reword warnings --- macros/PG.pl | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/macros/PG.pl b/macros/PG.pl index 502000992c..186de7bca0 100644 --- a/macros/PG.pl +++ b/macros/PG.pl @@ -300,16 +300,18 @@ sub CLEAR_RESPONSES { } ''; } + +#FIXME -- explain the difference between insert_response and extend_response sub INSERT_RESPONSE { my $ans_label = shift; my $response_label = shift; my $ans_value = shift; my $selected = shift; - # warn "\n\nanslabel $ans_label responselabel $response_label value $ans_value"; + # warn "\n\nin PG.pl\nanslabel $ans_label responselabel $response_label value $ans_value"; if (defined ($PG->{PG_ANSWERS_HASH}->{$ans_label}) ) { my $responsegroup = $PG->{PG_ANSWERS_HASH}->{$ans_label}->{response}; $responsegroup->append_response($response_label, $ans_value, $selected); - #warn "\n$responsegroup responses are now ", $responsegroup->responses; + # warn "There are ", scalar($responsegroup->responses), " $responsegroup responses." ; } ''; } @@ -319,14 +321,15 @@ sub EXTEND_RESPONSE { # for radio buttons and checkboxes my $response_label = shift; my $ans_value = shift; my $selected = shift; - # warn "\n\nanslabel $ans_label responselabel $response_label value $ans_value"; + # warn "\n\nin PG.pl \nanslabel $ans_label responselabel $response_label value $ans_value"; if (defined ($PG->{PG_ANSWERS_HASH}->{$ans_label}) ) { my $responsegroup = $PG->{PG_ANSWERS_HASH}->{$ans_label}->{response}; $responsegroup->extend_response($response_label, $ans_value,$selected); - #warn "\n$responsegroup responses are now ", pretty_print($response_group); + # warn "\n$responsegroup responses are now ", pretty_print($response_group); } ''; } + sub ENDDOCUMENT { # check that answers match # gather up PG_FLAGS elements From 98763192f2ed5ab8bb4e58a0a99bde35fb726dec Mon Sep 17 00:00:00 2001 From: Michael Gage Date: Sun, 21 May 2017 17:48:25 -0400 Subject: [PATCH 3/5] Core fix for multiAnswer bug. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The NAMED_ANSWER_EXTENSION() routine needs to update the PGresponse object in the PGanswergroup. Previously, when KEPT_ANSWERS was used, the routine calling NAMED_ANSWER_EXTENSION() had to add the extension answer blank to the KEPT_ANSWERS queue using RECORD_FORM_LABEL. Now the routine must include a key/value answer_group_name=>’name of answer group’ so the entire call in multiAnswer looks like: $data->named_ans_rule_extension( $name,$size, answer_group_name => $self->{answerName}, @_); --- macros/PGbasicmacros.pl | 9 ++++++--- macros/parserMultiAnswer.pl | 13 ++++++++++--- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/macros/PGbasicmacros.pl b/macros/PGbasicmacros.pl index 4697ad01bd..34637f4bd3 100644 --- a/macros/PGbasicmacros.pl +++ b/macros/PGbasicmacros.pl @@ -447,7 +447,7 @@ sub NAMED_ANS_RULE_OPTION { # deprecated } sub NAMED_ANS_RULE_EXTENSION { - my $name = shift; + my $name = shift; # this is the name of the response item my $col = shift; my %options = @_; @@ -457,7 +457,9 @@ sub NAMED_ANS_RULE_EXTENSION { } else { $label = generate_aria_label($name); } - + # this is the name of the parent answer group + my $answer_group_name = $options{answer_group_name}//''; + # warn "from named answer rule extension in PGbasic answer_group_name: |$answer_group_name|"; my $answer_value = ''; $answer_value = ${$inputs_ref}{$name} if defined(${$inputs_ref}{$name}); if ( defined( $rh_sticky_answers->{$name} ) ) { @@ -466,7 +468,8 @@ sub NAMED_ANS_RULE_EXTENSION { } # $answer_value =~ tr/\\$@`//d; #`## make sure student answers can not be interpolated by e.g. EV3 $answer_value =~ s/\s+/ /g; ## remove excessive whitespace from student answer - INSERT_RESPONSE($name,$name,$answer_value); #FIXME hack -- this needs more work to decide how to make it work + # warn "from named answer rule extension in PGbasic: name: |$name| answer value: |$answer_value|"; + INSERT_RESPONSE($answer_group_name,$name,$answer_value); #FIXME hack -- this needs more work to decide how to make it work $answer_value = encode_pg_and_html($answer_value); my $tcol = $col/2 > 3 ? $col/2 : 3; ## get max diff --git a/macros/parserMultiAnswer.pl b/macros/parserMultiAnswer.pl index d0d6160ef5..97e98b3414 100644 --- a/macros/parserMultiAnswer.pl +++ b/macros/parserMultiAnswer.pl @@ -450,9 +450,16 @@ sub ans_rule { my $label = main::generate_aria_label($answerPrefix.$name."_0"); return $data->named_ans_rule($name,$size,@_,aria_label=>$label); } - return $data->named_ans_rule_extension($self->NEW_NAME($name),$size,@_) - if ($self->{singleResult} && $self->{part} > 1); - return $data->named_ans_rule($name,$size,@_); + if ($self->{singleResult} && $self->{part} > 1) { + my $extension_ans_rule = + $data->named_ans_rule_extension( + $name,$size, answer_group_name => $self->{answerName}, + @_); + # warn "extension rule created: $extension_ans_rule"; + return $extension_ans_rule; + } else { + return $data->named_ans_rule($name,$size,@_); + } } # From afe09759ed483fb423643213422a1ae2730b6f6c Mon Sep 17 00:00:00 2001 From: Michael Gage Date: Tue, 23 May 2017 15:44:44 -0400 Subject: [PATCH 4/5] Update documentation --- macros/PG.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/macros/PG.pl b/macros/PG.pl index 186de7bca0..b65f163754 100644 --- a/macros/PG.pl +++ b/macros/PG.pl @@ -301,7 +301,7 @@ sub CLEAR_RESPONSES { ''; } -#FIXME -- explain the difference between insert_response and extend_response +#FIXME -- examine the difference between insert_response and extend_response sub INSERT_RESPONSE { my $ans_label = shift; my $response_label = shift; From 5bbefa4b8fb6b7fd741277b60030af693cb9091a Mon Sep 17 00:00:00 2001 From: Michael Gage Date: Tue, 6 Jun 2017 08:19:30 -0400 Subject: [PATCH 5/5] Correct passing answer_group_name. This now appears to work for all multiAnswer and matrix answer evaluators. Debugging messages have been removed --- lib/Value/AnswerChecker.pm | 25 ++++++++++++++++--------- macros/PGbasicmacros.pl | 26 +++++++++++++++++++++++--- macros/parserMultiAnswer.pl | 10 +++++++--- 3 files changed, 46 insertions(+), 15 deletions(-) diff --git a/lib/Value/AnswerChecker.pm b/lib/Value/AnswerChecker.pm index b18a42b4c7..3dc3dfc165 100644 --- a/lib/Value/AnswerChecker.pm +++ b/lib/Value/AnswerChecker.pm @@ -396,24 +396,31 @@ sub ans_matrix { $self->{ans_name} = $ename; $self->{ans_rows} = $rows; $self->{ans_cols} = $cols; + # warn "ans_matrix: ename=$ename answer_group_name=$options{answer_group_name}"; my @array = (); foreach my $i (0..$rows-1) { my @row = (); foreach my $j (0..$cols-1) { my $label; if ($options{aria_label}) { - $label = $options{aria_label}.'row '.($i+1).' col '.($j+1); + $label = $options{aria_label}.'row '.($i+1).' col '.($j+1); } else { - $label = pgCall('generate_aria_label',ANS_NAME($ename,$i,$j)); + $label = pgCall('generate_aria_label',ANS_NAME($ename,$i,$j)); } + my $answer_group_name = $options{answer_group_name}//$name; if ($i == 0 && $j == 0) { - if ($extend) { - push(@row,&$named_extension($name,$size,ans_label=>$name,aria_label=>$label)); - } else { - push(@row,&$named_ans_rule($name,$size,aria_label=>$label)); - } - } else { - push(@row,&$named_extension(ANS_NAME($ename,$i,$j),$size,ans_label=>$name,aria_label=>$label)); + if ($extend) { + push(@row,&$named_extension($name,$size, + answer_group_name=> $answer_group_name, + aria_label=>$label) + ); + } else { + push(@row,&$named_ans_rule($name,$size,aria_label=>$label)); + } + } else { + push(@row,&$named_extension(ANS_NAME($ename,$i,$j),$size, + answer_group_name => $answer_group_name, + aria_label=>$label)); } } push(@array,[@row]); diff --git a/macros/PGbasicmacros.pl b/macros/PGbasicmacros.pl index 34637f4bd3..7f2ce4847b 100644 --- a/macros/PGbasicmacros.pl +++ b/macros/PGbasicmacros.pl @@ -459,6 +459,10 @@ sub NAMED_ANS_RULE_EXTENSION { } # this is the name of the parent answer group my $answer_group_name = $options{answer_group_name}//''; + unless ($answer_group_name) { + WARN_MESSAGE("Error in NAMED_ANSWER_RULE_EXTENSION: every call to this subroutine needs + to have \$options{answer_group_name} defined. Answer blank name: $name"); + } # warn "from named answer rule extension in PGbasic answer_group_name: |$answer_group_name|"; my $answer_value = ''; $answer_value = ${$inputs_ref}{$name} if defined(${$inputs_ref}{$name}); @@ -468,7 +472,8 @@ sub NAMED_ANS_RULE_EXTENSION { } # $answer_value =~ tr/\\$@`//d; #`## make sure student answers can not be interpolated by e.g. EV3 $answer_value =~ s/\s+/ /g; ## remove excessive whitespace from student answer - # warn "from named answer rule extension in PGbasic: name: |$name| answer value: |$answer_value|"; + # warn "from NAMED_ANSWER_RULE_EXTENSION in PGbasic: + # answer_group_name: |$answer_group_name| name: |$name| answer value: |$answer_value|"; INSERT_RESPONSE($answer_group_name,$name,$answer_value); #FIXME hack -- this needs more work to decide how to make it work $answer_value = encode_pg_and_html($answer_value); @@ -1095,8 +1100,23 @@ sub NAMED_ANS_ARRAY_EXTENSION{ # $answer_value =~ tr/\\$@`//d; #`## make sure student answers can not be interpolated by e.g. EV3 # warn "ans_label $options{ans_label} $name $answer_value"; - if (defined($options{ans_label}) ) { - INSERT_RESPONSE($options{ans_label}, $name, $answer_value); + my $answer_group_name; # the name of the answer evaluator controlling this collection of responses. + # catch deprecated use of ans_label to pass answer_group_name + if (defined($options{ans_label})) { + WARN_MESSAGE("Error in NAMED_ANS_ARRAY_EXTENSION: the answer group name should be passed in ", + "\%options using answer_group_name=>\$answer_group_name", + "The use of ans_label=>\$answer_group_name is deprecated.", + "Answer blank name: $name" + ); + $answer_group_name = $options{ans_label}; + } + if (defined($options{answer_group_name}) ) { + $answer_group_name = $options{answer_group_name}; + } + if ($answer_group_name) { + INSERT_RESPONSE($options{answer_group_name}, $name, $answer_value); + } else { + WARN_MESSAGE("Error: answer_group_name must be defined for $name"); } $answer_value = encode_pg_and_html($answer_value); diff --git a/macros/parserMultiAnswer.pl b/macros/parserMultiAnswer.pl index 97e98b3414..898158d7f1 100644 --- a/macros/parserMultiAnswer.pl +++ b/macros/parserMultiAnswer.pl @@ -455,7 +455,7 @@ sub ans_rule { $data->named_ans_rule_extension( $name,$size, answer_group_name => $self->{answerName}, @_); - # warn "extension rule created: $extension_ans_rule"; + # warn "extension rule created: $extension_ans_rule for ", ref($data); return $extension_ans_rule; } else { return $data->named_ans_rule($name,$size,@_); @@ -473,10 +473,14 @@ sub ans_array { my $name = $self->ANS_NAME($self->{part}++); if ($self->{singleResult} && $self->{part} == 1) { my $label = main::generate_aria_label($answerPrefix.$name."_0"); - return $data->named_ans_array($name,$size,@_,aria_label=>$label); + return $data->named_ans_array($name,$size, + answer_group_name => $self->{answerName}, + @_,aria_label=>$label); } if ($self->{singleResult} && $self->{part} > 1) { - $HTML = $data->named_ans_array_extension($self->NEW_NAME($name),$size,@_); + $HTML = $data->named_ans_array_extension($self->NEW_NAME($name),$size, + answer_group_name => $self->{answerName}, @_); + # warn "array extension rule created: $HTML for ", ref($data); } else { $HTML = $data->named_ans_array($name,$size,@_); }