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.

drgrice1 · 2026-07-22T13:55:17Z

The first example I mentioned in #1425 (comment) is still not working.

pstaabp · 2026-07-23T13:16:24Z

I forgot to check that one again. I was double checking for comments. That issue is fixed now.

I decided to strip all comments from any loadMacros call. I was trying to figure out how to keep them, but also adding PGML.pl and removing any duplicates or unnecessary macros makes that quite difficult.

drgrice1 · 2026-07-24T12:34:14Z

Why does the bin/convert-to-pgml.pl have a -b|--backup option? It never writes to the original .pg file, so that option is entirely pointless.

Why does the bin/convert-to-pgml.pl write the converted problem to a file with a different extension than .pg? It uses the -s|--suffix option for the extension defaulting the .pgml. That is not a good way to go. It should use an option like that but instead it should use an extension like .pgml.pg (using the suffix option instead of pgml). That way you get an actual PG problem file.

drgrice1 · 2026-07-24T12:41:41Z

I see, when the -b|--backup option is used it does write to the original .pg file, but creates a backup first. In that case it also does not use the -s|--suffix option. Sorry, I missed that.

My second comment regarding the -s|--suffix option still stands.

pstaabp · 2026-07-27T12:11:43Z

Another update to add .pg to problem, so .pgml.pg is the default converted file suffix.

And rewrite the list of macros to be removed from the list.

The loadMacros parsing is now much more versatile. In addition to basic single and double quoting, and `qw` quotes, it can handle any quotelike quotes. There is a minimal attempt to keep comments inside of a `loadMacros` call. Basically, if a macro is removed from the list, then a comment after it is also removed. If a macro is kept, then so is the comment that was after it. If there is a line that only has a comment on it, then if the preceding line had a comment it is assumed that the comment continues that one, and if that is kept so is the next line. Otherwise the comment line is dropped. Previously the `MathObjects.pl` macro was removed if loaded, since it is loaded by `PGML.pl`. Now `niceTables.pl` is also removed, since `PGML.pl` also loads that. The naive approach of replacing `ans_rule` calls with PGML has been removed. Instead any `ans_rule` call is just wrapped in `[@ ... @]*`, and the corresponding `ANS` calls are left alone. One advantage of this approach is that after the conversion is completed, the problem actually works. In addition, the previous approach was clobbering the `ans_rule` calls in many cases that give information about what needs to be done to properly convert the answers into the PGML answer syntax. The con is that the conversion into PGML syntax must be done manually. However, since that really was the case before other than basically just putting a non-functional PGML answer template into place, that is not really a tradeoff. Instead of handling only two very specific forms of heredoc/nowdoc syntax `SOLUTION(EV3(<<'END_SOLUTION'));` and `TEXT(EV2(<<EOT))` where the heredoc/nowdoc terminator was fixed and must be exactly what is shown above, any such form is handled. The heredoc/nowdoc terminator can be anything the author specified and an unquoted, singled quoted, or double quoted terminator can be used. In addition both `EV2` and `EV3` are covered, and the `HINT` form is also handled. In addition spaces can be in those constructs as allowed by Perl. Note that there are OPL problems that do all of those things, so it was really quite naive to only literally cover those two cases. Variable interpolation now actually works much better, including for hash and array access, and hash reference and array reference access. Note that a file that already uses PGML and does not have any of the old style PG constructs is actually completely left alone. @pstaapb's openwebwork#1425 still modifies those files in some cases. The `bin/convert-to-pgml.pl` script no longer accepts `-s|--suffix` argument. Instead it accepts a `-e|--extension-prefix` argument. That is much the same except that its value is prepended to `.pg` instead of used as the extension itself. It defaults to `pgml` as the `suffix` option did before. So if `problem.pg` is converted, then the result will be written to `problem.pgml.pg`. Thus the resulting file has the correct pg problem file extension. A new option for the `bin/convert-to-pgml.pl`, `-p|--output-path` was added. When given, the converted file is written into that directory with the same name as the original file. The `-b|--backup` and `-e|--extension-prefix` options are ignored, unless the output path resolves to the directory containing the original file, in which case those options will still apply. TRhere were some other minor things fixed along the way as well that I did not list here, such as an issue with the conversion of the `$HR` variable into a PGML `---` in which the `---` was added, but the `$HR` variable left and changed into `[$HR]` resulting in ``` --- [$HR] ``` in the converted file. This was done in collaboration with Claude.

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

Another fix for parsing loadMacros.

9f50d87

pstaabp force-pushed the pgml-convert-improvements branch from f04f7d1 to 97f0093 Compare July 27, 2026 17:30

Add .pg to end of converted files with a suffix.

99ced63

And rewrite the list of macros to be removed from the list.

pstaabp force-pushed the pgml-convert-improvements branch from 97f0093 to 99ced63 Compare July 27, 2026 17:31

drgrice1 mentioned this pull request Jul 28, 2026

Pgml convert improvements revised #1482

Open

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!

drgrice1 commented Jul 22, 2026

Uh oh!

pstaabp commented Jul 23, 2026

Uh oh!

drgrice1 commented Jul 24, 2026

Uh oh!

drgrice1 commented Jul 24, 2026

Uh oh!

pstaabp commented Jul 27, 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