Skip to content

Convert to PGML: add the ability to parse loadMacros if the arguments are in a qw #1425

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.

Sign up for GitHub

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

Open
pstaabp wants to merge 3 commits into
base: PG-2.21
Choose a base branch
from
Open

Convert to PGML: add the ability to parse loadMacros if the arguments are in a qw #1425

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8100cfa
Add the ability to parse loadMacros if the arguments are in a qw
pstaabp May 28, 2026
9d0c244
remove debug statements
pstaabp Jun 1, 2026
ba081b2
Handle cases where qw blocks are mixed with strings in ' or "
pstaabp Jun 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 62 additions & 19 deletions lib/WeBWorK/PG/ConvertToPGML.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ my @ans_list;
sub convertToPGML {
my ($pg_source) = @_;

# 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.
Comment on lines +79 to +80

@drgrice1 drgrice1 Jun 20, 2026

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.

There is something wrong with this comment. The first line ends with a period, and the second line starts with a lowercase "and".


return $pg_source if ($pg_source =~ /loadMacros\((.*)PGML\.pl(.*)\)/ && $pg_source !~ /BEGIN_(TEXT|HINT|SOLUTION)/);

# First get a list of all of the ANS, LABELED_ANS, etc. in the problem.
@ans_list = getANS($pg_source);

Expand All @@ -101,29 +106,67 @@ sub convertToPGML {
} elsif ($in_pgml_block) {
push(@pgml_block, $row);
} elsif ($row =~ /loadMacros\(/) {
# Parse the macros, which may be on multiple rows.
# Remove comments within loadMacros block (should we keep them?)
my $macros = $row;
while ($row && $row !~ /\);\s*$/) {
# Parse the macros, which may be on multiple rows and may be in a qw block.
my $macros = '';
my $num_macro_lines = 0; # store the number of lines in the loadMacro so the output is similar to the input.
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*$/;
}
Comment on lines +112 to 119

@drgrice1 drgrice1 Jun 20, 2026

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 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.

@pstaabp pstaabp Jun 25, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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.

@drgrice1 drgrice1 Jun 25, 2026

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.

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.

# Split by commas and pull out the quotes.
my @macros =
grep { $_ !~ /^#/ }
grep {
$_ !~
/(PGstandard|PGML|PGauxiliaryFunctions|PGbasicmacros|PGanswermacros|MathObjects|PGcourse|AnswerFormatHelp).pl/
my @macros = ();
Comment thread
drgrice1 marked this conversation as resolved.
my ($qw_start, $qw_end); # the characters if the loadMacros has a qw block.

# The following can parse loadMacros in the form loadMacros('macro1.pl', 'macro2.pl'); or
# loadMacros(qw{macro1.pl macro2.pl});
if ($macros =~ /loadMacros\((.*?)\);/ms) {
my @macro_str = split(/\s*,\s*/, $1);

for my $str (@macro_str) {
if ($str =~ /^qw(.)/) {
my $qw_matches = { '{' => '}', '(' => ')', '[' => ']', '/' => '/', '|' => '|' };
$qw_start = $1;
$qw_end = $qw_matches->{$qw_start};

if ($str =~ /^qw\Q${qw_start}\E(.*?)\Q${qw_end}\E/) {
push(@macros, split(/\s+/, $1));
}
} else {
push(@macros, $str);
}
}
map {s/['"\s]//gr}
split(/\s*,\s*/, $macros =~ s/loadMacros\((.*)\)\;$/$1/r);

push(@all_lines,
'loadMacros('
. join(', ', map {"'$_'"} ('PGstandard.pl', 'PGML.pl', @macros, 'PGcourse.pl'))
. ');');
@macros =
grep {
$_
&& $_ !~
/(PGstandard|PGML|PGauxiliaryFunctions|PGbasicmacros|PGanswermacros|MathObjects|PGcourse|AnswerFormatHelp).pl/
Comment thread
pstaabp marked this conversation as resolved.
}
map {s/['"]//gr} @macros;

# Remove any duplicates:
my %seen;
@macros = grep { !$seen{$_}++ } @macros;
} else {
warn 'The loadMacros statement in this file could not be processed.';
}

@macros = ('PGstandard.pl', 'PGML.pl', @macros, 'PGcourse.pl');

if ($qw_start) {
if ($num_macro_lines > 1) { # put each macro on a separate line
push(@all_lines, "loadMacros(qw$qw_start");
push(@all_lines, "\t$_") for (@macros);
push(@all_lines, "$qw_end);");
} else {
push(@all_lines, "loadMacros(qw$qw_start" . join(' ', @macros) . "$qw_end);");
}
} else {
push(@all_lines, 'loadMacros(' . join(', ', map {"'$_'"} @macros) . ');');
}
} else {
push(@all_lines, cleanUpCode($row));
}
Expand Down
Loading