From 025b739f334cab75b66779508c1903c0bf81dc70 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Fri, 30 Jun 2023 09:48:01 -0500 Subject: [PATCH 1/5] Add the capability in the pg problem editor to pg perltidy code. 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 https://github.com/openwebwork/pg/pull/868). --- conf/snippets/newProblem.pg | 8 ++-- htdocs/js/PGProblemEditor/pgproblemeditor.js | 42 +++++++++++++++++++ .../Instructor/PGProblemEditor.pm | 11 +++-- lib/WebworkWebservice.pm | 1 + lib/WebworkWebservice/ProblemActions.pm | 20 +++++++++ .../PGProblemEditor/pgtidy_form.html.ep | 6 +++ .../InstructorPGProblemEditor.html.ep | 8 ++++ 7 files changed, 88 insertions(+), 8 deletions(-) create mode 100644 templates/ContentGenerator/Instructor/PGProblemEditor/pgtidy_form.html.ep diff --git a/conf/snippets/newProblem.pg b/conf/snippets/newProblem.pg index 3ab92489b4..75289789ee 100644 --- a/conf/snippets/newProblem.pg +++ b/conf/snippets/newProblem.pg @@ -14,9 +14,9 @@ DOCUMENT(); loadMacros( - "PGstandard.pl", # Standard macros for PG language - "PGML.pl", # PGML markup and Math Objects - "PGcourse.pl", # Customization file for the course + "PGstandard.pl", # Standard macros for PG language + "PGML.pl", # PGML markup and Math Objects + "PGcourse.pl", # Customization file for the course ); # Uncomment the following if you don't want to show which @@ -34,6 +34,6 @@ END_PGML BEGIN_PGML_SOLUTION You could type [|pi|]* or [|3.14|]*, or [|22/7|]*, among other options. -END_PGML_SOLUTION +END_PGML_SOLUTION ENDDOCUMENT(); diff --git a/htdocs/js/PGProblemEditor/pgproblemeditor.js b/htdocs/js/PGProblemEditor/pgproblemeditor.js index b0dedbf798..0e4f375fad 100644 --- a/htdocs/js/PGProblemEditor/pgproblemeditor.js +++ b/htdocs/js/PGProblemEditor/pgproblemeditor.js @@ -98,7 +98,49 @@ ?.addEventListener('change', () => deleteBackupCheck.checked = true); } + // Send a request to the server to perltidy the current PG code in the CodeMirror editor. + const tidyPGCode = () => { + const request_object = { + user: document.getElementById('hidden_user')?.value, + courseID: document.getElementsByName('courseID')[0]?.value, + key: document.getElementById('hidden_key')?.value + }; + + request_object.rpc_command = 'tidyPGCode'; + request_object.pgCode = webworkConfig?.pgCodeMirror?.getValue() + ?? document.getElementById('problemContents')?.value ?? ''; + + fetch(webserviceURL, { method: 'post', mode: 'same-origin', body: new URLSearchParams(request_object) }) + .then((response) => response.json()) + .then((data) => { + if (data.result_data.status) { + if (data.result_data.errors) { + renderArea.innerHTML = '
' + + '

PG perltidy errors:

