Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patches to scaffold.pl so Preview does not modify which sections are open #506

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Patches to scaffold.pl so Preview does not modify which sections are open #506

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
18d56d9
Patches to allow preventing scaffold from changing which sections
taniwallach Apr 20, 2023
d7778a2
Allow hiding the the structure of a MultiAnswer answer during
taniwallach Apr 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 69 additions & 10 deletions macros/core/scaffold.pl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,21 @@ =head1 DESCRIPTION
have the student open it by hand before anwering the questions. In
this case, set this value to 0 (it is 1 by default).

=item C<S<< allow_next_section_to_open_on_preview => 0 or 1 >>>

This determines whether Preview My Answers should be allowed to open
a new section. Set this to 0 to prevent the next section from opening
after Preview My Answers is clicked (it is 1 by default).

=item C<S<< prevent_close_by_preview => 0 or 1 >>>

This determines whether Preview My Answers should be allowed to close
a section which was open before, if the prior answers were changed to be
incorrect. By default the setting is 0, so such section closures can
occur due to a Preview, while when set to 1 sections will not be closed by
Preview.


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please delete the extra empty line here. No multiple empty lines! perltidy removes the ones not in POD, but doesn't handle multiple empty lines in POD.

=item C<S<< numbered => 0 or 1 >>>

