Skip to content

Fix an issue with parserAssignment.pl introduced in #609#622

Merged
pstaabp merged 1 commit into
openwebwork:mainfrom
drgrice1:fix-parser-assignment-parsing
Jan 18, 2022
Merged

Fix an issue with parserAssignment.pl introduced in #609#622
pstaabp merged 1 commit into
openwebwork:mainfrom
drgrice1:fix-parser-assignment-parsing

Conversation

@drgrice1

Copy link
Copy Markdown
Member

See the discussion in #609 for a test case.

…in PGML via

`[:: ::]` now fail to parse.
@drgrice1 drgrice1 force-pushed the fix-parser-assignment-parsing branch from e9e2713 to 302ba3f Compare December 28, 2021 03:09
@drgrice1 drgrice1 requested review from dpvc and drdrew42 December 28, 2021 03:10
@pstaabp

pstaabp commented Dec 29, 2021

Copy link
Copy Markdown
Member

It does fix this error.

Since I haven't used this macro before, reading the documentation, if there is a single variable, there seems to supposed to be an error, but Is the example about supposed to be skipped as an assignment since there isn't a single variable on the left?

@drgrice1

Copy link
Copy Markdown
Member Author

The parser::Assignment::Formula new method is called for the entire string in the example, and the type of that string is not an Assignment. It is resolved to a Number by the Value::Formula package. As such it does not have the data hash key which an Assignment would have. That causes an error on the later lines of the new method. Basically, not everything that is passed through the new method should end up being an Assignment.

@dpvc can hopefully give a better explanation, and I am hoping he can take a look at this when he gets a chance.

@pstaabp

pstaabp commented Dec 29, 2021

Copy link
Copy Markdown
Member

Just a heads up, should this also be added to develop? Or is the plan to pull in any hot fixes to main back to develop at some point?

@drgrice1

Copy link
Copy Markdown
Member Author

Yes, this will also be added to develop once it is approved. At this point all hotfix pull requests that have been merged have also had a duplicate pull request merged into develop.

@dpvc dpvc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks reasonable.

@pstaabp pstaabp merged commit 4400e4f into openwebwork:main Jan 18, 2022
@drgrice1 drgrice1 deleted the fix-parser-assignment-parsing branch January 19, 2022 02:53
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