-
-
Notifications
You must be signed in to change notification settings - Fork 80
PTX stuff for parserRadioMultiAnswer #1359
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
base: develop
Are you sure you want to change the base?
Conversation
drgrice1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this. Hopefully this means that usage of this macro is starting to take hold!
|
There is some more to do here, so I converted to draft. |
|
OK, this can be reviewed again.
|
drgrice1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks good still other than a few suggestions.
|
Since I added things, I need to update the POD... |
|
Yeah, I noticed the |
|
OK, I think all that's been discussed has been addressed now. |
Well, I made use of it a lot over the last year in a linear algebra course, and then in a differential equations course. Any situation where maybe something can't be done, or maybe it can, but if it can then go ahead and do it. That was all direct PG problem writing though, not with PTX. So I set out to add an example of using this in the PTX sample docs, and that led to this PR. |
drgrice1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is another problem that I forgot to check on before. This seems to be a common problem that you have to watch for with the showInStatic option. You have to ensure that you don't just bail on the ans_rule method before the answer is recorded. Otherwise there are problems. For example, in hardcopy if you want to show student answers, then the answers for the radio multi answer either won't be shown, or if there are answers after it, then it will be but the later answers won't be. That is a problem with this implementation. It is going to be tricky to get this right for this macro. I will see what I can work up, unless you get to it first. The important things are that that lines 771 and 772 are executed for all parts and that the named_ans_array_extension and named_ans_rule_extension methods are called on lines 673 and 689.
| sub ans_rule { | ||
| my ($self, $size, @options) = @_; | ||
|
|
||
| return '' if !$self->{showInStatic} && ($main::displayMode eq 'TeX' || $main::displayMode eq 'PTX'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To ensure that the answer is recorded, this line will need to be moved to after the for loop that begins at line 661 below. Then another copy of this line will need to be added at line 773 in the begin_radio method. Also add next if !$self->{showInStatic} && ($main::displayMode eq 'TeX' || $main::displayMode eq 'PTX'); after line 663 in the for loop below (the named_ans_array_extension and named_ans_rule_extension methods do not need to be called contrary to what I said before).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, scratch the next ... that I said to add on line 663 in the for loop. The named_ans_array_extension and named_ans_rule_extension methods do need to be called. So just remove the existing line 652, and copy it to the other two locations I gave before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working that out. I made those changes and force pushed.
drgrice1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good now.
I think I never got around to helping make parserRadioMultiAnswer work well for PreTeXt. These changes do that with my testing.
ul. And the attributes being used here make PTX understand theulis for answering something.