Add the capability in the pg problem editor to pg perltidy code. #2098

drgrice1 · 2023-06-30T16:30:16Z

This adds a new tab to the PG problem editor (unless editing a course configuration file). The tab contains basic information about what it does, but if the "Tidy Code" button is pressed it perltidies the code in the CodeMirror editor window. The tidied code is also saved to the temprorary file.

The actual tidying is done by a new webwork webservice action (not a form submission). If errors occur, they are shown in the renderer window.

The webwork webwservice depends on a new PG module defined in a corresponding PG pull request (see openwebwork/pg#868).

pstaabp · 2023-06-30T19:11:26Z

Haven't tried this out yet, but was thinking that we should add some documentation to the PGEditor help page for this.

drgrice1 · 2023-06-30T20:03:10Z

Yeah. I will add that.

drgrice1 · 2023-06-30T20:52:43Z

I added a section about the new tab to the help file. Although, I didn't have anything else to say other than what the tab already says.

drgrice1 · 2023-06-30T21:57:56Z

I decided that that lengthy explanation should only be in the help, and not directly on the tab. So the tab now simply say "Reformat the code using perltidy."

Alex-Jordan · 2023-07-01T23:46:30Z

I am at commit 651fa85 on pg, and 597103b on webwork2, so I think I have both branches where they should be. I have run npm ci in both places, and restarted webwork2. I restored localOveerrides.conf following my changes for the hardcopy refactor branch.

But I get this error anywhere on the problem editor page.

drgrice1 · 2023-07-02T01:28:51Z

I added Perl::Tidy to check_modules.pl . It needs to be version 20220613 or newer. Note that newer versions cause reformatting of webwork2 and pg code from that version, but should be fine for what is needed for the tidying of problems.

drgrice1 · 2023-07-02T01:49:30Z

@Alex-Jordan: I suspect that not having Perl::Tidy installed is your problem.

Alex-Jordan · 2023-07-02T03:46:31Z

That wasn't it. II have Perl::Tidy and it is $VERSION = '20220613';. Still getting that error.

The webwork2 error long has:

[2023-07-01 20:43:35.12892] [164601] [error] [eNZmnALDPO1S] Can't locate loadable object for module main::Tie::Hash::NamedCapture in @INC (@INC contains: /opt/webwork/pg/lib /opt/webwork/webwork2/lib /usr/local/lib64/perl5 /usr/local/share/perl5 /usr/lib64/perl5/vendor_perl /usr/share/perl5/vendor_perl /usr/lib64/perl5 /usr/share/perl5) at /usr/share/perl5/English.pm line 155.

Not sure what to make of that. Install Tie::Hash::NamedCapture?

Alex-Jordan · 2023-07-02T04:51:17Z

I had Perl::Tidy installed already somehow. I ran sudo cpanm Perl::Tidy to see what it would do, and it installed version 20230701. But nothing has changed. I still get that error in the webwork2 log file.

Alex-Jordan · 2023-07-02T04:53:51Z

OK, I see quite a bit about this after Googling the error message, for instance here: https://rt.cpan.org/Public/Bug/Display.html?id=77916

drgrice1 · 2023-07-02T10:42:32Z

Both Safe and WWSafe use Tie::Hash::NamedCapture. Safe is in check_modules.pl, although both Safe and Tie::Hash::NamedCapture are core perl modules. If you have perl installed on your system, you should have both of those.

This issue happens for you with this branch, but not with the WeBWorK-2.18 branch? Also, this doesn't happen when you view a problem in a set in the usual way?

Alex-Jordan · 2023-07-02T15:46:47Z

I have them installed and don't have this issue except with this branch. Did you see my later post that points to a known perl bug?

…

On Sun, Jul 2, 2023, 3:42 AM Glenn Rice ***@***.***> wrote: Both Safe and WWSafe use Tie::Hash::NamedCapture. Safe is in check_modules.pl, although both Safe and Tie::Hash::NamedCapture are core perl modules. If you have perl installed on your system, you should have both of those. This issue happens for you with this branch, but not with the WeBWorK-2.18 branch? Also, this doesn't happen when you view a problem in a set in the usual way? — Reply to this email directly, view it on GitHub <https://protect2.fireeye.com/v1/url?k=31323334-501cfaeb-3132feb7-454455535732-1667f6b60b096065&q=1&e=dd40cee6-e016-4957-a267-80d5b9a3f2bf&u=https%3A%2F%2Fgithub.com%2Fopenwebwork%2Fwebwork2%2Fpull%2F2098%23issuecomment-1616590040>, or unsubscribe <https://protect2.fireeye.com/v1/url?k=31323334-501cfaeb-3132feb7-454455535732-86bd7f120c7e1c5a&q=1&e=dd40cee6-e016-4957-a267-80d5b9a3f2bf&u=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FABEDOAAB37IHK43ATGXD4B3XOFGCHANCNFSM6AAAAAAZ2B5ADI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

drgrice1 · 2023-07-02T16:39:06Z

I have them installed and don't have this issue except with this branch. Did you see my later post that points to a known perl bug?

I saw that, but I don't see how that has anything to do with this pull request. This pull request doesn't change anything with the Safe or WWSafe usage, or problem rendering in general. That is why I asked if the issues you are seeing are restricted to this pull request, or also occur for the WeBWorK-2.18 branch.

drgrice1 · 2023-07-02T16:41:13Z

I also can not reproduce the issue on any of my systems.

Alex-Jordan · 2023-07-02T18:06:45Z

It may intersect with this flavor of OS (Oracle 8). I will try some things based on what people have posted. Seems to be about English.pm that is a dependency.

…

On Sun, Jul 2, 2023, 9:41 AM Glenn Rice ***@***.***> wrote: I also can not reproduce the issue on any of my systems. — Reply to this email directly, view it on GitHub <https://protect2.fireeye.com/v1/url?k=31323334-501cfaeb-3132feb7-454455535732-3d1fdabdc7c3b2ac&q=1&e=4d3f8dc7-0c2a-4b83-86c6-0e68f2411f21&u=https%3A%2F%2Fgithub.com%2Fopenwebwork%2Fwebwork2%2Fpull%2F2098%23issuecomment-1616727304>, or unsubscribe <https://protect2.fireeye.com/v1/url?k=31323334-501cfaeb-3132feb7-454455535732-2e4ff72c86d7fb59&q=1&e=4d3f8dc7-0c2a-4b83-86c6-0e68f2411f21&u=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FABEDOABHWURNTGECZCTQW2DXOGQDJANCNFSM6AAAAAAZ2B5ADI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

Alex-Jordan · 2023-07-02T21:36:00Z

I followed the suggestion from that link, putting *+; right before calling Safe->new; in lib/WeBWorK/CourseEnvironment.pm, and that resolves the bug for me. I confess I have no idea what *+; actually does.

Now that I can test the feature, it works!

This adds a new tab to the PG problem editor (unless editing a course configuration file). The tab contains basic information about what it does, but if the "Tidy Code" button is pressed it perltidies the code in the CodeMirror editor window. The tidied code is also saved to the temprorary file. The actual tidying is done by a new webwork webservice action (not a form submission). If errors occur, they are shown in the renderer window. The webwork webwservice depends on a new PG module defined in a corresponding PG pull request (see openwebwork/pg#868).

The version needs to be at least 20220613. Newer versions than that should be okay for this, although those cause reformatting of webwork2 and pg code from the 20220613.

the code in the editor window.

drgrice1 · 2023-07-03T10:55:54Z

I added a small tweak this. If there are no changes to the code when perltidy runs, then the code in the editor window is not updated, and the toast just says "There were no changes to the code."

@Alex-Jordan: I am not sure what to do about the Safe issue. Should I add *+; before calling Safe->new in CourseEnvironment.pm?

Alex-Jordan · 2023-07-03T16:21:07Z

Should I add *+; before calling Safe->new in CourseEnvironment.pm?

I would say yes, but I would like to know what that command even does first. If we know what that does, we can at least think about if it could have potential bad side effects. (Knowing how it "fixes" the issue I encountered would be nice, but not as important imho.)

I am not a perl monk and I wouldn't have even guessed that was a legal command. It's been a challenge to Google about it given the nature of that character string.

Alex-Jordan · 2023-07-04T08:45:38Z

OK, I sort of understand now. Maybe 85%.

Safe wants to use the special hash %+ for which you can read its meaning in perldoc perlvar. This special hash is implemented by Tie::Hash::NamedCapture, which is loaded dynamically when it's needed. Apparently on my system, something about this branch leads to Tie::Hash::NamedCapture not being loaded fast enough. So by accessing %+ before Safe is loaded, we make sure Tie::Hash::NamedCapture is loaded in time. And *+ seems like overkill, accessing %+ but also other things like $+. (I do not understand the star sigil...)

Anyway I tested that it is enough on my system to access %+ right before Safe. So I guess my suggestion is that you add %+; right before Safe is loaded. And include some comment, maybe referencing the bug report or this GitHub thread.

drgrice1 · 2023-07-04T11:52:45Z

@Alex-Jordan I added %+; before Safe is used in CourseEnvironment.pm. Thanks for figuring out what is needed there.

Alex-Jordan

Just did a fresh test and it's working well.

pstaabp · 2023-07-06T12:07:11Z

lib/WebworkWebservice/ProblemActions.pm

+sub tidyPGCode {
+	my ($invocant, $self, $params) = @_;
+
+	debug('in tidyPGCode');


Remove this line?

This has been the standard in the WebworkWebservice action, particularly in this file. Each method calls debug('in thismethod');. So I just followed suit. Most of the other WebworkWebservice modules also do this, although not as consistently as this file. I would certainly be fine with removing all of those. That was probably more meaningful with the old XML pipeline that this was inside of. Now it is probably just clutter.

pstaabp

Looks great.

…s.pm.

drgrice1 · 2023-07-07T14:23:42Z

lib/WeBWorK/CourseEnvironment.pm

+	# The following line is a work around for a bug that occurs on some systems.  See
+	# https://rt.cpan.org/Public/Bug/Display.html?id=77916 and
+	# https://github.com/openwebwork/webwork2/pull/2098#issuecomment-1619812699.
+	%+;


@Alex-Jordan: This is not right. I didn't notice this when I added this before. This is causing a warning every time the webwork2 app starts about the useless use of a variable in void context. This needs further investigation.

…ork#2098 This just assigns the result of calling `%+` to a dummy variable to avoid the warning that perl gives stating that this is a useless use of a variable in void context.

Fix a warning that is being emitted due to the hack added in #2098

This just assigns the result of calling `%+` to a dummy variable to avoid the warning that perl gives stating that this is a useless use of a variable in void context.

This was missed in openwebwork#2098.

drgrice1 force-pushed the pg-editor-perldity branch from bc4ac7f to 7aecd36 Compare June 30, 2023 16:34

drgrice1 force-pushed the pg-editor-perldity branch from 7aecd36 to 4932df7 Compare June 30, 2023 20:51

drgrice1 force-pushed the pg-editor-perldity branch from 4932df7 to 597103b Compare June 30, 2023 21:56

drgrice1 added 2 commits July 3, 2023 05:33

Add Perl::Tidy to check_modules.pl.

9f48f13

The version needs to be at least 20220613. Newer versions than that should be okay for this, although those cause reformatting of webwork2 and pg code from the 20220613.

drgrice1 force-pushed the pg-editor-perldity branch from 172df44 to 9f48f13 Compare July 3, 2023 10:33

If perltidy does not change the code, then report that and don't change

947f649

the code in the editor window.

Add a work around for a Tie::Hash::NamedCapture issue with Safe.

2931306

Alex-Jordan approved these changes Jul 4, 2023

View reviewed changes

pstaabp reviewed Jul 6, 2023

View reviewed changes

pstaabp approved these changes Jul 6, 2023

View reviewed changes

Remove the useless debug calls in lib/WebworkWebservice/ProblemAction…

404ef99

…s.pm.

pstaabp merged commit 6029d38 into openwebwork:WeBWorK-2.18 Jul 6, 2023

drgrice1 deleted the pg-editor-perldity branch July 6, 2023 13:37

drgrice1 commented Jul 7, 2023

View reviewed changes

Alex-Jordan added a commit that referenced this pull request Jul 12, 2023

Merge pull request #2104 from drgrice1/no-warn-useless-use

5f0a78b

Fix a warning that is being emitted due to the hack added in #2098

drgrice1 added a commit to drgrice1/webwork2 that referenced this pull request Jul 20, 2023

Add the installation of Perl::Tidy to the docker build.

b2e0a30

This was missed in openwebwork#2098.

drgrice1 mentioned this pull request Jul 20, 2023

Add the installation of Perl::Tidy to the docker build. #2142

Merged

Uh oh!

Add the capability in the pg problem editor to pg perltidy code. #2098

Add the capability in the pg problem editor to pg perltidy code. #2098

Uh oh!

Conversation

drgrice1 commented Jun 30, 2023

Uh oh!

pstaabp commented Jun 30, 2023

Uh oh!

drgrice1 commented Jun 30, 2023

Uh oh!

drgrice1 commented Jun 30, 2023

Uh oh!

drgrice1 commented Jun 30, 2023

Uh oh!

Alex-Jordan commented Jul 1, 2023

Uh oh!

drgrice1 commented Jul 2, 2023

Uh oh!

drgrice1 commented Jul 2, 2023

Uh oh!

Alex-Jordan commented Jul 2, 2023

Uh oh!

Alex-Jordan commented Jul 2, 2023

Uh oh!

Alex-Jordan commented Jul 2, 2023

Uh oh!

drgrice1 commented Jul 2, 2023

Uh oh!

Alex-Jordan commented Jul 2, 2023 via email

Uh oh!

drgrice1 commented Jul 2, 2023

Uh oh!

drgrice1 commented Jul 2, 2023

Uh oh!

Alex-Jordan commented Jul 2, 2023 via email

Uh oh!

Alex-Jordan commented Jul 2, 2023

Uh oh!

drgrice1 commented Jul 3, 2023

Uh oh!

Alex-Jordan commented Jul 3, 2023

Uh oh!

Alex-Jordan commented Jul 4, 2023

Uh oh!

drgrice1 commented Jul 4, 2023

Uh oh!

Alex-Jordan left a comment

Choose a reason for hiding this comment

Uh oh!

pstaabp Jul 6, 2023

Choose a reason for hiding this comment

Uh oh!

drgrice1 Jul 6, 2023

Choose a reason for hiding this comment

Uh oh!

pstaabp left a comment

Choose a reason for hiding this comment

Uh oh!

drgrice1 Jul 7, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants