Convert to PGML: add the ability to parse loadMacros if the arguments are in a qw#1425
Convert to PGML: add the ability to parse loadMacros if the arguments are in a qw#1425pstaabp wants to merge 3 commits into
Conversation
block. Also, removes empty macros and duplicate macros. In addition, if it appears that the problem is already in PGML mode, then return the file without changes.
pstaabp
changed the title
|
There are some outlier cases that are doing some weird things with this. These cases probably are not worth putting much effort into, but they could happen, and are valid Perl, and this turns them into invalid Perl. Either loadMacros(qw{
PGstandard.pl
PGML.pl
draggableProof.pl
PGcourse.pl
');The second case is probably more of an issue, because that is probably not that unlikely. Closely related and more serious though is the following. loadMacros(qw{
PGstandard.pl
PGML.pl
draggableProof.pl}
PGcourse.pl
);This needs to handle the possibility of white space between the |
pstaabp
force-pushed
the
pgml-convert-improvements
branch
from
1db3b80 to
ba081b2
Compare
|
This addresses the cases of @drgrice1. I also tried to format the output on a single line if the input was on a single line and multiple lines if the input was that way. |
drgrice1
left a comment
drgrice1
left a comment
There was a problem hiding this comment.
This still needs some more work.
| # Check that the file is not already in PGML format by looking for PGML.pl in the loadMacros statement. | ||
| # and there are no BEGIN_TEXT, BEGIN_SOLUTION, etc. blocks. |
There was a problem hiding this comment.
There is something wrong with this comment. The first line ends with a period, and the second line starts with a lowercase "and".
| while (1) { | ||
| # Remove comments within loadMacros block (should we keep them?) | ||
| $row =~ s/#.*$//; | ||
| $macros .= $row; | ||
| last if ($row =~ /(.*)\);/); | ||
| ++$num_macro_lines; | ||
| $row = shift @rows; | ||
| my @mrow = split(/#/, $row); | ||
| # This only adds the row if there is something relevent to the left of a # | ||
| $macros .= $mrow[0] if $mrow[0] !~ /^\s*$/; | ||
| } |
There was a problem hiding this comment.
This could potentially loop forever. I tried to convert the following to PGML, and that happened:
DOCUMENT();
loadMacros('PGstandard.pl', 'PGML.pl', 'parserMultiAnswer.pl', 'PGcourse.pl')
BEGIN_TEXT
Bad stuff
END_TEXT
Yes, it is intentionally designed to make this loop forever, but someone using the PG problem editor should not be able to crash the server by trying to convert bad code. Note that the above code does not cause an infinite loop with the code for this in the PG-2.21 branch.
There was a problem hiding this comment.
Good point. I'm wondering the best way to handle this situation. We could throw an error in this situation, but I don't want this to become a syntax checker.
There was a problem hiding this comment.
To an extent, any code formatter needs to be a syntax checker. If the code it is trying to format is not valid, then it really cannot proceed. All of the code formatters such as perltidy and prettier work this way. Basically, what should happen is that if something invalid is encountered, then the formatter should just stop and give some error message.
Regardless, the point is that the infinite loop cannot happen. So this pull request is stopped until this is a definite no go until this is resolved.
3 participants
Add the ability to parse loadMacros if the arguments are in a qw block. Also, removes empty macros and duplicate macros.
In addition, if it appears that the problem is already in PGML mode, then return the file without changes.
This is in response to openwebwork/webwork2#2908