Convert to PGML: add the ability to parse loadMacros if the arguments are in a qw by pstaabp · Pull Request #1425 · openwebwork/pg

pstaabp · 2026-05-28T19:34:27Z

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

drgrice1 · 2026-06-01T19:14:42Z

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 MathObjects.pl}, 'draggableProof.pl'); or loadMacros(qw{PGstandard.pl MathObjects.pl draggableProof.pl},); becomes

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 MathObjects.pl draggableProof.pl} ); becomes

loadMacros(qw{
	PGstandard.pl
	PGML.pl
	draggableProof.pl}
	PGcourse.pl
 );

This needs to handle the possibility of white space between the qw ending and the ending parenthesis.

pstaabp · 2026-06-09T16:02:18Z

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

This still needs some more work.

pstaabp · 2026-07-07T18:45:01Z

This now checks for some parsing of the loadMacros call.

Additionally, this returns a hash for the output with any parsing errors. I will post a PR for WeBWorK to handle the errors in the PGeditor.

The command line works with no additional code.

pstaabp · 2026-07-08T11:14:52Z

Note: there is now updated PGEditor code in openwebwork/webwork2#3046 that handles errors in the PG problem editor.

drgrice1

Something is definitely not working. The example given by @somiaj in issue #2908 doesn't even work. It gives the error PGML conversion errors: The loadMacros command cannot be parsed. In fact, every single file I have tested, regardless of any content that it has, gives that error unless PGML.pl happens to be on the same line as the loadMacros call.

The bin/convert-to-pgml.pl script is really poorly written. There should at the very least be a help option using pod2usage from Pod::Usage. If the script is called with no arguments which are required, then instead of dying with the message that arguments must have a list of files the help should be shown. Also, the SYNOPSIS section in the POD should be updated to show the usual help that is shown with the help option with the pod2usage method that shows the way the script should be called with all options described.

Also, the ConvertToPGML.pm module exports symbols by default. That shouldn't happen. @EXPORT_OK should be used instead of @EXPORT.

I must not have reviewed this well before when you first added this. Those things should not have been allowed.

drgrice1 · 2026-07-09T15:22:56Z

+	# and there are no BEGIN_TEXT, BEGIN_SOLUTION, etc. blocks.
+
+	return { pgmlCode => $pg_source }
+		if ($pg_source =~ /loadMacros\((.*)PGML\.pl(.*)\)/m && $pg_source !~ /BEGIN_(TEXT|HINT|SOLUTION)/);