' + + '
' +
+							data.result_data.errors
+							.replace(/^[\s\S]*Begin Error Output Stream\n\n/, '')
+							.replace(/\n\d*: To save a full \.LOG file rerun with -g/, '') +
+							'
'; + } + showMessage('Errors occurred perltidying code.', false); + return; + } + if (webworkConfig?.pgCodeMirror) webworkConfig.pgCodeMirror.setValue(data.result_data.tidiedPGCode); + else document.getElementById('problemContents').value = data.result_data.tidiedPGCode; + saveTempFile(); + showMessage('Successfuly perltidied code.', true); + }) + .catch((err) => showMessage(`Error: ${err?.message ?? err}`)); + }; + document.getElementById('take_action')?.addEventListener('click', async (e) => { + if (document.getElementById('current_action')?.value === 'pgtidy') { + e.preventDefault(); + tidyPGCode(); + return; + } + const actionView = document.getElementById('view'); const editorForm = document.getElementById('editor'); diff --git a/lib/WeBWorK/ContentGenerator/Instructor/PGProblemEditor.pm b/lib/WeBWorK/ContentGenerator/Instructor/PGProblemEditor.pm index 8a8f2dc3a1..2600999e38 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/PGProblemEditor.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/PGProblemEditor.pm @@ -96,6 +96,7 @@ the submit button pressed (the action). Make this set header for: action = add_problem Revert: action = revert Generate Hardcopy: action = hardcopy + Tidy Code: action = pgtidy An undefined or invalid action is interpreted as an initial edit of the file. @@ -115,13 +116,14 @@ use WeBWorK::Utils::Instructor qw(assignProblemToAllSetUsers addProblemToSet); use constant DEFAULT_SEED => 123456; # Editor tabs -use constant ACTION_FORMS => [qw(view hardcopy save save_as add_problem revert)]; +use constant ACTION_FORMS => [qw(view hardcopy pgtidy save save_as add_problem revert)]; use constant ACTION_FORM_TITLES => { view => x('View/Reload'), hardcopy => x('Generate Hardcopy'), - add_problem => x('Append'), + pgtidy => x('Tidy Code'), save => x('Save'), save_as => x('Save As'), + add_problem => x('Append'), revert => x('Revert'), }; @@ -778,9 +780,10 @@ sub view_handler ($c) { return; } -# The hardcopy action is handled by javascript. This is provided just in case -# something goes wrong and the action gets called. +# The hardcopy and pgtidy actions are handled by javascript. These are provided just in case +# something goes wrong and the actions are called. sub hardcopy_action { } +sub pgtidy_action { } sub add_problem_handler ($c) { my $db = $c->db; diff --git a/lib/WebworkWebservice.pm b/lib/WebworkWebservice.pm index f5f162d968..d6e6f68a23 100644 --- a/lib/WebworkWebservice.pm +++ b/lib/WebworkWebservice.pm @@ -268,6 +268,7 @@ sub command_permission { putUserProblem => 'modify_student_data', putProblemVersion => 'modify_student_data', putPastAnswer => 'modify_student_data', + tidyPGCode => 'access_instructor_tools', # WebworkWebservice::RenderProblem renderProblem => 'proctor_quiz_login', diff --git a/lib/WebworkWebservice/ProblemActions.pm b/lib/WebworkWebservice/ProblemActions.pm index ae51dc1866..f4ec133336 100644 --- a/lib/WebworkWebservice/ProblemActions.pm +++ b/lib/WebworkWebservice/ProblemActions.pm @@ -22,6 +22,7 @@ use warnings; use Data::Structure::Util qw(unbless); use WeBWorK::Debug; +use WeBWorK::PG::Tidy qw(pgtidy); sub getUserProblem { my ($invocant, $self, $params) = @_; @@ -133,4 +134,23 @@ sub putPastAnswer { }; } +sub tidyPGCode { + my ($invocant, $self, $params) = @_; + + debug('in tidyPGCode'); + + local @ARGV = (); + + my $code = $params->{pgCode}; + my $tidiedPGCode; + my $errors; + + my $result = pgtidy(source => \$code, destination => \$tidiedPGCode, errorfile => \$errors); + + return { + ra_out => { tidiedPGCode => $tidiedPGCode, status => $result, errors => $errors }, + text => 'Tidied code' + }; +} + 1; diff --git a/templates/ContentGenerator/Instructor/PGProblemEditor/pgtidy_form.html.ep b/templates/ContentGenerator/Instructor/PGProblemEditor/pgtidy_form.html.ep new file mode 100644 index 0000000000..c099b968d5 --- /dev/null +++ b/templates/ContentGenerator/Instructor/PGProblemEditor/pgtidy_form.html.ep @@ -0,0 +1,6 @@ +% # Hardcopy headers are previewed from the hardcopy generation tab. +% last if $c->{file_type} eq 'course_info'; +% +
+

<%= maketext('Reformat the code using perltidy.') =%>

+
diff --git a/templates/HelpFiles/InstructorPGProblemEditor.html.ep b/templates/HelpFiles/InstructorPGProblemEditor.html.ep index ebdf6d65dc..d7b5395cc6 100644 --- a/templates/HelpFiles/InstructorPGProblemEditor.html.ep +++ b/templates/HelpFiles/InstructorPGProblemEditor.html.ep @@ -120,6 +120,14 @@ . 'generating the PDF file using pdflatex.') =%> +
<%= maketext('Tidy Code') %>
+
+ <%= maketext('Reformat the code using perltidy. This will change the code in the editor window, and save ' + . 'changes to the temporary file. In some cases (if the code contains backslashes or double tildes) this ' + . 'can result in odd spacing in the code. So make sure to inspect the formatted code, and edit further or ' + . 'revert if needed.') =%> +
+
<%= maketext('Save') %>
<%= maketext('Save the contents of the editor window to the file on disk and re-render the problem. If ' From 9f48f13c8a8e7b41a3251b3304e8b397073a0a5f Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Sat, 1 Jul 2023 20:26:04 -0500 Subject: [PATCH 2/5] Add Perl::Tidy to check_modules.pl. 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. --- bin/check_modules.pl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bin/check_modules.pl b/bin/check_modules.pl index 7bb9a38bb7..6aef84329d 100755 --- a/bin/check_modules.pl +++ b/bin/check_modules.pl @@ -127,6 +127,7 @@ =head1 DESCRIPTION Opcode PadWalker Path::Class + Perl::Tidy PHP::Serialization Pod::Simple::Search Pod::Simple::XHTML @@ -160,6 +161,7 @@ =head1 DESCRIPTION 'LWP::Protocol::https' => 6.06, 'Mojolicious' => 9.22, 'Net::SSLeay' => 1.46, + 'Perl::Tidy' => 20220613 ); my ($test_programs, $test_modules, $show_help); From 947f64911af7a7f5ef7055d613ad262086322fc6 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Mon, 3 Jul 2023 05:47:22 -0500 Subject: [PATCH 3/5] If perltidy does not change the code, then report that and don't change the code in the editor window. --- htdocs/js/PGProblemEditor/pgproblemeditor.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/htdocs/js/PGProblemEditor/pgproblemeditor.js b/htdocs/js/PGProblemEditor/pgproblemeditor.js index 0e4f375fad..64a92a0ca3 100644 --- a/htdocs/js/PGProblemEditor/pgproblemeditor.js +++ b/htdocs/js/PGProblemEditor/pgproblemeditor.js @@ -126,10 +126,14 @@ showMessage('Errors occurred perltidying code.', false); return; } - if (webworkConfig?.pgCodeMirror) webworkConfig.pgCodeMirror.setValue(data.result_data.tidiedPGCode); - else document.getElementById('problemContents').value = data.result_data.tidiedPGCode; - saveTempFile(); - showMessage('Successfuly perltidied code.', true); + if (request_object.pgCode === data.result_data.tidiedPGCode) { + showMessage('There were no changes to the code.', true); + } else { + if (webworkConfig?.pgCodeMirror) webworkConfig.pgCodeMirror.setValue(data.result_data.tidiedPGCode); + else document.getElementById('problemContents').value = data.result_data.tidiedPGCode; + saveTempFile(); + showMessage('Successfuly perltidied code.', true); + } }) .catch((err) => showMessage(`Error: ${err?.message ?? err}`)); }; From 2931306a27b28f6a93832185c0cd03ec2341b677 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Tue, 4 Jul 2023 06:51:24 -0500 Subject: [PATCH 4/5] Add a work around for a `Tie::Hash::NamedCapture` issue with `Safe`. --- lib/WeBWorK/CourseEnvironment.pm | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/WeBWorK/CourseEnvironment.pm b/lib/WeBWorK/CourseEnvironment.pm index afe0c9d502..ac1efeaeee 100644 --- a/lib/WeBWorK/CourseEnvironment.pm +++ b/lib/WeBWorK/CourseEnvironment.pm @@ -101,6 +101,11 @@ sub new { my %ORIG_SIG; $ORIG_SIG{$_} = $SIG{$_} for keys %SIG; + # 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. + %+; + my $safe = Safe->new; $safe->permit('rand'); # seed course environment with initial values From 404ef99f4c6ef280a46b15d793824938c34caa27 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Thu, 6 Jul 2023 07:16:03 -0500 Subject: [PATCH 5/5] Remove the useless debug calls in lib/WebworkWebservice/ProblemActions.pm. --- lib/WebworkWebservice/ProblemActions.pm | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/lib/WebworkWebservice/ProblemActions.pm b/lib/WebworkWebservice/ProblemActions.pm index f4ec133336..8f80e35782 100644 --- a/lib/WebworkWebservice/ProblemActions.pm +++ b/lib/WebworkWebservice/ProblemActions.pm @@ -21,14 +21,11 @@ use warnings; use Data::Structure::Util qw(unbless); -use WeBWorK::Debug; use WeBWorK::PG::Tidy qw(pgtidy); sub getUserProblem { my ($invocant, $self, $params) = @_; - debug('in getUserProblem'); - my $db = $self->db; my $userProblem = $db->getUserProblem($params->{user_id}, $params->{set_id}, $params->{problem_id}); @@ -44,8 +41,6 @@ sub getUserProblem { sub putUserProblem { my ($invocant, $self, $params) = @_; - debug('in putUserProblem'); - my $db = $self->db; my $userProblem = $db->getUserProblem($params->{user_id}, $params->{set_id}, $params->{problem_id}); @@ -76,8 +71,6 @@ sub putUserProblem { sub putProblemVersion { my ($invocant, $self, $params) = @_; - debug('in putProblemVersion'); - my $db = $self->db; my $problemVersion = @@ -109,8 +102,6 @@ sub putProblemVersion { sub putPastAnswer { my ($invocant, $self, $params) = @_; - debug('in putPastAnswer'); - my $db = $self->db; my $pastAnswer = $db->getPastAnswer($params->{answer_id}); @@ -137,8 +128,6 @@ sub putPastAnswer { sub tidyPGCode { my ($invocant, $self, $params) = @_; - debug('in tidyPGCode'); - local @ARGV = (); my $code = $params->{pgCode};