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

Create addMessage method for parserMultiAnswer.pl (alternative to #999) #1001

Merged

Conversation

somiaj
Copy link
Contributor

@somiaj somiaj commented Feb 3, 2024

This is an alternative to #999 addressing the comments by @drgrice1. This adds a new method, addMessage, that can add multiple messages which will all be joined together in the end. This also uses a different method, so calling setMessage(0, $message) will still function like it use to and add a message to the last message rule.

  The MultiAnswer object addMessage method will add the given
  message to an array of single_ans_messages. If using singleResult,
  these messages are joined together and displayed for the whole
  result, not attached to any specific answer rule. In addition
  any specific answer rule messages are appended to the final message.
@somiaj
Copy link
Contributor Author

somiaj commented Feb 3, 2024

Here is the test problem I was using for this.

DOCUMENT();
loadMacros('PGstandard.pl', 'PGML.pl', 'parserMultiAnswer.pl', 'PGcourse.pl');

$ma = MultiAnswer(1, 2, 3)->with(
    singleResult => 1,
    checker => sub {
        my ($correct, $student, $self, $ans) = @_;

        $self->setMessage('2', 'Message2');
        $self->addMessage('First single message');
        $self->addMessage('Second single message');
        return 0.5;
    }
);

BEGIN_PGML
a) Enter '1': [_]{$ma}

b) Enter '2': [_]{$ma}

c) Enter '3': [_]{$ma}
END_PGML
ENDDOCUMENT();

@drgrice1
Copy link
Member

drgrice1 commented Feb 3, 2024

I think that you misinterpreted my second comment in the other pull request. That was not meant to point out behavior that I believe is correct or good, but rather to point out behavior that I think is contrary to the intention of the code (as I interpret it, although @drdrew42 may have meant otherwise). I don't think that having the zero case set the message for the last answer is a good thing, and I think it is meant for the die statement to occur if an index does not match the existing answer indices counting from one. It is just what happens with the code prior to these pull request (and with the other pull request if single result is not set).

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.

Although, this does address my first comment that I think to me is more important. This is more how I envision doing this also. I have been meaning to basically implement what you have in this pull request.

@somiaj
Copy link
Contributor Author

somiaj commented Feb 3, 2024

I wasn't thinking of the behavior of 0 in setMessage good or bad, but addressed that comment to keep current behavior.

Copy link
Member

@pstaabp pstaabp left a comment

Choose a reason for hiding this comment

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

Looks good.

@pstaabp pstaabp merged commit 0eeb12c into openwebwork:develop Feb 7, 2024
2 checks passed
@somiaj somiaj deleted the multi-answer-single-message-array branch September 29, 2024 03:51
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.

3 participants