This extremely naive regular expression check is just not going to work. First, since you do not include the s flag on the regular expression, the . character does not match newlines. So unless PGML.pl is literally on the same line as the loadMacros call, this check will not find it. So this will catch something like loadMacros( #PGML.pl) which it shouldn't, but it will not catch something like

loadMacros(qw{ PGstandard.pl PGML.pl });

Note that just adding the s flag won't fix this either because then it would still catch the false positive noted above, but would also catch something like

loadMacros('PGstandard.pl'); # (This does not use PGML.pl) In addition, the second regular expression check will also catch many incorrect things. For example, if someone mentions `BEGIN_TEXT` in a comment in the file, but does not use it.

I had noticed this a few days ago too. I think changing to the /s on the RegEx and then

$pg_source =~ /loadMacros$(.*)PGML\.pl(.*)$\s*;/s

parses your first example correctly, but not the 2nd one.

I do see that putting BEGIN_TEXT comment will catch it incorrectly. Would

$pg_source !~ /^\s*BEGIN_(TEXT|HINT|SOLUTION)/ms

be sufficient?

My hope was to catch most of the cases that have already been converted.

You will need $pg_source !~ /^\s*BEGIN_(TEXT|HINT|SOLUTION)/m. Remove the s flag for this one. Then this will be pretty much what the translator uses. Note that the s flag makes . match a new line, so the ^ anchor would be nullified if the s flag is added.

The loadMacros check is more complicated. As I said, adding the s flag is not going to be enough. I am not sure what to do for this.

pstaabp · 2026-07-14T13:59:51Z

New version of this.

Added better documentation of the convert-to-pgml script.
Separated the loadMacros to a separate subroutine and perform error handling in the subroutine to better capture possible syntax errors.
Also, add a better way to parse the answer blanks in PGML form.

drgrice1 · 2026-07-14T15:05:06Z

This is still not working. Testing with the source

DOCUMENT();

loadMacros(    # PGML.pl)
    qw{
        PGstandard.pl
        MathObjects.pl
        parserPopUp.pl
        PGcourse.pl
    }
);
# (This file does not use PGML.pl.)

$mc = DropDown([ '1.5', '2 ', 3 ], 2);    # Correct answer is 3

BEGIN_TEXT
Problem text.
\{ $mc->ans_rule \}
END_TEXT

ANS($mc->cmp);

ENDDOCUMENT();

gives

DOCUMENT();

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

# (This file does not use PGML.pl.)

$mc = DropDown([ '1.5', '2 ', 3 ], 2);    # Correct answer is 3

BEGIN_PGML
Problem text.
[_]{$mc}
END_PGML

# ANS($mc->cmp);

ENDDOCUMENT();

It removes the parserPopUp.pl macro.

pstaabp · 2026-07-14T18:34:32Z

The newline between the loadMacros( and the qw{ was a problem. Just pushed another fix.

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.

in loadMacros

Also, return a hash of the converted code and any errors.

Add better documentation of the convert-to-pgml script. Separate the loadMacros to a separate subroutine and perform error handling in the subroutine to better capture possible syntax errors. Also, add a better way to parse the answer blanks in PGML form.

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.

in loadMacros

Also, return a hash of the converted code and any errors.

Add better documentation of the convert-to-pgml script. Separate the loadMacros to a separate subroutine and perform error handling in the subroutine to better capture possible syntax errors. Also, add a better way to parse the answer blanks in PGML form.

pstaabp changed the title ~~Add the ability to parse loadMacros if the arguments are in a qw~~ Convert to PGML: add the ability to parse loadMacros if the arguments are in a qw May 29, 2026

somiaj reviewed May 31, 2026

View reviewed changes

Comment thread lib/WeBWorK/PG/ConvertToPGML.pm Outdated

pstaabp force-pushed the pgml-convert-improvements branch from 1db3b80 to ba081b2 Compare June 9, 2026 15:50

drgrice1 reviewed Jun 20, 2026

View reviewed changes

Comment thread lib/WeBWorK/PG/ConvertToPGML.pm Outdated

Comment thread lib/WeBWorK/PG/ConvertToPGML.pm Outdated

Comment thread lib/WeBWorK/PG/ConvertToPGML.pm Outdated

Comment thread lib/WeBWorK/PG/ConvertToPGML.pm Outdated

pstaabp force-pushed the pgml-convert-improvements branch from ba081b2 to d8f5572 Compare July 7, 2026 18:42

pstaabp force-pushed the pgml-convert-improvements branch from d8f5572 to e8114b7 Compare July 8, 2026 11:01

pstaabp mentioned this pull request Jul 8, 2026

Add error handling in the pg problem editor openwebwork/webwork2#3046

Open

drgrice1 requested changes Jul 9, 2026

View reviewed changes

pstaabp added 6 commits July 14, 2026 15:14

Add the ability to parse loadMacros if the arguments are in a qw

4bcd539

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.

remove debug statements

b814fa0

Handle cases where qw blocks are mixed with strings in ' or "

4edb2b0

in loadMacros

Add some error checking for loadMacros on the source

c8bd029

Also, return a hash of the converted code and any errors.

Update to handle another loadMacros case.

3b91a3d

pstaabp force-pushed the pgml-convert-improvements branch from 245ac50 to 3b91a3d Compare July 14, 2026 19:15

pstaabp added 6 commits July 16, 2026 12:25

Add the ability to parse loadMacros if the arguments are in a qw

f378626

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.

remove debug statements

9ccd028

Handle cases where qw blocks are mixed with strings in ' or "

9f50297

in loadMacros

Add some error checking for loadMacros on the source

f357023

Also, return a hash of the converted code and any errors.

Update to handle another loadMacros case.

663322f

Uh oh!

Uh oh!

Conversation

pstaabp commented May 28, 2026

Uh oh!

Uh oh!

drgrice1 commented Jun 1, 2026

Uh oh!

pstaabp commented Jun 9, 2026

Uh oh!

drgrice1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pstaabp commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pstaabp commented Jul 8, 2026

Uh oh!

drgrice1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

drgrice1 Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

pstaabp Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

drgrice1 Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

pstaabp commented Jul 14, 2026

Uh oh!

drgrice1 commented Jul 14, 2026

Uh oh!

pstaabp commented Jul 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pstaabp commented Jul 7, 2026 •

edited

Loading

pstaabp Jul 13, 2026 •

edited

Loading