Add PG critic to the PG problem editor.

drgrice1 · drgrice1 · commit f0e438fd1f75 · 2025-07-20T07:25:59.000-05:00
This adds the PG critic results from `WeBWorK::PG::Critic` to the PG
problem editor.  Note that the "Format Code" tab in the editor is now
the "Code Maintenance" tab.  This is because this new option is not a
code formatter.  All of the options fit into the category of code
maintenance, so this is a better name.
diff --git a/bin/check_modules.pl b/bin/check_modules.pl
@@ -88,6 +88,7 @@ =head1 DESCRIPTION
 	Net::OAuth
 	Opcode
 	Pandoc
+	Perl::Critic
 	Perl::Tidy
 	PHP::Serialization
 	Pod::Simple::Search
diff --git a/htdocs/js/PGProblemEditor/pgproblemeditor.js b/htdocs/js/PGProblemEditor/pgproblemeditor.js
@@ -161,6 +161,15 @@
 			?.addEventListener('change', () => (deleteBackupCheck.checked = true));
 	}
 
+	const renderArea = document.getElementById('pgedit-render-area');
+
+	const scrollToRenderArea = () => {
+		// Scroll to the top of the render window if the current scroll position is below that.
+		const renderAreaRect = renderArea.getBoundingClientRect();
+		const topBarHeight = document.querySelector('.webwork-logo')?.getBoundingClientRect().height ?? 0;
+		if (renderAreaRect.top < topBarHeight) window.scrollBy(0, renderAreaRect.top - topBarHeight);
+	};
+
 	// Send a request to the server to perltidy the current PG code in the CodeMirror editor.
 	const tidyPGCode = () => {
 		const request_object = { courseID: document.getElementsByName('courseID')[0]?.value };
@@ -177,6 +186,8 @@
 		fetch(webserviceURL, { method: 'post', mode: 'same-origin', body: new URLSearchParams(request_object) })
 			.then((response) => response.json())
 			.then((data) => {
+				if (data.error) throw new Error(data.error);
+				if (!data.result_data) throw new Error('An invalid response was received.');
 				if (data.result_data.status) {
 					if (data.result_data.errors) {
 						renderArea.innerHTML =
@@ -219,6 +230,8 @@
 		fetch(webserviceURL, { method: 'post', mode: 'same-origin', body: new URLSearchParams(request_object) })
 			.then((response) => response.json())
 			.then((data) => {
+				if (data.error) throw new Error(data.error);
+				if (!data.result_data) throw new Error('An invalid response was received.');
 				if (request_object.pgCode === data.result_data.pgmlCode) {
 					showMessage('There were no changes to the code.', true);
 				} else {
@@ -247,24 +260,26 @@
 		fetch(webserviceURL, { method: 'post', mode: 'same-origin', body: new URLSearchParams(request_object) })
 			.then((response) => response.json())
 			.then((data) => {
+				if (data.error) throw new Error(data.error);
+				if (!data.result_data) throw new Error('An invalid response was received.');
 				renderArea.innerHTML = data.result_data.html;
+				scrollToRenderArea();
 			})
-			.catch((err) => {
-				console.log(err);
-				showMessage(`Error: ${err?.message ?? err}`);
-			});
+			.catch((err) => showMessage(`Error: ${err?.message ?? err}`));
 	};
 
 	document.getElementById('take_action')?.addEventListener('click', async (e) => {
-		if (document.getElementById('current_action')?.value === 'format_code') {
+		if (document.getElementById('current_action')?.value === 'code_maintenance') {
 			e.preventDefault();
-			if (document.querySelector('input[name="action.format_code"]:checked').value == 'tidyPGCode') {
+			if (document.querySelector('input[name="action.code_maintenance"]:checked').value === 'tidyPGCode') {
 				tidyPGCode();
 			} else if (
-				document.querySelector('input[name="action.format_code"]:checked').value == 'convertCodeToPGML'
+				document.querySelector('input[name="action.code_maintenance"]:checked').value === 'convertCodeToPGML'
 			) {
 				convertCodeToPGML();
-			} else if (document.querySelector('input[name="action.format_code"]:checked').value == 'runPGCritic') {
+			} else if (
+				document.querySelector('input[name="action.code_maintenance"]:checked').value === 'runPGCritic'
+			) {
 				runPGCritic();
 			}
 			return;
@@ -328,7 +343,6 @@
 	});
 
 	const renderURL = `${webworkConfig?.webwork_url ?? '/webwork2'}/render_rpc`;
-	const renderArea = document.getElementById('pgedit-render-area');
 	const fileType = document.getElementsByName('file_type')[0]?.value;
 
 	// This is either the div containing the CodeMirror editor or the problemContents textarea in the case that
@@ -412,11 +426,7 @@
 		}
 
 		adjustIFrameHeight();
-
-		// Scroll to the top of the render window if the current scroll position is below that.
-		const renderAreaRect = renderArea.getBoundingClientRect();
-		const topBarHeight = document.querySelector('.webwork-logo')?.getBoundingClientRect().height ?? 0;
-		if (renderAreaRect.top < topBarHeight) window.scrollBy(0, renderAreaRect.top - topBarHeight);
+		scrollToRenderArea();
 	});
 
 	const render = () =>
diff --git a/lib/WeBWorK/ContentGenerator/Instructor/PGProblemEditor.pm b/lib/WeBWorK/ContentGenerator/Instructor/PGProblemEditor.pm
@@ -105,7 +105,7 @@ the submit button pressed (the action).
     Requested actions and aliases
         View/Reload                action = view
         Generate Hardcopy:         action = hardcopy
-        Format Code:               action = format_code
+        Code Maintenance:          action = code_maintenance
         Save:                      action = save
         Save as:                   action = save_as
         Append:                    action = add_problem
@@ -133,15 +133,15 @@ use SampleProblemParser        qw(getSampleProblemCode generateMetadata);
 use constant DEFAULT_SEED => 123456;
 
 # Editor tabs
-use constant ACTION_FORMS => [qw(view hardcopy format_code save save_as add_problem revert)];
+use constant ACTION_FORMS => [qw(view hardcopy code_maintenance save save_as add_problem revert)];
 use constant ACTION_FORM_TITLES => {
-	view        => x('View/Reload'),
-	hardcopy    => x('Generate Hardcopy'),
-	format_code => x('Format Code'),
-	save        => x('Save'),
-	save_as     => x('Save As'),
-	add_problem => x('Append'),
-	revert      => x('Revert'),
+	view             => x('View/Reload'),
+	hardcopy         => x('Generate Hardcopy'),
+	code_maintenance => x('Code Maintenance'),
+	save             => x('Save'),
+	save_as          => x('Save As'),
+	add_problem      => x('Append'),
+	revert           => x('Revert'),
 };
 
 my $BLANKPROBLEM = 'newProblem.pg';
@@ -862,9 +862,9 @@ sub view_handler ($c) {
 	return;
 }
 
-# The format_code action is handled by javascript.  This is provided just in case
+# The code_maintenance action is handled by javascript.  This is provided just in case
 # something goes wrong and the handler is called.
-sub format_code_handler { }
+sub code_maintenance_handler { }
 
 sub hardcopy_handler ($c) {
 	# Redirect to problem editor page.
diff --git a/lib/WebworkWebservice/ProblemActions.pm b/lib/WebworkWebservice/ProblemActions.pm
@@ -21,9 +21,9 @@ use warnings;
 
 use Data::Structure::Util qw(unbless);
 
-use WeBWorK::PG::Tidy            qw(pgtidy);
-use WeBWorK::PG::ConvertToPGML   qw(convertToPGML);
-use WeBWorK::PG::PGProblemCritic qw(analyzePGcode);
+use WeBWorK::PG::Tidy          qw(pgtidy);
+use WeBWorK::PG::ConvertToPGML qw(convertToPGML);
+use WeBWorK::PG::Critic        qw(critiquePGCode);
 
 sub getUserProblem {
 	my ($invocant, $self, $params) = @_;
@@ -183,16 +183,13 @@ sub convertCodeToPGML {
 
 sub runPGCritic {
 	my ($invocant, $self, $params) = @_;
-	my $pg_critic_results = analyzePGcode($params->{pgCode});
-
-	my $html_output = $self->c->render_to_string(
-		template => 'ContentGenerator/Instructor/PGProblemEditor/pg_critic',
-		results  => $pg_critic_results
-	);
 
 	return {
 		ra_out => {
-			html => $html_output
+			html => $self->c->render_to_string(
+				template => 'ContentGenerator/Instructor/PGProblemEditor/pg_critic',
+				results  => [ critiquePGCode($params->{pgCode}) ]
+			)
 		},
 		text => 'The script pg-critic has been run successfully.'
 	};
diff --git a/templates/ContentGenerator/Instructor/PGProblemEditor/code_maintenance_form.html.ep b/templates/ContentGenerator/Instructor/PGProblemEditor/code_maintenance_form.html.ep
@@ -1,9 +1,9 @@
 % last unless $c->{is_pg};
 <div>
 	<div class="form-check">
-		<%= radio_button 'action.format_code' => 'tidyPGCode',
-			id => 'action_format_code_perltidy', class => 'form-check-input', checked => undef  =%>
-		<%= label_for 'action_format_code_perltidy', class => 'form-check-label', begin =%>
+		<%= radio_button 'action.code_maintenance' => 'tidyPGCode',
+			id => 'action_code_maintenance_perltidy', class => 'form-check-input', checked => undef  =%>
+		<%= label_for 'action_code_maintenance_perltidy', class => 'form-check-label', begin =%>
 			<%== maketext('Reformat the code using perltidy.') =%>
 		<% end =%>
 		<a class="help-popup" data-bs-content="<%== maketext('Perltidy is a reformatting '
@@ -15,31 +15,31 @@
 		</a>
 	</div>
 	<div class="form-check">
-		<%= radio_button 'action.format_code' => 'convertCodeToPGML',
-			id => 'action_format_code_convert_PGML', class => 'form-check-input'=%>
-		<%= label_for 'action_format_code_convert_PGML', class => 'form-check-label', begin =%>
+		<%= radio_button 'action.code_maintenance' => 'convertCodeToPGML',
+			id => 'action_code_maintenance_convert_PGML', class => 'form-check-input'=%>
+		<%= label_for 'action_code_maintenance_convert_PGML', class => 'form-check-label', begin =%>
 			<%== maketext('Convert the code to PGML') =%>
 		<% end =%>
 		<a class="help-popup" data-bs-content="<%== maketext('This option converts the text blocks '
 			. 'in the problem code to PGML and updates the loadMacros to include PGML and drop others. '
 			. 'This can be used as a first pass of the conversion, however the author will still need '
 			. 'to ensure the problem functions.  One area of attention should be the answer blanks, '
 			. 'which may not be converted correctly.') =%>"
-		data-bs-placement="top" data-bs-toggle="popover" role="button">
+			data-bs-placement="top" data-bs-toggle="popover" role="button">
 			<i aria-hidden="true" class="fas fa-question-circle"></i>
 			<span class="visually-hidden"><%= maketext('PGML Conversion Help') %></span>
 		</a>
 	</div>
 	<div class="form-check">
-		<%= radio_button 'action.format_code' => 'runPGCritic',
-			id => 'action_format_code_run_pgcritic', class => 'form-check-input'=%>
-		<%= label_for 'action_format_code_run_pgcritic', class => 'form-check-label', begin =%>
+		<%= radio_button 'action.code_maintenance' => 'runPGCritic',
+			id => 'action_code_maintenance_run_pgcritic', class => 'form-check-input'=%>
+		<%= label_for 'action_code_maintenance_run_pgcritic', class => 'form-check-label', begin =%>
 			<%== maketext('Run the PG Critic Analyzer') =%>
 		<% end =%>
 		<a class="help-popup" data-bs-content="<%== maketext('This option runs the PG Critic '
-			. 'code analyzer, which gives suggestions on using more modern PG constructs and  '
-			. 'ensure that you include important features. ') =%>"
-		data-bs-placement="top" data-bs-toggle="popover" role="button">
+			. 'code analyzer which gives suggestions on using modern PG constructs and  '
+			. 'ensures that you include important features.') =%>"
+			data-bs-placement="top" data-bs-toggle="popover" role="button">
 			<i aria-hidden="true" class="fas fa-question-circle"></i>
 			<span class="visually-hidden"><%= maketext('PG Critic Help') %></span>
 		</a>
diff --git a/templates/ContentGenerator/Instructor/PGProblemEditor/pg_critic.html.ep b/templates/ContentGenerator/Instructor/PGProblemEditor/pg_critic.html.ep
diff --git a/templates/HelpFiles/InstructorPGProblemEditor.html.ep b/templates/HelpFiles/InstructorPGProblemEditor.html.ep