This determines whether each section is automatically numbered before
Expand Down Expand Up @@ -316,6 +331,7 @@ sub Begin {
unshift(@scaffolds, $self);
$scaffold = $self;
$scaffold_depth++;
$self->{key_last_open} = "ScAfFoLd_${PREFIX}_SC-" . $self->{number} . "_last_open";
$self->{previous_output} =
[ splice(@{$PG_OUTPUT}, 0) ]; # get output and clear it without changing the array pointer
$self->{output} = []; # the contents of the scaffold
Expand All @@ -338,6 +354,12 @@ sub End {
$self->hide_other_results(@{ $self->{open} }); # hide results of unnopened sections in the results table
push(@$PG_OUTPUT, @{ $self->{previous_output} }, @{ $self->{output} })
; # put back original output and scaffold output

if (!$self->{isPreview}) {
# Record the last_open value for future renders
main::update_persistent_data($self->{key_last_open}, $self->{open}[-1] ? $self->{open}[-1] : 0);
}

delete $self->{previous_output};
delete $self->{output}; # don't need these any more
shift(@scaffolds);
Expand Down Expand Up @@ -372,6 +394,11 @@ sub new {
hardcopy_is_open => "always", # open all possible sections in hardcopy
open_first_section => 1, # 0 means don't open any sections initially
numbered => 0, # 1 means sections will be printed with their number

# control whether Preview should be allowed to open/close sections
allow_next_section_to_open_on_preview => 1, # can Preview open another section
prevent_close_by_preview => 0, # can Preview close previously open sections

@_,
number => ++$scaffold_no, # the number for this scaffold
depth => $scaffold_depth, # the nesting depth for this scaffold
Expand Down Expand Up @@ -402,7 +429,7 @@ sub start_section {

#
# Add the content from the current section into the scaffold's output
# and remove the current_section (so we can tell that no sectionis open).
# and remove the current_section (so we can tell that no section is open).
#
sub end_section {
my $self = shift;
Expand All @@ -415,8 +442,10 @@ sub end_section {
# keeping the scores for future reference.
#
sub section_answers {
my $self = shift;
my ($self, @this_section_answers) = @_;
my %answers;
my $section_no = $self->{section_no};
my $prior_last_open = main::get_persistent_data($self->{key_last_open}) // 0;
#
# MultiAnswer objects can set the answer hash score when the last answer is evaluated,
# so save the hashes and look up the scores after they have all been called.
Expand All @@ -425,21 +454,51 @@ sub section_answers {
# to consider the score to be 1 (bug in PGessaymacros.pl prevents us from using
# it for essay_cmp(), though).
#
push(@{ $self->{ans_names} }, @_);
foreach my $name (@_) {
push(@{ $self->{ans_names} }, @this_section_answers);
foreach my $name (@this_section_answers) {
my $input = $main::inputs_ref->{$name};
my $evaluator = $PG_ANSWERS_HASH->{$name}->ans_eval;
Parser::Eval(sub { $answers{$name} = $evaluator->evaluate($input) }) if defined($input) && $input ne "";
$answers{$name}{score} = 1
if $answers{$name} && (($answers{$name}{type} || "") eq "essay" || $answers{$name}{"scaffold_force"});
$evaluator->{rh_ans}{ans_message} = "";
delete $evaluator->{rh_ans}{error_message};
# In sections which are not yet open, make that known to answer
# evaluators. For MultiAnswer using a single result, this allows
# hiding the structure of the answer during a preview.
$PG_ANSWERS_HASH->{$name}{ans_eval}{rh_ans}{scaffold_past_open} = 1 if ($section_no > $prior_last_open);
}

my $myLastSet; # we save the name of the last answer field whose score was set
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$myLastSet is a poor choice of variable name. my is a reserve word used to declare a variable, it should not be part of the name. Just use $lastSet. Although this is a confusing name. What was last set? Something more descriptive of what it means would be better.

for my $name (@this_section_answers) {
if ($answers{$name}) {
$self->{scores}{$name} = $answers{$name}{score};
$myLastSet = $name;
}
}
if ($myLastSet) {
$self->{isPreview} = $answers{$myLastSet}{isPreview} if (!defined($self->{isPreview}));
if ($self->{isPreview}) { # Only handle for Preview mode
if (
($section_no == $prior_last_open) # Only should consider modifying the last previously open section
&& !$self->{allow_next_section_to_open_on_preview}
)
{
# This should prevent the next section from opening on Preview
# if all answers to this section are correct, unless it would open anyway.
$self->{scores}{$myLastSet} = 0;
} elsif (($section_no < $prior_last_open) && $self->{prevent_close_by_preview}) {
# This section was open before - AVOID closing it due to a change in prior sections and a Preview
for my $name (@{ $self->{ans_names} }) {
$self->{scores}{$name} = 1.0 if ($answers{$name}); # to prevent it from being closed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why 1.0? Perl doesn't distinguish between integer and floating point data types, so there is no need. Just use 1.

}
}
}
}
foreach my $name (@_) { $self->{scores}{$name} = $answers{$name}{score} if $answers{$name} }
}

#
# Add the given sections to the list of sections to be openned
# Add the given sections to the list of sections to be opened
# for this scaffold
#
sub is_open {
Expand Down Expand Up @@ -601,7 +660,7 @@ sub new {
# Adds the necessary HTML around the content of the section.
#
# First, determine the is_correct and can_open status and save them.
# Then check if the section is to be openned, and if so, add it
# Then check if the section is to be opened, and if so, add it
# to the open list of the scaffold.
#
# The $PG_OUTPUT variable holds just the contents of this section,
Expand Down Expand Up @@ -745,11 +804,11 @@ sub new_answers {
package Section::can_open;

#
# Always can be openned
# Always can be opened
#
sub always { return 1 }
#
# Can be openned when all the answers from previous sections are correct
# Can be opened when all the answers from previous sections are correct
#
sub when_previous_correct {
my $section = shift;
Expand All @@ -772,7 +831,7 @@ sub incorrect {
return !$section->{is_correct};
}
#
# Never can be openned
# Never can be opened
#
sub never { return 0 }

Expand Down
23 changes: 16 additions & 7 deletions macros/parsers/parserMultiAnswer.pl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,10 @@ sub single_check {
$ans->{_filter_name} = "MultiAnswer Single Check";
my $inputs = $main::inputs_ref;
$self->{ans}[0] = $self->{cmp}[0]->evaluate($ans->{student_ans});
# Allow hiding the the structure of the answer for an
# answer in an unopened part of a scaffold problem.
my $hide_preview = 0;
$hide_preview = 1 if ($ans->{scaffold_past_open});
Comment on lines +295 to +298
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this need to be here in this convoluted format?

foreach my $i (1 .. $self->length - 1) {
$self->{ans}[$i] = $self->{cmp}[$i]->evaluate($inputs->{ $self->ANS_NAME($i) });
}
Expand Down Expand Up @@ -328,13 +332,18 @@ sub single_check {
. '</TABLE>';
}
if ($nonblank) {
$ans->{preview_latex_string} =
(
defined($self->{tex_format})
? sprintf($self->{tex_format}, @latex)
: join($self->{tex_separator}, @latex));
$ans->{preview_text_string} =
(defined($self->{format}) ? sprintf($self->{format}, @text) : join($self->{separator}, @text));
if ($hide_preview) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't this just be if ($ans->{scaffold_past_open}) {, with the part above deleted? (Although I don't think any of this is working anyway.)

$ans->{preview_latex_string} = "";
$ans->{preview_text_string} = "";
} else {
$ans->{preview_latex_string} =
(
defined($self->{tex_format})
? sprintf($self->{tex_format}, @latex)
: join($self->{tex_separator}, @latex));
$ans->{preview_text_string} =
(defined($self->{format}) ? sprintf($self->{format}, @text) : join($self->{separator}, @text));
}
$ans->{student_ans} =
(defined($self->{format}) ? sprintf($self->{format}, @student) : join($self->{separator}, @student));
}
Expand Down