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.

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

fix bugs with answerHints from issue #964 #974

Merged
merged 2 commits into from
Dec 18, 2023

Conversation

Alex-Jordan
Copy link
Contributor

This fixes the issues discussed in #964.

@Alex-Jordan
Copy link
Contributor Author

Thanks @dpvc. This now also addresses #872.

@Alex-Jordan
Copy link
Contributor Author

Each issue has a MWE problem to test. For me, they both work following these changes.

@pstaabp
Copy link
Member

pstaabp commented Dec 4, 2023

This fixes #872, however I'm not seeing answer hints show up at all for the various MWEs. Reading through #964 we need to implement the translation between the answer and the internal value like @dpvc pointed out, but that doesn't work for me either.

@Alex-Jordan
Copy link
Contributor Author

I thought we merged this yesterday and so last night, I closed the two issues tied to this. Sorry!

Copy link
Member

@drgrice1 drgrice1 left a comment

Choose a reason for hiding this comment

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

This looks good, and works well in my testing. This makes answer hints work well with parserRadioButtons.pl, parserCheckboxLists.pl, and parserPopUp.pl answers. See my comments on this in #964.

It is also good to fix the reference address issue.

@drgrice1
Copy link
Member

drgrice1 commented Dec 6, 2023

I realized that I forgot to test issue #872 with this. The answer hints in the original MWE in that issue still do not work with this pull request. However, the errors that were begin shown are fixed.

I added the suggestion of @dpvc to the MWE and that does get the answer hints to work (you need to replace backslashes with two tildes though do to translation). Here is the MWE that works:

DOCUMENT();
loadMacros('PGstandard.pl', 'PGML.pl', 'parserNumberWithUnits.pl', 'answerHints.pl');

$ans = NumberWithUnits('1 m');
$cmp = $ans->cmp->withPostFilter(sub {
	my $ansHash = shift;
	if ($ansHash->{student_ans} =~ m/^~~s*(?:~~d+(?:~~.~~d*)?|~~.~~d+)~~s*$/) {
		$ansHash->{student_value}   = main::Real($ansHash->{student_ans});
		$ansHash->{student_formula} = main::Formula($ansHash->{student_value});
	}
	return $ansHash;
})->withPostFilter(AnswerHints(2 => [ 'no', replaceMessage => 1 ]));

BEGIN_PGML
Enter [`[$ans]`].

[_]{$cmp}{10}
END_PGML
ENDDOCUMENT();

@Alex-Jordan
Copy link
Contributor Author

Do I understand right that this can be merged, and #964 can remain closed, but #872 should be reopened but with extended comments? I think we ultimately want something where you do not have to insert that postFilter, and so #872's original MWE remains an issue although not quite for the same reasons. Or maybe #872 remains closed but I open a new issue.

@drgrice1
Copy link
Member

This can be merged. If you want to re-open #872 or create another issue regarding creating an easier way to make answer hints work with radio buttons or checkboxes, that would be good.

@pstaabp pstaabp merged commit fd7267a into openwebwork:develop Dec 18, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants